From 5019a8394c1c36bbceb45b4e790ebf0fda2d87b8 Mon Sep 17 00:00:00 2001 From: Salvatore Bonaccorso Date: Sat, 9 Feb 2019 15:10:48 +0100 Subject: [PATCH] HID: debug: fix the ring buffer implementation (CVE-2019-3819) --- debian/changelog | 1 + ...g-fix-the-ring-buffer-implementation.patch | 259 ++++++++++++++++++ debian/patches/series | 1 + 3 files changed, 261 insertions(+) create mode 100644 debian/patches/bugfix/all/HID-debug-fix-the-ring-buffer-implementation.patch diff --git a/debian/changelog b/debian/changelog index a56afbb05..7c9b02fa2 100644 --- a/debian/changelog +++ b/debian/changelog @@ -407,6 +407,7 @@ linux (4.19.20-1) UNRELEASED; urgency=medium (CVE-2019-7222) * [x86] KVM: nVMX: unconditionally cancel preemption timer in free_nested (CVE-2019-7221) + * HID: debug: fix the ring buffer implementation (CVE-2019-3819) [ Hideki Yamane ] * [x86] Enable Touchpad support on Gemini Lake via CONFIG_PINCTRL_GEMINILAKE diff --git a/debian/patches/bugfix/all/HID-debug-fix-the-ring-buffer-implementation.patch b/debian/patches/bugfix/all/HID-debug-fix-the-ring-buffer-implementation.patch new file mode 100644 index 000000000..5c7a0b408 --- /dev/null +++ b/debian/patches/bugfix/all/HID-debug-fix-the-ring-buffer-implementation.patch @@ -0,0 +1,259 @@ +From: Vladis Dronov +Date: Tue, 29 Jan 2019 11:58:35 +0100 +Subject: HID: debug: fix the ring buffer implementation +Origin: https://git.kernel.org/linus/13054abbaa4f1fd4e6f3b4b63439ec033b4c8035 +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2019-3819 + +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 +Reviewed-by: Oleg Nesterov +Signed-off-by: Benjamin Tissoires +--- + 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 c530476edba6..ac9fda1b5a72 100644 +--- a/drivers/hid/hid-debug.c ++++ b/drivers/hid/hid-debug.c +@@ -30,6 +30,7 @@ + + #include + #include ++#include + #include + #include + #include +@@ -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); + +@@ -1083,8 +1078,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; + } +@@ -1104,77 +1099,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; +@@ -1185,7 +1160,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; +@@ -1200,7 +1175,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; +@@ -1246,4 +1221,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 ++ + #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.11.0 + diff --git a/debian/patches/series b/debian/patches/series index e16ef5e09..65da4c754 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -142,6 +142,7 @@ debian/i386-686-pae-pci-set-pci-nobios-by-default.patch bugfix/all/kvm-fix-kvm_ioctl_create_device-reference-counting-C.patch bugfix/x86/KVM-x86-work-around-leak-of-uninitialized-stack-cont.patch bugfix/x86/KVM-nVMX-unconditionally-cancel-preemption-timer-in-.patch +bugfix/all/HID-debug-fix-the-ring-buffer-implementation.patch # Fix exported symbol versions bugfix/all/module-disable-matching-missing-version-crc.patch