Skip to content

Commit d47e3da

Browse files
torvaldstorvalds
committed
Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid
Pull HID fix from Jiri Kosina: "A fix for a bug in hid-debug that can lock up the kernel in infinite loop (CVE-2019-3819), from Vladis Dronov" * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid: HID: debug: fix the ring buffer implementation
2 parents 6f64e3a + 13054ab commit d47e3da

File tree

2 files changed

+51
-78
lines changed

2 files changed

+51
-78
lines changed

‎drivers/hid/hid-debug.c

Lines changed: 47 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
#include <linux/debugfs.h>
3232
#include <linux/seq_file.h>
33+
#include <linux/kfifo.h>
3334
#include <linux/sched/signal.h>
3435
#include <linux/export.h>
3536
#include <linux/slab.h>
@@ -661,17 +662,12 @@ EXPORT_SYMBOL_GPL(hid_dump_device);
661662
/* enqueue string to 'events' ring buffer */
662663
void hid_debug_event(struct hid_device *hdev, char *buf)
663664
{
664-
unsigned i;
665665
struct hid_debug_list *list;
666666
unsigned long flags;
667667

668668
spin_lock_irqsave(&hdev->debug_list_lock, flags);
669-
list_for_each_entry(list, &hdev->debug_list, node) {
670-
for (i = 0; buf[i]; i++)
671-
list->hid_debug_buf[(list->tail + i) % HID_DEBUG_BUFSIZE] =
672-
buf[i];
673-
list->tail = (list->tail + i) % HID_DEBUG_BUFSIZE;
674-
}
669+
list_for_each_entry(list, &hdev->debug_list, node)
670+
kfifo_in(&list->hid_debug_fifo, buf, strlen(buf));
675671
spin_unlock_irqrestore(&hdev->debug_list_lock, flags);
676672

677673
wake_up_interruptible(&hdev->debug_wait);
@@ -722,8 +718,7 @@ void hid_dump_input(struct hid_device *hdev, struct hid_usage *usage, __s32 valu
722718
hid_debug_event(hdev, buf);
723719

724720
kfree(buf);
725-
wake_up_interruptible(&hdev->debug_wait);
726-
721+
wake_up_interruptible(&hdev->debug_wait);
727722
}
728723
EXPORT_SYMBOL_GPL(hid_dump_input);
729724

@@ -1083,8 +1078,8 @@ static int hid_debug_events_open(struct inode *inode, struct file *file)
10831078
goto out;
10841079
}
10851080

