hid_debug_events_read() does not properly handle the case when tail < head
and count < HID_DEBUG_BUFSIZE - head and returns corrupted data. Also,
after commit 717adfdaf147 ("HID: debug: check length before
copy_to_user()") it can enter an infinite loop if called with count == 0.
Fix this by properly handling this case and adding a check.
Also, the function has "while (ret == 0)" loop which is not needed, remove
it.
Signed-off-by: Vladis Dronov <vdronov@xxxxxxxxxx>
---
drivers/hid/hid-debug.c | 109 ++++++++++++++++++++++++----------------
1 file changed, 65 insertions(+), 44 deletions(-)
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index b48100236df8..20580871b0ec 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -722,7 +722,7 @@ void hid_dump_input(struct hid_device *hdev, struct hid_usage *usage, __s32 valu
hid_debug_event(hdev, buf);
kfree(buf);
- wake_up_interruptible(&hdev->debug_wait);
+ wake_up_interruptible(&hdev->debug_wait);
}
EXPORT_SYMBOL_GPL(hid_dump_input);
@@ -1112,73 +1112,95 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
int ret = 0, len;
DECLARE_WAITQUEUE(wait, current);
- mutex_lock(&list->read_mutex);
- while (ret == 0) {
- if (list->head == list->tail) {
- add_wait_queue(&list->hdev->debug_wait, &wait);
- set_current_state(TASK_INTERRUPTIBLE);
+ if (!count)
+ return 0;
- while (list->head == list->tail) {
- if (file->f_flags & O_NONBLOCK) {
- ret = -EAGAIN;
- break;
- }
- if (signal_pending(current)) {
- ret = -ERESTARTSYS;
- break;
- }
+ mutex_lock(&list->read_mutex);
+ if (list->head == list->tail) {
+ add_wait_queue(&list->hdev->debug_wait, &wait);
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ while (list->head == list->tail) {
+ if (file->f_flags & O_NONBLOCK) {
+ ret = -EAGAIN;
+ break;
+ }
- if (!list->hdev || !list->hdev->debug) {
- ret = -EIO;
- set_current_state(TASK_RUNNING);
- goto out;
- }
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
+ }
- /* allow O_NONBLOCK from other threads */
- mutex_unlock(&list->read_mutex);
- schedule();
- mutex_lock(&list->read_mutex);
- set_current_state(TASK_INTERRUPTIBLE);
+ if (!list->hdev || !list->hdev->debug) {
+ ret = -EIO;
+ set_current_state(TASK_RUNNING);
+ goto out;
}
- set_current_state(TASK_RUNNING);
- remove_wait_queue(&list->hdev->debug_wait, &wait);
+ /* allow O_NONBLOCK from other threads */
+ mutex_unlock(&list->read_mutex);
+ schedule();
+ mutex_lock(&list->read_mutex);
+ set_current_state(TASK_INTERRUPTIBLE);
}
- if (ret)
- goto out;
+ set_current_state(TASK_RUNNING);
+ remove_wait_queue(&list->hdev->debug_wait, &wait);
+ }
+
+ if (ret)
+ goto out;
- /* pass the ringbuffer contents to userspace */
+ /* pass the ringbuffer content to userspace */
copy_rest:
- if (list->tail == list->head)
- goto out;
- if (list->tail > list->head) {
- len = list->tail - list->head;
- if (len > count)
- len = count;
+ if (list->tail == list->head)
+ goto out;
+
+ /* the data from the head to the tail in the buffer is linear */
+ if (list->tail > list->head) {
+ len = list->tail - list->head;
+ if (len > count)
+ len = count;
- if (copy_to_user(buffer + ret, &list->hid_debug_buf[list->head], len)) {
+ if (copy_to_user(buffer + ret,
+ &list->hid_debug_buf[list->head], len)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ ret += len;
+ list->head += len;
+ } else {
+ len = HID_DEBUG_BUFSIZE - list->head;
+
+ /* the data is split in 2 pieces: from the head to the end of
+ * the ring buffer and from the start of the ring buffer to the
+ * tail. check if we need to copy out only the part between the
+ * head and the end.
+ */
+ if (len > count) {
+ len = count;
+
+ if (copy_to_user(buffer,
+ &list->hid_debug_buf[list->head], len)) {
ret = -EFAULT;
goto out;
}
ret += len;
list->head += len;
} else {
- len = HID_DEBUG_BUFSIZE - list->head;
- if (len > count)
- len = count;
-
- if (copy_to_user(buffer, &list->hid_debug_buf[list->head], len)) {
+ if (copy_to_user(buffer,
+ &list->hid_debug_buf[list->head], len)) {
ret = -EFAULT;
goto out;
}
list->head = 0;
ret += len;
count -= len;
+
+ /* check for a corner case when len == count */
if (count > 0)
goto copy_rest;
}
-
}
out:
mutex_unlock(&list->read_mutex);
@@ -1256,4 +1278,3 @@ void hid_debug_exit(void)
{
debugfs_remove_recursive(hid_debug_root);
}
-
--
2.19.0
