From 4957133fe32fec9a5fc57ce95b6107e976dcc251 Mon Sep 17 00:00:00 2001 From: Sean Young Date: Sat, 4 Nov 2017 08:30:45 -0400 Subject: 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 Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 239 +++++++++++++++++++++++++------------------- 1 file changed, 137 insertions(+), 102 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 7b9246fb9652..218658917cf6 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -233,15 +233,21 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf, 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 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf, 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 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf, * 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 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf, 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 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf, } out: + mutex_unlock(&dev->lock); kfree(txbuf); kfree(raw); return ret; @@ -358,8 +381,8 @@ static long ir_lirc_ioctl(struct file *file, unsigned int cmd, 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 @@ static long ir_lirc_ioctl(struct file *file, unsigned int cmd, 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 @@ static long ir_lirc_ioctl(struct file *file, unsigned int cmd, /* 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; - - /* 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 (!dev->max_timeout) { + ret = -ENOTTY; + } else if (val > U32_MAX / 1000) { + /* Check for multiply overflow */ + ret = -EINVAL; + } else { + u32 tmp = val * 1000; + + 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; } -- cgit v1.2.3-59-g8ed1b