From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA9F2C169C4 for ; Mon, 11 Feb 2019 17:30:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 914322229E for ; Mon, 11 Feb 2019 17:30:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Aw2yVkaD" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729984AbfBKRao (ORCPT ); Mon, 11 Feb 2019 12:30:44 -0500 Received: from mail-pl1-f194.google.com ([209.85.214.194]:39630 "EHLO mail-pl1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728741AbfBKRao (ORCPT ); Mon, 11 Feb 2019 12:30:44 -0500 Received: by mail-pl1-f194.google.com with SMTP id 101so5606379pld.6 for ; Mon, 11 Feb 2019 09:30:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id; bh=Dulzz2TGZcOB5rocTEAFYduDI4eeORcRXvhspJP9cw0=; b=Aw2yVkaDbiKsw3xZ0rSblZSwB8ZQ4MvrO+SAlrvLXHhGJKcNR/bthJ8TBnQJBVMzbu z9Ja2EBSZYyuZqwogre25OfThXWm1t2J6tStxzKCXVIN/t0yVZrPLYVr8/c0QhyMMDzn c08V66TkAXMu14fMN51ffb8stn1g4E4TuDJp+sfwcXSpiK9nU5W5Qb8m8/+rYCyKtfud J5aZ8ZjvnMgzgLz7feRbtoLO0P1Q7txE8W7DFT9Oz7u2J6NZNdLE6qKwzoHHMxAjwPHQ QXT/6ROgHdHOIUZKiw/aC0L5mZCziQ3cCYg59SYk6E09mJLzuimmRmdbK0tTKoiAS3uB eEUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id; bh=Dulzz2TGZcOB5rocTEAFYduDI4eeORcRXvhspJP9cw0=; b=E05Ci7isQyPg35XMg4X8E7dG1rhCKCTbXx8pAbpjpbHSbQGunBrbsydXXjA9fvD0ep LER2kt3KfzSjJjMdeik+3Qx2WPMlDO1r1pMs0wNQmwPgz43ntc04kA/SIOY8M8k1g93Y oOLbp3hHTZ5oiHlVCJFeL5lLpxClMpo0ONg0tV6qiB5MqpuMq4dvG7YaMgiZJWa1RZFB fi8X6ZSjA/PZpxAgch53qcnzmT1mGMT1jP/bCZlwQTk1TGKU7YbkiXOvAkwEn29RUOZs XGLGuJumuCOCk3tz6OWkROtK+qbsj6FaIIiQ+NRVABaB2u2n++8RVnakO251WJTRt2av mBsA== X-Gm-Message-State: AHQUAua9EatJh/57kcAdNNvYMmirTgStIiKxuII9ktlqJbZXyO0eWwTg CTq2M2bCBzQBjZHMsZboeGI= X-Google-Smtp-Source: AHgI3IYXAOFl2olLKLy6rrzhJFDEE3TTVoPK6WY4rrpXxRL69YoJ4N26ZDiAIbEF9CMSBuS//cMx1w== X-Received: by 2002:a17:902:b118:: with SMTP id q24mr38892536plr.209.1549906243417; Mon, 11 Feb 2019 09:30:43 -0800 (PST) Received: from localhost ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id 15sm17323269pfs.113.2019.02.11.09.30.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Feb 2019 09:30:42 -0800 (PST) From: Guenter Roeck To: Greg Kroah-Hartman Cc: stable@vger.kernel.org, Vladis Dronov , Oleg Nesterov , Benjamin Tissoires , Guenter Roeck Subject: [PATCH v4.9.y] HID: debug: fix the ring buffer implementation Date: Mon, 11 Feb 2019 09:30:28 -0800 Message-Id: <1549906228-26949-1-git-send-email-linux@roeck-us.net> X-Mailer: git-send-email 2.7.4 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Vladis Dronov commit 13054abbaa4f1fd4e6f3b4b63439ec033b4c8035 upstream. 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 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 Signed-off-by: Greg Kroah-Hartman [groeck: backport to v4.9.y] Signed-off-by: Guenter Roeck --- This patch is marked v4.18+, but commit 717adfdaf147 is marked for stable and found its way into all stable releases. Therefore, this patch is needed in older stable releases as well. This patch only applies to v4.9.y. Note: The offending patch is in v4.4.y as well, but trying to apply the fix to v4.4.y was messy, so I gave up on it. Copying patch author and reviewers to make sure I didn't miss anything. drivers/hid/hid-debug.c | 121 ++++++++++++++++++---------------------------- include/linux/hid-debug.h | 9 ++-- 2 files changed, 51 insertions(+), 79 deletions(-) diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index 29423691c105..07833e4add78 100644 --- a/drivers/hid/hid-debug.c +++ b/drivers/hid/hid-debug.c @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -659,17 +660,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); @@ -720,8 +716,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); @@ -1086,8 +1081,8 @@ static int hid_debug_events_open(struct inode *inode, struct file *file) goto out; } - if (!(list->hid_debug_buf = kzalloc(sizeof(char) * 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; } @@ -1107,77 +1102,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; @@ -1187,8 +1162,7 @@ static unsigned int 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 POLLIN | POLLRDNORM; if (!list->hdev->debug) return POLLERR | POLLHUP; @@ -1203,7 +1177,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; @@ -1254,4 +1228,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.7.4