Commit 13054abb authored by Vladis Dronov's avatar Vladis Dronov Committed by Benjamin Tissoires
Browse files

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 717adfda
("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: cd667ce2 ("HID: use debugfs for events/reports dumping")
Fixes: 717adfda ("HID: debug: check length before copy_to_user()")
Signed-off-by: default avatarVladis Dronov <vdronov@redhat.com>
Reviewed-by: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarBenjamin Tissoires <benjamin.tissoires@redhat.com>
parent 1950f462

drivers/hid/hid-debug.c

+47 −73
Original line number Diff line number Diff line
@@ -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);
@@ -723,7 +719,6 @@ void hid_dump_input(struct hid_device *hdev, struct hid_usage *usage, __s32 valu 


	kfree(buf);

	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,25 +1099,30 @@ 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) {

	if (kfifo_is_empty(&list->hid_debug_fifo)) {

		add_wait_queue(&list->hdev->debug_wait, &wait);

		set_current_state(TASK_INTERRUPTIBLE);



			while (list->head == list->tail) {

		while (kfifo_is_empty(&list->hid_debug_fifo)) {

			if (file->f_flags & O_NONBLOCK) {

				ret = -EAGAIN;

				break;

			}



			if (signal_pending(current)) {

				ret = -ERESTARTSYS;

				break;

			}



			/* 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);
@@ -1136,45 +1136,20 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer, 
			set_current_state(TASK_INTERRUPTIBLE);

		}



			set_current_state(TASK_RUNNING);

		__set_current_state(TASK_RUNNING);

		remove_wait_queue(&list->hdev->debug_wait, &wait);

		}



		if (ret)

			goto out;



		/* pass the ringbuffer contents 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 (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
Original line number Diff line number Diff line
@@ -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

CRA Git | Maintained and supported by SUSTech CRA and CCSE