1086-
if (!(list->hid_debug_buf = kzalloc(HID_DEBUG_BUFSIZE, GFP_KERNEL))) {
1087-
err = -ENOMEM;
1081+
err = kfifo_alloc(&list->hid_debug_fifo, HID_DEBUG_FIFOSIZE, GFP_KERNEL);
1082+
if (err) {
10881083
kfree(list);
10891084
goto out;
10901085
}
@@ -1104,77 +1099,57 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
11041099
size_t count, loff_t *ppos)
11051100
{
11061101
struct hid_debug_list *list = file->private_data;
1107-
int ret = 0, len;
1102+
int ret = 0, copied;
11081103
DECLARE_WAITQUEUE(wait, current);
11091104

11101105
mutex_lock(&list->read_mutex);
1111-
while (ret == 0) {
1112-
if (list->head == list->tail) {
1113-
add_wait_queue(&list->hdev->debug_wait, &wait);
1114-
set_current_state(TASK_INTERRUPTIBLE);
1115-
1116-
while (list->head == list->tail) {
1117-
if (file->f_flags & O_NONBLOCK) {
1118-
ret = -EAGAIN;
1119-
break;
1120-
}
1121-
if (signal_pending(current)) {
1122-
ret = -ERESTARTSYS;
1123-
break;
1124-
}
1106+
if (kfifo_is_empty(&list->hid_debug_fifo)) {
1107+
add_wait_queue(&list->hdev->debug_wait, &wait);
1108+
set_current_state(TASK_INTERRUPTIBLE);
1109+
1110+
while (kfifo_is_empty(&list->hid_debug_fifo)) {
1111+
if (file->f_flags & O_NONBLOCK) {
1112+
ret = -EAGAIN;
1113+
break;
1114+
}
11251115

1126-
if (!list->hdev || !list->hdev->debug) {
1127-
ret = -EIO;
1128-
set_current_state(TASK_RUNNING);
1129-
goto out;
1130-
}
1116+
if (signal_pending(current)) {
1117+
ret = -ERESTARTSYS;
1118+
break;
1119+
}
11311120

1132-
/* allow O_NONBLOCK from other threads */
1133-
mutex_unlock(&list->read_mutex);
1134-
schedule();
1135-
mutex_lock(&list->read_mutex);
1136-
set_current_state(TASK_INTERRUPTIBLE);
1121+
/* if list->hdev is NULL we cannot remove_wait_queue().
1122+
* if list->hdev->debug is 0 then hid_debug_unregister()
1123+
* was already called and list->hdev is being destroyed.
1124+
* if we add remove_wait_queue() here we can hit a race.
1125+
*/
1126+
if (!list->hdev || !list->hdev->debug) {
1127+
ret = -EIO;
1128+
set_current_state(TASK_RUNNING);
1129+
goto out;
11371130
}
11381131

1139-
set_current_state(TASK_RUNNING);
1140-
remove_wait_queue(&list->hdev->debug_wait, &wait);
1132+
/* allow O_NONBLOCK from other threads */
1133+
mutex_unlock(&list->read_mutex);
1134+
schedule();
1135+
mutex_lock(&list->read_mutex);
1136+
set_current_state(TASK_INTERRUPTIBLE);
11411137
}
11421138

1143-
if (ret)
1144-
goto out;
1139+
__set_current_state(TASK_RUNNING);
1140+
remove_wait_queue(&list->hdev->debug_wait, &wait);
11451141

1146-
/* pass the ringbuffer contents to userspace */
1147-
copy_rest:
1148-
if (list->tail == list->head)
1142+
if (ret)
11491143
goto out;
1150-
if (list->tail > list->head) {
1151-
len = list->tail - list->head;
1152-
if (len > count)
1153-
len = count;
1154-
1155-
if (copy_to_user(buffer + ret, &list->hid_debug_buf[list->head], len)) {
1156-
ret = -EFAULT;
1157-
goto out;
1158-
}
1159-
ret += len;
1160-
list->head += len;
1161-
} else {
1162-
len = HID_DEBUG_BUFSIZE - list->head;
1163-
if (len > count)
1164-
len = count;
1165-
1166-
if (copy_to_user(buffer, &list->hid_debug_buf[list->head], len)) {
1167-
ret = -EFAULT;
1168-
goto out;
1169-
}
1170-
list->head = 0;
1171-
ret += len;
1172-
count -= len;
1173-
if (count > 0)
1174-
goto copy_rest;
1175-
}
1176-
11771144
}
1145+
1146+
/* pass the fifo content to userspace, locking is not needed with only
1147+
* one concurrent reader and one concurrent writer
1148+
*/
1149+
ret = kfifo_to_user(&list->hid_debug_fifo, buffer, count, &copied);
1150+
if (ret)
1151+
goto out;
1152+
ret = copied;
11781153
out:
11791154
mutex_unlock(&list->read_mutex);
11801155
return ret;
@@ -1185,7 +1160,7 @@ static __poll_t hid_debug_events_poll(struct file *file, poll_table *wait)
11851160
struct hid_debug_list *list = file->private_data;
11861161

11871162
poll_wait(file, &list->hdev->debug_wait, wait);
1188-
if (list->head != list->tail)
1163+
if (!kfifo_is_empty(&list->hid_debug_fifo))
11891164
return EPOLLIN | EPOLLRDNORM;
11901165
if (!list->hdev->debug)
11911166
return EPOLLERR | EPOLLHUP;
@@ -1200,7 +1175,7 @@ static int hid_debug_events_release(struct inode *inode, struct file *file)
12001175
spin_lock_irqsave(&list->hdev->debug_list_lock, flags);
12011176
list_del(&list->node);
12021177
spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags);
1203-
kfree(list->hid_debug_buf);
1178+
kfifo_free(&list->hid_debug_fifo);
12041179
kfree(list);
12051180

12061181
return 0;
@@ -1246,4 +1221,3 @@ void hid_debug_exit(void)
12461221
{
12471222
debugfs_remove_recursive(hid_debug_root);
12481223
}
1249-

‎include/linux/hid-debug.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@
2424

2525
#ifdef CONFIG_DEBUG_FS
2626

27+
#include <linux/kfifo.h>
28+
2729
#define HID_DEBUG_BUFSIZE 512
30+
#define HID_DEBUG_FIFOSIZE 512
2831

2932
void hid_dump_input(struct hid_device *, struct hid_usage *, __s32);
3033
void hid_dump_report(struct hid_device *, int , u8 *, int);
@@ -37,11 +40,8 @@ void hid_debug_init(void);
3740
void hid_debug_exit(void);
3841
void hid_debug_event(struct hid_device *, char *);
3942

40-
4143
struct hid_debug_list {
42-
char *hid_debug_buf;
43-
int head;
44-
int tail;
44+
DECLARE_KFIFO_PTR(hid_debug_fifo, char);
4545
struct fasync_struct *fasync;
4646
struct hid_device *hdev;
4747
struct list_head node;
@@ -64,4 +64,3 @@ struct hid_debug_list {
6464
#endif
6565

6666
#endif
67-

0 commit comments

Comments
 (0)