HID: debug: fix the ring buffer implementation (CVE-2019-3819)
This commit is contained in:
parent
9c88b474fe
commit
5019a8394c
|
@ -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
|
||||
|
|
259
debian/patches/bugfix/all/HID-debug-fix-the-ring-buffer-implementation.patch
vendored
Normal file
259
debian/patches/bugfix/all/HID-debug-fix-the-ring-buffer-implementation.patch
vendored
Normal file
|
@ -0,0 +1,259 @@
|
|||
From: Vladis Dronov <vdronov@redhat.com>
|
||||
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 <vdronov@redhat.com>
|
||||
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
|
||||
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
|
||||
---
|
||||
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 <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);
|
||||
|
||||
@@ -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 <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.11.0
|
||||
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue