From 250f7a5f62a08985af5cf7728ae7ba9edbfdc0a9 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 17 Nov 2010 02:20:15 -0300 Subject: [PATCH] [media] lirc_dev: fixes in lirc_dev_fop_read() This makes several changes but they're in one function and sort of related: "buf" was leaked on error. The leak if we try to read an invalid length is the main concern because it could be triggered over and over. If the copy_to_user() failed, then the original code returned the number of bytes remaining. read() is supposed to be the opposite way, where we return the number of bytes copied. I changed it to just return -EFAULT on errors. Also I changed the debug output from "-EFAULT" to just "" because it isn't -EFAULT necessarily. And since we go though that path if the length is invalid now, there was another debug print that I removed. Signed-off-by: Dan Carpenter Reviewed-by: Jarod Wilson Acked-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/IR/lirc_dev.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/media/IR/lirc_dev.c b/drivers/media/IR/lirc_dev.c index 8ab9d87f3a46..756656e17bdd 100644 --- a/drivers/media/IR/lirc_dev.c +++ b/drivers/media/IR/lirc_dev.c @@ -647,18 +647,18 @@ ssize_t lirc_dev_fop_read(struct file *file, if (!buf) return -ENOMEM; - if (mutex_lock_interruptible(&ir->irctl_lock)) - return -ERESTARTSYS; + if (mutex_lock_interruptible(&ir->irctl_lock)) { + ret = -ERESTARTSYS; + goto out_unlocked; + } if (!ir->attached) { - mutex_unlock(&ir->irctl_lock); - return -ENODEV; + ret = -ENODEV; + goto out_locked; } if (length % ir->chunk_size) { - dev_dbg(ir->d.dev, LOGHEAD "read result = -EINVAL\n", - ir->d.name, ir->d.minor); - mutex_unlock(&ir->irctl_lock); - return -EINVAL; + ret = -EINVAL; + goto out_locked; } /* @@ -709,18 +709,23 @@ ssize_t lirc_dev_fop_read(struct file *file, lirc_buffer_read(ir->buf, buf); ret = copy_to_user((void *)buffer+written, buf, ir->buf->chunk_size); - written += ir->buf->chunk_size; + if (!ret) + written += ir->buf->chunk_size; + else + ret = -EFAULT; } } remove_wait_queue(&ir->buf->wait_poll, &wait); set_current_state(TASK_RUNNING); + +out_locked: mutex_unlock(&ir->irctl_lock); out_unlocked: kfree(buf); dev_dbg(ir->d.dev, LOGHEAD "read result = %s (%d)\n", - ir->d.name, ir->d.minor, ret ? "-EFAULT" : "OK", ret); + ir->d.name, ir->d.minor, ret ? "" : "", ret); return ret ? ret : written; } -- 2.20.1