diff options
author | Devin Heitmueller <dheitmueller@kernellabs.com> | 2012-08-06 22:47:07 -0300 |
---|---|---|
committer | Mauro Carvalho Chehab <mchehab@redhat.com> | 2012-08-11 12:37:55 -0300 |
commit | 77fc286328e6fd9a80ef8ce6963db7fbf4e07a1a (patch) | |
tree | b7b320eadf82fd8649790a2791f37a1273661307 /drivers/media/video/au0828/au0828-core.c | |
parent | 1240ad56edfdd9ccc70d9cbce6da7d5d939d5e83 (diff) | |
download | linux-stable-77fc286328e6fd9a80ef8ce6963db7fbf4e07a1a.tar.gz linux-stable-77fc286328e6fd9a80ef8ce6963db7fbf4e07a1a.tar.bz2 linux-stable-77fc286328e6fd9a80ef8ce6963db7fbf4e07a1a.zip |
[media] au0828: fix possible race condition in usage of dev->ctrlmsg
The register read function is referencing the dev->ctrlmsg structure outside
of the dev->mutex lock, which can cause corruption of the value if multiple
callers are invoking au0828_readreg() simultaneously.
Use a stack variable to hold the result, and copy the buffer returned by
usb_control_msg() to that variable.
In reality, the whole recv_control_msg() function can probably be collapsed
into au0288_readreg() since it is the only caller.
Also get rid of cmd_msg_dump() since the only case in which the function is
ever called only is ever passed a single byte for the response (and it is
already logged).
Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Diffstat (limited to 'drivers/media/video/au0828/au0828-core.c')
-rw-r--r-- | drivers/media/video/au0828/au0828-core.c | 40 |
1 files changed, 12 insertions, 28 deletions
diff --git a/drivers/media/video/au0828/au0828-core.c b/drivers/media/video/au0828/au0828-core.c index 65914bc421d0..745a80a798c8 100644 --- a/drivers/media/video/au0828/au0828-core.c +++ b/drivers/media/video/au0828/au0828-core.c @@ -56,9 +56,12 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value, u32 au0828_readreg(struct au0828_dev *dev, u16 reg) { - recv_control_msg(dev, CMD_REQUEST_IN, 0, reg, dev->ctrlmsg, 1); - dprintk(8, "%s(0x%04x) = 0x%02x\n", __func__, reg, dev->ctrlmsg[0]); - return dev->ctrlmsg[0]; + u8 result = 0; + + recv_control_msg(dev, CMD_REQUEST_IN, 0, reg, &result, 1); + dprintk(8, "%s(0x%04x) = 0x%02x\n", __func__, reg, result); + + return result; } u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val) @@ -67,24 +70,6 @@ u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val) return send_control_msg(dev, CMD_REQUEST_OUT, val, reg); } -static void cmd_msg_dump(struct au0828_dev *dev) -{ - int i; - - for (i = 0; i < sizeof(dev->ctrlmsg); i += 16) - dprintk(2, "%s() %02x %02x %02x %02x %02x %02x %02x %02x " - "%02x %02x %02x %02x %02x %02x %02x %02x\n", - __func__, - dev->ctrlmsg[i+0], dev->ctrlmsg[i+1], - dev->ctrlmsg[i+2], dev->ctrlmsg[i+3], - dev->ctrlmsg[i+4], dev->ctrlmsg[i+5], - dev->ctrlmsg[i+6], dev->ctrlmsg[i+7], - dev->ctrlmsg[i+8], dev->ctrlmsg[i+9], - dev->ctrlmsg[i+10], dev->ctrlmsg[i+11], - dev->ctrlmsg[i+12], dev->ctrlmsg[i+13], - dev->ctrlmsg[i+14], dev->ctrlmsg[i+15]); -} - static int send_control_msg(struct au0828_dev *dev, u16 request, u32 value, u16 index) { @@ -118,24 +103,23 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value, int status = -ENODEV; mutex_lock(&dev->mutex); if (dev->usbdev) { - - memset(dev->ctrlmsg, 0, sizeof(dev->ctrlmsg)); - - /* cp must be memory that has been allocated by kmalloc */ status = usb_control_msg(dev->usbdev, usb_rcvctrlpipe(dev->usbdev, 0), request, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, value, index, - cp, size, 1000); + dev->ctrlmsg, size, 1000); status = min(status, 0); if (status < 0) { printk(KERN_ERR "%s() Failed receiving control message, error %d.\n", __func__, status); - } else - cmd_msg_dump(dev); + } + + /* the host controller requires heap allocated memory, which + is why we didn't just pass "cp" into usb_control_msg */ + memcpy(cp, dev->ctrlmsg, size); } mutex_unlock(&dev->mutex); return status; |