[media] lirc_dev: fixes in lirc_dev_fop_read() (250f7a5f) · Commits · 戴 / test

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 "<fail>" 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 <error27@gmail.com> Reviewed-by:

Jarod Wilson <jarod@redhat.com> Acked-by:

Jarod Wilson <jarod@redhat.com> Signed-off-by:

Mauro Carvalho Chehab <mchehab@redhat.com>

drivers/media/IR/lirc_dev.c

+15 −10

Original line number	Diff line number	Diff line
		@@ -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);
		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 ? "<fail>" : "<ok>", ret);

		return ret ? ret : written;
		}

Admin message