FAILED: patch "[PATCH] HID: debug: fix the ring buffer implementation" failed to apply to 4.14-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 13054abbaa4f1fd4e6f3b4b63439ec033b4c8035 Mon Sep 17 00:00:00 2001
From: Vladis Dronov <[email protected]>
Date: Tue, 29 Jan 2019 11:58:35 +0100
Subject: [PATCH] HID: debug: fix the ring buffer implementation
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()
drivers/hid/hid-debug.c +47 -73
@@ -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 valuhid_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); } -
include/linux/hid-debug.h +4 -5
@@ -24,7 +24,10 @@#ifdef CONFIG_DEBUG_FS +#include <linux/kfifo.h> +#define HID_DEBUG_BUFSIZE 512 +#define HID_DEBUG_FIFOSIZE 512void 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 -