media: lirc: improve locking
Once rc_unregister_device() has been called, no driver function
should be called.
This prevents some nasty race conditions with an ioctl calls
driver functions when the driver specific data has been freed.
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 7b9246f..2186589 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -233,15 +233,21 @@
struct rc_dev *dev = fh->rc;
unsigned int *txbuf = NULL;
struct ir_raw_event *raw = NULL;
- ssize_t ret = -EINVAL;
+ ssize_t ret;
size_t count;
ktime_t start;
s64 towait;
unsigned int duration = 0; /* signal duration in us */
int i;
- if (!dev->registered)
- return -ENODEV;
+ ret = mutex_lock_interruptible(&dev->lock);
+ if (ret)
+ return ret;
+
+ if (!dev->registered) {
+ ret = -ENODEV;
+ goto out;
+ }
start = ktime_get();
@@ -253,14 +259,20 @@
if (fh->send_mode == LIRC_MODE_SCANCODE) {
struct lirc_scancode scan;
- if (n != sizeof(scan))
- return -EINVAL;
+ if (n != sizeof(scan)) {
+ ret = -EINVAL;
+ goto out;
+ }
- if (copy_from_user(&scan, buf, sizeof(scan)))
- return -EFAULT;
+ if (copy_from_user(&scan, buf, sizeof(scan))) {
+ ret = -EFAULT;
+ goto out;
+ }
- if (scan.flags || scan.keycode || scan.timestamp)
- return -EINVAL;
+ if (scan.flags || scan.keycode || scan.timestamp) {
+ ret = -EINVAL;
+ goto out;
+ }
/*
* The scancode field in lirc_scancode is 64-bit simply
@@ -269,12 +281,16 @@
* are supported.
*/
if (scan.scancode > U32_MAX ||
- !rc_validate_scancode(scan.rc_proto, scan.scancode))
- return -EINVAL;
+ !rc_validate_scancode(scan.rc_proto, scan.scancode)) {
+ ret = -EINVAL;
+ goto out;
+ }
raw = kmalloc_array(LIRCBUF_SIZE, sizeof(*raw), GFP_KERNEL);
- if (!raw)
- return -ENOMEM;
+ if (!raw) {
+ ret = -ENOMEM;
+ goto out;
+ }
ret = ir_raw_encode_scancode(scan.rc_proto, scan.scancode,
raw, LIRCBUF_SIZE);
@@ -300,16 +316,22 @@
dev->s_tx_carrier(dev, carrier);
}
} else {
- if (n < sizeof(unsigned int) || n % sizeof(unsigned int))
- return -EINVAL;
+ if (n < sizeof(unsigned int) || n % sizeof(unsigned int)) {
+ ret = -EINVAL;
+ goto out;
+ }
count = n / sizeof(unsigned int);
- if (count > LIRCBUF_SIZE || count % 2 == 0)
- return -EINVAL;
+ if (count > LIRCBUF_SIZE || count % 2 == 0) {
+ ret = -EINVAL;
+ goto out;
+ }
txbuf = memdup_user(buf, n);
- if (IS_ERR(txbuf))
- return PTR_ERR(txbuf);
+ if (IS_ERR(txbuf)) {
+ ret = PTR_ERR(txbuf);
+ goto out;
+ }
}
for (i = 0; i < count; i++) {
@@ -347,6 +369,7 @@
}
out:
+ mutex_unlock(&dev->lock);
kfree(txbuf);
kfree(raw);
return ret;
@@ -358,8 +381,8 @@
struct lirc_fh *fh = file->private_data;
struct rc_dev *dev = fh->rc;
u32 __user *argp = (u32 __user *)(arg);
- int ret = 0;
- __u32 val = 0, tmp;
+ u32 val = 0;
+ int ret;
if (_IOC_DIR(cmd) & _IOC_WRITE) {
ret = get_user(val, argp);
@@ -367,8 +390,14 @@
return ret;
}
- if (!dev->registered)
- return -ENODEV;
+ ret = mutex_lock_interruptible(&dev->lock);
+ if (ret)
+ return ret;
+
+ if (!dev->registered) {
+ ret = -ENODEV;
+ goto out;
+ }
switch (cmd) {
case LIRC_GET_FEATURES:
@@ -409,155 +438,161 @@
/* mode support */
case LIRC_GET_REC_MODE:
if (dev->driver_type == RC_DRIVER_IR_RAW_TX)
- return -ENOTTY;
-
- val = fh->rec_mode;
+ ret = -ENOTTY;
+ else
+ val = fh->rec_mode;
break;
case LIRC_SET_REC_MODE:
switch (dev->driver_type) {
case RC_DRIVER_IR_RAW_TX:
- return -ENOTTY;
+ ret = -ENOTTY;
+ break;
case RC_DRIVER_SCANCODE:
if (val != LIRC_MODE_SCANCODE)
- return -EINVAL;
+ ret = -EINVAL;
break;
case RC_DRIVER_IR_RAW:
if (!(val == LIRC_MODE_MODE2 ||
val == LIRC_MODE_SCANCODE))
- return -EINVAL;
+ ret = -EINVAL;
break;
}
- fh->rec_mode = val;
- return 0;
+ if (!ret)
+ fh->rec_mode = val;
+ break;
case LIRC_GET_SEND_MODE:
if (!dev->tx_ir)
- return -ENOTTY;
-
- val = fh->send_mode;
+ ret = -ENOTTY;
+ else
+ val = fh->send_mode;
break;
case LIRC_SET_SEND_MODE:
if (!dev->tx_ir)
- return -ENOTTY;
-
- if (!(val == LIRC_MODE_PULSE || val == LIRC_MODE_SCANCODE))
- return -EINVAL;
-
- fh->send_mode = val;
- return 0;
+ ret = -ENOTTY;
+ else if (!(val == LIRC_MODE_PULSE || val == LIRC_MODE_SCANCODE))
+ ret = -EINVAL;
+ else
+ fh->send_mode = val;
+ break;
/* TX settings */
case LIRC_SET_TRANSMITTER_MASK:
if (!dev->s_tx_mask)
- return -ENOTTY;
-
- return dev->s_tx_mask(dev, val);
+ ret = -ENOTTY;
+ else
+ ret = dev->s_tx_mask(dev, val);
+ break;
case LIRC_SET_SEND_CARRIER:
if (!dev->s_tx_carrier)
- return -ENOTTY;
-
- return dev->s_tx_carrier(dev, val);
+ ret = -ENOTTY;
+ else
+ ret = dev->s_tx_carrier(dev, val);
+ break;
case LIRC_SET_SEND_DUTY_CYCLE:
if (!dev->s_tx_duty_cycle)
- return -ENOTTY;
-
- if (val <= 0 || val >= 100)
- return -EINVAL;
-
- return dev->s_tx_duty_cycle(dev, val);
+ ret = -ENOTTY;
+ else if (val <= 0 || val >= 100)
+ ret = -EINVAL;
+ else
+ ret = dev->s_tx_duty_cycle(dev, val);
+ break;
/* RX settings */
case LIRC_SET_REC_CARRIER:
if (!dev->s_rx_carrier_range)
- return -ENOTTY;
-
- if (val <= 0)
- return -EINVAL;
-
- return dev->s_rx_carrier_range(dev,
- fh->carrier_low,
- val);
+ ret = -ENOTTY;
+ else if (val <= 0)
+ ret = -EINVAL;
+ else
+ ret = dev->s_rx_carrier_range(dev, fh->carrier_low,
+ val);
+ break;
case LIRC_SET_REC_CARRIER_RANGE:
if (!dev->s_rx_carrier_range)
- return -ENOTTY;
-
- if (val <= 0)
- return -EINVAL;
-
- fh->carrier_low = val;
- return 0;
+ ret = -ENOTTY;
+ else if (val <= 0)
+ ret = -EINVAL;
+ else
+ fh->carrier_low = val;
+ break;
case LIRC_GET_REC_RESOLUTION:
if (!dev->rx_resolution)
- return -ENOTTY;
-
- val = dev->rx_resolution / 1000;
+ ret = -ENOTTY;
+ else
+ val = dev->rx_resolution / 1000;
break;
case LIRC_SET_WIDEBAND_RECEIVER:
if (!dev->s_learning_mode)
- return -ENOTTY;
-
- return dev->s_learning_mode(dev, !!val);
+ ret = -ENOTTY;
+ else
+ ret = dev->s_learning_mode(dev, !!val);
+ break;
case LIRC_SET_MEASURE_CARRIER_MODE:
if (!dev->s_carrier_report)
- return -ENOTTY;
-
- return dev->s_carrier_report(dev, !!val);
+ ret = -ENOTTY;
+ else
+ ret = dev->s_carrier_report(dev, !!val);
+ break;
/* Generic timeout support */
case LIRC_GET_MIN_TIMEOUT:
if (!dev->max_timeout)
- return -ENOTTY;
- val = DIV_ROUND_UP(dev->min_timeout, 1000);
+ ret = -ENOTTY;
+ else
+ val = DIV_ROUND_UP(dev->min_timeout, 1000);
break;
case LIRC_GET_MAX_TIMEOUT:
if (!dev->max_timeout)
- return -ENOTTY;
- val = dev->max_timeout / 1000;
+ ret = -ENOTTY;
+ else
+ val = dev->max_timeout / 1000;
break;
case LIRC_SET_REC_TIMEOUT:
- if (!dev->max_timeout)
- return -ENOTTY;
+ if (!dev->max_timeout) {
+ ret = -ENOTTY;
+ } else if (val > U32_MAX / 1000) {
+ /* Check for multiply overflow */
+ ret = -EINVAL;
+ } else {
+ u32 tmp = val * 1000;
- /* Check for multiply overflow */
- if (val > U32_MAX / 1000)
- return -EINVAL;
-
- tmp = val * 1000;
-
- if (tmp < dev->min_timeout || tmp > dev->max_timeout)
- return -EINVAL;
-
- if (dev->s_timeout)
- ret = dev->s_timeout(dev, tmp);
- if (!ret)
- dev->timeout = tmp;
+ if (tmp < dev->min_timeout || tmp > dev->max_timeout)
+ ret = -EINVAL;
+ else if (dev->s_timeout)
+ ret = dev->s_timeout(dev, tmp);
+ else if (!ret)
+ dev->timeout = tmp;
+ }
break;
case LIRC_SET_REC_TIMEOUT_REPORTS:
if (!dev->timeout)
- return -ENOTTY;
-
- fh->send_timeout_reports = !!val;
+ ret = -ENOTTY;
+ else
+ fh->send_timeout_reports = !!val;
break;
default:
- return -ENOTTY;
+ ret = -ENOTTY;
}
- if (_IOC_DIR(cmd) & _IOC_READ)
+ if (!ret && _IOC_DIR(cmd) & _IOC_READ)
ret = put_user(val, argp);
+out:
+ mutex_unlock(&dev->lock);
return ret;
}