Home Home > GIT Browse > stable
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJiri Slaby <jslaby@suse.cz>2019-02-12 22:18:26 +0100
committerJiri Slaby <jslaby@suse.cz>2019-02-12 22:21:20 +0100
commitebdc530edd06737819e4912fc1854d0d20413766 (patch)
tree16a07efc138218ad735f519a4ac661fb07e4d7eb
parentc0245fb67d9e3e5ead14e79de210ffd1f80ead0c (diff)
HID: debug: fix the ring buffer implementation (bnc#1012628).
-rw-r--r--patches.kernel.org/4.20.8-315-HID-debug-fix-the-ring-buffer-implementation.patch264
-rw-r--r--series.conf1
2 files changed, 265 insertions, 0 deletions
diff --git a/patches.kernel.org/4.20.8-315-HID-debug-fix-the-ring-buffer-implementation.patch b/patches.kernel.org/4.20.8-315-HID-debug-fix-the-ring-buffer-implementation.patch
new file mode 100644
index 0000000000..537fc2d8d5
--- /dev/null
+++ b/patches.kernel.org/4.20.8-315-HID-debug-fix-the-ring-buffer-implementation.patch
@@ -0,0 +1,264 @@
+From: Vladis Dronov <vdronov@redhat.com>
+Date: Tue, 29 Jan 2019 11:58:35 +0100
+Subject: [PATCH] HID: debug: fix the ring buffer implementation
+References: bnc#1012628
+Patch-mainline: 4.20.8
+Git-commit: 13054abbaa4f1fd4e6f3b4b63439ec033b4c8035
+
+commit 13054abbaa4f1fd4e6f3b4b63439ec033b4c8035 upstream.
+
+Ring buffer implementation in hid_debug_event() and hid_debug_events_read()
+is strange allowing lost or corrupted data. After commit 717adfdaf147
+("HID: debug: check length before copy_to_user()") it is possible to enter
+an infinite loop in hid_debug_events_read() by providing 0 as count, this
+locks up a system. Fix this by rewriting the ring buffer implementation
+with kfifo and simplify the code.
+
+This fixes CVE-2019-3819.
+
+v2: fix an execution logic and add a comment
+v3: use __set_current_state() instead of set_current_state()
+
+Link: https://bugzilla.redhat.com/show_bug.cgi?id=1669187
+Cc: stable@vger.kernel.org # v4.18+
+Fixes: cd667ce24796 ("HID: use debugfs for events/reports dumping")
+Fixes: 717adfdaf147 ("HID: debug: check length before copy_to_user()")
+Signed-off-by: Vladis Dronov <vdronov@redhat.com>
+Reviewed-by: Oleg Nesterov <oleg@redhat.com>
+Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Signed-off-by: Jiri Slaby <jslaby@suse.cz>
+---
+ drivers/hid/hid-debug.c | 120 +++++++++++++++-----------------------
+ include/linux/hid-debug.h | 9 ++-
+ 2 files changed, 51 insertions(+), 78 deletions(-)
+
+diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
+index b48100236df8..ebc9ffde41e9 100644
+--- a/drivers/hid/hid-debug.c
++++ b/drivers/hid/hid-debug.c
+@@ -30,6 +30,7 @@
+
+ #include <linux/debugfs.h>
+ #include <linux/seq_file.h>
++#include <linux/kfifo.h>
+ #include <linux/sched/signal.h>
+ #include <linux/export.h>
+ #include <linux/slab.h>
+@@ -661,17 +662,12 @@ EXPORT_SYMBOL_GPL(hid_dump_device);
+ /* enqueue string to 'events' ring buffer */
+ void hid_debug_event(struct hid_device *hdev, char *buf)
+ {
+- unsigned i;
+ struct hid_debug_list *list;
+ unsigned long flags;
+
+ spin_lock_irqsave(&hdev->debug_list_lock, flags);
+- list_for_each_entry(list, &hdev->debug_list, node) {
+- for (i = 0; buf[i]; i++)
+- list->hid_debug_buf[(list->tail + i) % HID_DEBUG_BUFSIZE] =
+- buf[i];
+- list->tail = (list->tail + i) % HID_DEBUG_BUFSIZE;
+- }
++ list_for_each_entry(list, &hdev->debug_list, node)
++ kfifo_in(&list->hid_debug_fifo, buf, strlen(buf));
+ spin_unlock_irqrestore(&hdev->debug_list_lock, flags);
+
+ wake_up_interruptible(&hdev->debug_wait);
+@@ -722,8 +718,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);
+
+@@ -1088,8 +1083,8 @@ static int hid_debug_events_open(struct inode *inode, struct file *file)
+ goto out;
+ }
+
+- if (!(list->hid_debug_buf = kzalloc(HID_DEBUG_BUFSIZE, GFP_KERNEL))) {
+- err = -ENOMEM;
++ err = kfifo_alloc(&list->hid_debug_fifo, HID_DEBUG_FIFOSIZE, GFP_KERNEL);
++ if (err) {
+ kfree(list);
+ goto out;
+ }
+@@ -1109,77 +1104,57 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
+ size_t count, loff_t *ppos)
+ {
+ struct hid_debug_list *list = file->private_data;
+- int ret = 0, len;
++ int ret = 0, copied;
+ 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);
+-
+- while (list->head == list->tail) {
+- if (file->f_flags & O_NONBLOCK) {
+- ret = -EAGAIN;
+- break;
+- }
+- if (signal_pending(current)) {
+- ret = -ERESTARTSYS;
+- break;
+- }
++ if (kfifo_is_empty(&list->hid_debug_fifo)) {
++ add_wait_queue(&list->hdev->debug_wait, &wait);
++ set_current_state(TASK_INTERRUPTIBLE);
++
++ while (kfifo_is_empty(&list->hid_debug_fifo)) {
++ 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 is NULL we cannot remove_wait_queue().
++ * if list->hdev->debug is 0 then hid_debug_unregister()
++ * was already called and list->hdev is being destroyed.
++ * if we add remove_wait_queue() here we can hit a race.
++ */
++ 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);
+
+- /* pass the ringbuffer contents to userspace */
+-copy_rest:
+- if (list->tail == list->head)
++ if (ret)
+ goto out;
+- 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)) {
+- 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)) {
+- ret = -EFAULT;
+- goto out;
+- }
+- list->head = 0;
+- ret += len;
+- count -= len;
+- if (count > 0)
+- goto copy_rest;
+- }
+-
+ }
++
++ /* pass the fifo content to userspace, locking is not needed with only
++ * one concurrent reader and one concurrent writer
++ */
++ ret = kfifo_to_user(&list->hid_debug_fifo, buffer, count, &copied);
++ if (ret)
++ goto out;
++ ret = copied;
+ out:
+ mutex_unlock(&list->read_mutex);
+ return ret;
+@@ -1190,7 +1165,7 @@ static __poll_t hid_debug_events_poll(struct file *file, poll_table *wait)
+ struct hid_debug_list *list = file->private_data;
+
+ poll_wait(file, &list->hdev->debug_wait, wait);
+- if (list->head != list->tail)
++ if (!kfifo_is_empty(&list->hid_debug_fifo))
+ return EPOLLIN | EPOLLRDNORM;
+ if (!list->hdev->debug)
+ return EPOLLERR | EPOLLHUP;
+@@ -1205,7 +1180,7 @@ static int hid_debug_events_release(struct inode *inode, struct file *file)
+ spin_lock_irqsave(&list->hdev->debug_list_lock, flags);
+ list_del(&list->node);
+ spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags);
+- kfree(list->hid_debug_buf);
++ kfifo_free(&list->hid_debug_fifo);
+ kfree(list);
+
+ return 0;
+@@ -1256,4 +1231,3 @@ void hid_debug_exit(void)
+ {
+ debugfs_remove_recursive(hid_debug_root);
+ }
+-
+diff --git a/include/linux/hid-debug.h b/include/linux/hid-debug.h
+index 8663f216c563..2d6100edf204 100644
+--- a/include/linux/hid-debug.h
++++ b/include/linux/hid-debug.h
+@@ -24,7 +24,10 @@
+
+ #ifdef CONFIG_DEBUG_FS
+
++#include <linux/kfifo.h>
++
+ #define HID_DEBUG_BUFSIZE 512
++#define HID_DEBUG_FIFOSIZE 512
+
+ void hid_dump_input(struct hid_device *, struct hid_usage *, __s32);
+ void hid_dump_report(struct hid_device *, int , u8 *, int);
+@@ -37,11 +40,8 @@ void hid_debug_init(void);
+ void hid_debug_exit(void);
+ void hid_debug_event(struct hid_device *, char *);
+
+-
+ struct hid_debug_list {
+- char *hid_debug_buf;
+- int head;
+- int tail;
++ DECLARE_KFIFO_PTR(hid_debug_fifo, char);
+ struct fasync_struct *fasync;
+ struct hid_device *hdev;
+ struct list_head node;
+@@ -64,4 +64,3 @@ struct hid_debug_list {
+ #endif
+
+ #endif
+-
+--
+2.20.1
+
diff --git a/series.conf b/series.conf
index 67845b5f23..f07e290432 100644
--- a/series.conf
+++ b/series.conf
@@ -1051,6 +1051,7 @@
patches.kernel.org/4.20.8-312-fuse-decrement-NR_WRITEBACK_TEMP-on-the-right-.patch
patches.kernel.org/4.20.8-313-fuse-handle-zero-sized-retrieve-correctly.patch
patches.kernel.org/4.20.8-314-cuse-fix-ioctl.patch
+ patches.kernel.org/4.20.8-315-HID-debug-fix-the-ring-buffer-implementation.patch
########################################################
# Build fixes that apply to the vanilla kernel too.