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 0CB0DC282D7 for ; Mon, 11 Feb 2019 17:26:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C2656222A0 for ; Mon, 11 Feb 2019 17:26:31 +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="FuR8ZfSY" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730776AbfBKR0a (ORCPT ); Mon, 11 Feb 2019 12:26:30 -0500 Received: from mail-pf1-f195.google.com ([209.85.210.195]:39622 "EHLO mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730200AbfBKR03 (ORCPT ); Mon, 11 Feb 2019 12:26:29 -0500 Received: by mail-pf1-f195.google.com with SMTP id f132so5566010pfa.6 for ; Mon, 11 Feb 2019 09:26:28 -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=C46xbxNJBzjd5f1Aj+rliZC/YszczwAD3ovxXG/SoT4=; b=FuR8ZfSYcxEtB5MQYwWOwlpnG4ViYpcbtq2bG+0ISUY1gERZK0SLVPepehuwJq0Bhu cUYuEN/5Dw3DAGAOxgW6Ww/C5pp12PM455Z68VBZhmpecMkYkoHs30jafY4SADDBg9o7 9oJW/JIwxZQu7BfeEiumPt1WZP6qKzDa2MwSXfmXTF0vEGgbY641a1ifNEpsmA/HUvxU hEuApyF3WdROCzn3zhOjLyozAcEdvf53Wldug4vT3W1Ezg0mnINraW8Kt7FLDPBNgxTr WPQL0bSRds58L5+0BHIEJXAsVP/p9Q6apD7rME3NyJ+dmG3PTYLC5XOxwcEZDv3kEXPc 3vgQ== 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=C46xbxNJBzjd5f1Aj+rliZC/YszczwAD3ovxXG/SoT4=; b=cBl0dNv/1pqRDqYp27xMeFJ3PC5SOt8AA3rBFaG/sGNxF33xm5apoIFlX2b11gcIHl d77hEkIFo6RbCucnnFU0a90frMS7fZdeF8rrfsR+918QYqZcg2XAIhmYnolGGJB9Ep/L SU0PwsphhD9KiRuIFFD7Jx9WWp7gUisE31l0S6FAPw4NCwkFM/FrREKzUMat1HsBYVdT XJRnIPVtBnRbuRA8tFbIlHujdxXQmZh0JCsxgDomIdAVvhphrNWeW6+v/UA41P4rhdC4 KPwleTMPWG1YgSh+pVohcHWsbKFkRkZIGMRX5wahjXF+4YukysmpXszwokL9RQgfWABn uAtg== X-Gm-Message-State: AHQUAuZzL7gBMnuaYfRVCW2HhsQTJYlWP+ggKcpvNWk0RS2ynZJGfbAb NQtx6rSn/VRt1Z689M7ra3LYQQfK X-Google-Smtp-Source: AHgI3Ibt+Nt2b4DLEJ0K0vDscXoSLvO9+XvP6Qp902c2i0BFUC5KmSWf9YuNvUAsmDtGOsqWrU3YMw== X-Received: by 2002:a62:190e:: with SMTP id 14mr37683694pfz.70.1549905988367; Mon, 11 Feb 2019 09:26:28 -0800 (PST) Received: from localhost ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id s79sm15508617pgs.50.2019.02.11.09.26.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Feb 2019 09:26:27 -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.14.y] HID: debug: fix the ring buffer implementation Date: Mon, 11 Feb 2019 09:26:25 -0800 Message-Id: <1549905985-28911-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.14.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.14.y; backport to v4.9.y will follow. 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 ae8c8e66a6c4..b4de6656f65f 100644 --- a/drivers/hid/hid-debug.c +++ b/drivers/hid/hid-debug.c @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -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); @@ -1088,8 +1083,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; } @@ -1109,77 +1104,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; @@ -1189,8 +1164,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; @@ -1205,7 +1179,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; @@ -1256,4 +1230,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