[PATCH v3] HID: debug: fix the ring buffer implementation

[PATCH v3] HID: debug: fix the ring buffer implementation

[Vladis Dronov]

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
Cc: stable@vger.kernel.org # v4.18+
Fixes: cd667ce24796 ("HID: use debugfs for events/reports dumping")
Fixes: 717adfdaf147 ("HID: debug: check length before copy_to_user()")
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
---
 drivers/hid/hid-debug.c   | 116 ++++++++++++++------------------------
 include/linux/hid-debug.h |   9 ++-
 2 files changed, 47 insertions(+), 78 deletions(-)

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index c530476edba6..08870c909268 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -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 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);
 
@@ -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 (!list->hdev || !list->hdev->debug) {
-					ret = -EIO;
-					set_current_state(TASK_RUNNING);
-					goto out;
-				}
-
-				/* allow O_NONBLOCK from other threads */
-				mutex_unlock(&list->read_mutex);
-				schedule();
-				mutex_lock(&list->read_mutex);
-				set_current_state(TASK_INTERRUPTIBLE);
-			}
-
-			set_current_state(TASK_RUNNING);
-			remove_wait_queue(&list->hdev->debug_wait, &wait);
-		}
-
-		if (ret)
-			goto out;
+	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 (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);
+				goto out;
+			}
+
+			/* allow O_NONBLOCK from other threads */
+			mutex_unlock(&list->read_mutex);
+			schedule();
+			mutex_lock(&list->read_mutex);
+			set_current_state(TASK_INTERRUPTIBLE);
+		}
+
+		__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);
 }
-
diff --git a/include/linux/hid-debug.h b/include/linux/hid-debug.h
index 8663f216c563..e7a7c92aaf09 100644
--- a/include/linux/hid-debug.h
+++ b/include/linux/hid-debug.h
@@ -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);
@@ -38,10 +41,7 @@ void hid_debug_event(struct hid_device *, char *);
 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
-

Re: [PATCH v3] HID: debug: fix the ring buffer implementation

[Benjamin Tissoires]

On Tue, Jan 29, 2019 at 11:58 AM Vladis Dronov <vdronov@redhat.com> wrote:
>
> 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
> Cc: stable@vger.kernel.org # v4.18+
> Fixes: cd667ce24796 ("HID: use debugfs for events/reports dumping")
> Fixes: 717adfdaf147 ("HID: debug: check length before copy_to_user()")
> Signed-off-by: Vladis Dronov <vdronov@redhat.com>
> ---

Thanks for the quick v3.

I have now applied this to for-5.0/upstream-fixes with Oleg's rev-by.

Cheers,
Benjamin

>  drivers/hid/hid-debug.c   | 116 ++++++++++++++------------------------
>  include/linux/hid-debug.h |   9 ++-
>  2 files changed, 47 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index c530476edba6..08870c909268 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -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 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);
>
> @@ -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 (!list->hdev || !list->hdev->debug) {
> -                                       ret = -EIO;
> -                                       set_current_state(TASK_RUNNING);
> -                                       goto out;
> -                               }
> -
> -                               /* allow O_NONBLOCK from other threads */
> -                               mutex_unlock(&list->read_mutex);
> -                               schedule();
> -                               mutex_lock(&list->read_mutex);
> -                               set_current_state(TASK_INTERRUPTIBLE);
> -                       }
> -
> -                       set_current_state(TASK_RUNNING);
> -                       remove_wait_queue(&list->hdev->debug_wait, &wait);
> -               }
> -
> -               if (ret)
> -                       goto out;
> +       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 (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);
> +                               goto out;
> +                       }
> +
> +                       /* allow O_NONBLOCK from other threads */
> +                       mutex_unlock(&list->read_mutex);
> +                       schedule();
> +                       mutex_lock(&list->read_mutex);
> +                       set_current_state(TASK_INTERRUPTIBLE);
> +               }
> +
> +               __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);
>  }
> -
> diff --git a/include/linux/hid-debug.h b/include/linux/hid-debug.h
> index 8663f216c563..e7a7c92aaf09 100644
> --- a/include/linux/hid-debug.h
> +++ b/include/linux/hid-debug.h
> @@ -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);
> @@ -38,10 +41,7 @@ void hid_debug_event(struct hid_device *, char *);
>  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
> -

Re: [PATCH v3] HID: debug: fix the ring buffer implementation

[Jiri Kosina]

On Wed, 30 Jan 2019, Sasha Levin wrote:

> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: cd667ce24796 HID: use debugfs for events/reports dumping.
> 
> The bot has tested the following trees: v4.20.5, v4.19.18, v4.14.96, v4.9.153, v4.4.172, v3.18.133.
> 
> v4.20.5: Build OK!
> v4.19.18: Build OK!
> v4.14.96: Failed to apply! Possible dependencies:
>     0eecc636e5a2 ("bus: ti-sysc: Add minimal TI sysc interconnect target driver")
>     49a0a3d805df ("bus: ti-sysc: Make omap_hwmod_sysc_fields into sysc_regbits platform data")
>     566a9b05e1fa ("bus: ti-sysc: Handle module quirks based dts configuration")
>     6396bb221514 ("treewide: kzalloc() -> kcalloc()")
>     695eea3d2c7f ("ARM: OMAP2+: Enable ti-sysc to use device tree data for smartreflex")
>     70a65240efb1 ("bus: ti-sysc: Add register bits for interconnect target modules")
>     a7199e2b91de ("bus: ti-sysc: Detect i2c interconnect target module based on register layout")
>     bf8070522298 ("ARM: OMAP2+: Move all omap_hwmod_sysc_fields to omap_hwmod_common_data.c")
>     c5a2de97fbd2 ("bus: ti-sysc: Add parsing of module capabilities")
>     d060b40523dc ("ARM: OMAP2+: Prepare to pass auxdata for smartreflex")
>     ef70b0bdeaf8 ("bus: ti-sysc: Add support for platform data callbacks")
> 
> v4.9.153: Failed to apply! Possible dependencies:
>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
>     1cec20f0ea0e ("dma-buf: Restart reservation_object_wait_timeout_rcu() after writes")
>     555570d744f8 ("sched/clock: Update static_key usage")
>     78010cd9736e ("dma-buf/fence: add an lockdep_assert_held()")
>     983de5f97169 ("firmware: tegra: Add BPMP support")
>     9881b024b7d7 ("sched/clock: Delay switching sched_clock to stable")
>     acb04058de49 ("sched/clock: Fix hotplug crash")
>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
>     b52992c06c90 ("drm/i915: Support asynchronous waits on struct fence from i915_gem_request")
>     ca791d7f4256 ("firmware: tegra: Add IVC library")
>     e601757102cf ("sched/headers: Prepare for new header dependencies before moving code to <linux/sched/clock.h>")
>     ea8b1c4a6019 ("drivers: psci: PSCI checker module")
>     f54d1867005c ("dma-buf: Rename struct fence to dma_fence")
>     fedf54132d24 ("dma-buf: Restart reservation_object_get_fences_rcu() after writes")
> 
> v4.4.172: Failed to apply! Possible dependencies:
>     0529900a01cb ("crypto: omap-aes - Support crypto engine framework")
>     0bc40be85f33 ("drm/i915: Rename intel_engine_cs function parameters")
>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
>     2589ad84047f ("crypto: engine - move crypto engine to its own header")
>     4cba7cf025f3 ("crypto: engine - permit to enqueue ashash_request")
>     735d37b5424b ("crypto: engine - Introduce the block request crypto engine framework")
>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
>     c81d46138da6 ("drm/i915: Convert trace-irq to the breadcrumb waiter")
>     ca82580c9cea ("drm/i915: Do not call API requiring struct_mutex where it is not available")
>     cbdc12a9fc9d ("drm/i915: make A0 wa's applied to A1")
>     e87a005d90c3 ("drm/i915: add helpers for platform specific revision id range checks")
>     ea8b1c4a6019 ("drivers: psci: PSCI checker module")
>     ef712bb4b700 ("drm/i915: remove parens around revision ids")
>     f1b77aaca85a ("crypto: omap-des - Integrate with the crypto engine framework")
>     fffda3f4fb49 ("drm/i915/bxt: add revision id for A1 stepping and use it")
> 
> v3.18.133: Failed to apply! Possible dependencies:
>     00aa37206e1a ("of/reconfig: Add debug output for OF_RECONFIG notifiers")
>     0529900a01cb ("crypto: omap-aes - Support crypto engine framework")
>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
>     2589ad84047f ("crypto: engine - move crypto engine to its own header")
>     310b0d55f030 ("crypto: omap-aes - Fix CTR mode")
>     4cba7cf025f3 ("crypto: engine - permit to enqueue ashash_request")
>     596103cf8fb0 ("crypto: drivers - Fix Kconfig selects")
>     6c5063434098 ("crypto: ccp - Add ACPI support")
>     7011a122383e ("crypto: nx - add NX-842 platform frontend driver")
>     735d37b5424b ("crypto: engine - Introduce the block request crypto engine framework")
>     8c98ebd7a6ff ("crypto: img-hash - CRYPTO_DEV_IMGTEC_HASH should depend on HAS_DMA")
>     a5bd093af0d1 ("crypto: ccp - Update CCP build support")
>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
>     b53a2340d0d3 ("of/reconfig: Add of_reconfig_get_state_change() of notifier helper.")
>     cfa8e7e70341 ("crypto: img-hash - Fix Kconfig selections")
>     d2e3ae6f3aba ("crypto: vmx - Enabling VMX module for PPC64")
>     d358f1abbf71 ("crypto: img-hash - Add Imagination Technologies hw hash accelerator")
>     f1b77aaca85a ("crypto: omap-des - Integrate with the crypto engine framework")
>     f5242e5a883b ("of/reconfig: Always use the same structure for notifiers")
>     f6892d193fb9 ("of/reconfig: Add empty stubs for the of_reconfig methods")
>     fdd05e4b9ae2 ("crypto: nx - rename nx-842.c to nx-842-pseries.c")

FWIW the list of possible dependencies looks completely confused. If this 
is not some interminnent bug, the way how it's generated should probably 
be revisited.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v3] HID: debug: fix the ring buffer implementation

[Sasha Levin]

On Wed, Jan 30, 2019 at 05:59:43PM +0100, Jiri Kosina wrote:
>On Wed, 30 Jan 2019, Sasha Levin wrote:
>
>> Hi,
>>
>> [This is an automated email]
>>
>> This commit has been processed because it contains a "Fixes:" tag,
>> fixing commit: cd667ce24796 HID: use debugfs for events/reports dumping.
>>
>> The bot has tested the following trees: v4.20.5, v4.19.18, v4.14.96, v4.9.153, v4.4.172, v3.18.133.
>>
>> v4.20.5: Build OK!
>> v4.19.18: Build OK!
>> v4.14.96: Failed to apply! Possible dependencies:
>>     0eecc636e5a2 ("bus: ti-sysc: Add minimal TI sysc interconnect target driver")
>>     49a0a3d805df ("bus: ti-sysc: Make omap_hwmod_sysc_fields into sysc_regbits platform data")
>>     566a9b05e1fa ("bus: ti-sysc: Handle module quirks based dts configuration")
>>     6396bb221514 ("treewide: kzalloc() -> kcalloc()")
>>     695eea3d2c7f ("ARM: OMAP2+: Enable ti-sysc to use device tree data for smartreflex")
>>     70a65240efb1 ("bus: ti-sysc: Add register bits for interconnect target modules")
>>     a7199e2b91de ("bus: ti-sysc: Detect i2c interconnect target module based on register layout")
>>     bf8070522298 ("ARM: OMAP2+: Move all omap_hwmod_sysc_fields to omap_hwmod_common_data.c")
>>     c5a2de97fbd2 ("bus: ti-sysc: Add parsing of module capabilities")
>>     d060b40523dc ("ARM: OMAP2+: Prepare to pass auxdata for smartreflex")
>>     ef70b0bdeaf8 ("bus: ti-sysc: Add support for platform data callbacks")
>>
>> v4.9.153: Failed to apply! Possible dependencies:
>>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
>>     1cec20f0ea0e ("dma-buf: Restart reservation_object_wait_timeout_rcu() after writes")
>>     555570d744f8 ("sched/clock: Update static_key usage")
>>     78010cd9736e ("dma-buf/fence: add an lockdep_assert_held()")
>>     983de5f97169 ("firmware: tegra: Add BPMP support")
>>     9881b024b7d7 ("sched/clock: Delay switching sched_clock to stable")
>>     acb04058de49 ("sched/clock: Fix hotplug crash")
>>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
>>     b52992c06c90 ("drm/i915: Support asynchronous waits on struct fence from i915_gem_request")
>>     ca791d7f4256 ("firmware: tegra: Add IVC library")
>>     e601757102cf ("sched/headers: Prepare for new header dependencies before moving code to <linux/sched/clock.h>")
>>     ea8b1c4a6019 ("drivers: psci: PSCI checker module")
>>     f54d1867005c ("dma-buf: Rename struct fence to dma_fence")
>>     fedf54132d24 ("dma-buf: Restart reservation_object_get_fences_rcu() after writes")
>>
>> v4.4.172: Failed to apply! Possible dependencies:
>>     0529900a01cb ("crypto: omap-aes - Support crypto engine framework")
>>     0bc40be85f33 ("drm/i915: Rename intel_engine_cs function parameters")
>>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
>>     2589ad84047f ("crypto: engine - move crypto engine to its own header")
>>     4cba7cf025f3 ("crypto: engine - permit to enqueue ashash_request")
>>     735d37b5424b ("crypto: engine - Introduce the block request crypto engine framework")
>>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
>>     c81d46138da6 ("drm/i915: Convert trace-irq to the breadcrumb waiter")
>>     ca82580c9cea ("drm/i915: Do not call API requiring struct_mutex where it is not available")
>>     cbdc12a9fc9d ("drm/i915: make A0 wa's applied to A1")
>>     e87a005d90c3 ("drm/i915: add helpers for platform specific revision id range checks")
>>     ea8b1c4a6019 ("drivers: psci: PSCI checker module")
>>     ef712bb4b700 ("drm/i915: remove parens around revision ids")
>>     f1b77aaca85a ("crypto: omap-des - Integrate with the crypto engine framework")
>>     fffda3f4fb49 ("drm/i915/bxt: add revision id for A1 stepping and use it")
>>
>> v3.18.133: Failed to apply! Possible dependencies:
>>     00aa37206e1a ("of/reconfig: Add debug output for OF_RECONFIG notifiers")
>>     0529900a01cb ("crypto: omap-aes - Support crypto engine framework")
>>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
>>     2589ad84047f ("crypto: engine - move crypto engine to its own header")
>>     310b0d55f030 ("crypto: omap-aes - Fix CTR mode")
>>     4cba7cf025f3 ("crypto: engine - permit to enqueue ashash_request")
>>     596103cf8fb0 ("crypto: drivers - Fix Kconfig selects")
>>     6c5063434098 ("crypto: ccp - Add ACPI support")
>>     7011a122383e ("crypto: nx - add NX-842 platform frontend driver")
>>     735d37b5424b ("crypto: engine - Introduce the block request crypto engine framework")
>>     8c98ebd7a6ff ("crypto: img-hash - CRYPTO_DEV_IMGTEC_HASH should depend on HAS_DMA")
>>     a5bd093af0d1 ("crypto: ccp - Update CCP build support")
>>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
>>     b53a2340d0d3 ("of/reconfig: Add of_reconfig_get_state_change() of notifier helper.")
>>     cfa8e7e70341 ("crypto: img-hash - Fix Kconfig selections")
>>     d2e3ae6f3aba ("crypto: vmx - Enabling VMX module for PPC64")
>>     d358f1abbf71 ("crypto: img-hash - Add Imagination Technologies hw hash accelerator")
>>     f1b77aaca85a ("crypto: omap-des - Integrate with the crypto engine framework")
>>     f5242e5a883b ("of/reconfig: Always use the same structure for notifiers")
>>     f6892d193fb9 ("of/reconfig: Add empty stubs for the of_reconfig methods")
>>     fdd05e4b9ae2 ("crypto: nx - rename nx-842.c to nx-842-pseries.c")
>
>FWIW the list of possible dependencies looks completely confused. If this
>is not some interminnent bug, the way how it's generated should probably
>be revisited.

It's actually somewhat accurate, but useless in this case :)

The reason for that mess is that on <=4.14 kernels, there is a
dependency on 6396bb22151 ("treewide: kzalloc() -> kcalloc()"), but
since that patch is a treewide change, to bring that in we need a big
pile of seemingly random patches from all over the tree, which is the
output you see.

--
Thanks,
Sasha

Re: [PATCH v3] HID: debug: fix the ring buffer implementation

[Vladis Dronov]

Hello, Sasha, all,

> The reason for that mess is that on <=4.14 kernels, there is a
> dependency on 6396bb22151 ("treewide: kzalloc() -> kcalloc()"), but
> since that patch is a treewide change, to bring that in we need a big
> pile of seemingly random patches from all over the tree, which is the
> output you see.

Indeed, this patch depends on 2 tree-wide patches, one being 6396bb22151
("treewide: kzalloc() -> kcalloc()") as you've mentioned and another one
is a9a08845e9ac ("vfs: do bulk POLL* -> EPOLL* replacement"). In the part
of HID driver they basically do:

#6396bb22151

-        buf = kzalloc(sizeof(char) * HID_DEBUG_BUFSIZE, GFP_ATOMIC);
+        buf = kzalloc(HID_DEBUG_BUFSIZE, GFP_ATOMIC);

-    if (!(list->hid_debug_buf = kzalloc(sizeof(char) * HID_DEBUG_BUFSIZE, GFP_KERNEL))) {
+    if (!(list->hid_debug_buf = kzalloc(HID_DEBUG_BUFSIZE, GFP_KERNEL))) {

#a9a08845e9ac
-        return POLLIN | POLLRDNORM;
+        return EPOLLIN | EPOLLRDNORM;

-        return POLLERR | POLLHUP;
+        return EPOLLERR | EPOLLHUP;

Adding this 2 changes to the 4.14.y make HID patch to apply cleanly (except
offsets). I suppose, I need to backport HID patch (adding these 2 changes
above) and post to stable@.

I will follow this process:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

In case I do smth wrong, please, tell.

Best regards,
Vladis Dronov | Red Hat, Inc. | Product Security | Senior Software Engineer

Re: [PATCH v3] HID: debug: fix the ring buffer implementation

[Vladis Dronov]

Hello,

On Thu, Feb 7, 2019 at 9:42 PM Salvatore Bonaccorso <carnil@debian.org> wrote:
> 717adfdaf147 ("HID: debug: check length before copy_to_user()") was
> introduced in v4.18-rc5, but it was backported to other stable
> branches, for instance:
>
> 50b4d984f55e7e8d75f75da6803505ca3c122cef (4.14.55),
> 4a30c12542290f1def08b9ef0d677c024c500589 (4.9.112),
> ef111ea31575bdc50c0c914fe036a1d0ad0cae4e (4.4.140),
> f7e1dd8ebca4d67411c333223e4205879d141eaa (3.18.115),
> e44ab03f41ba55e181f4ed64e546feac8f8e69dc (3.16.59).

Indeed, 717adfdaf147 is there.  I'm preparing backports of this my patch
(as it is not applying cleanly to 4.14 and earlier branches due to some
tree-wide changes) and plan to post them to stable@ soon.

Best regards,
Vladis Dronov | Red Hat, Inc. | Product Security | Senior Software Engineer

----- Original Message -----
> From: "Sasha Levin" <sashal@kernel.org>
> To: "Jiri Kosina" <jikos@kernel.org>
> Cc: "Vladis Dronov" <vdronov@redhat.com>, stable@vger.kernel.org
> Sent: Wednesday, January 30, 2019 6:52:02 PM
> Subject: Re: [PATCH v3] HID: debug: fix the ring buffer implementation
> 
> On Wed, Jan 30, 2019 at 05:59:43PM +0100, Jiri Kosina wrote:
> >On Wed, 30 Jan 2019, Sasha Levin wrote:
> >
> >> Hi,
> >>
> >> [This is an automated email]
> >>
> >> This commit has been processed because it contains a "Fixes:" tag,
> >> fixing commit: cd667ce24796 HID: use debugfs for events/reports dumping.
> >>
> >> The bot has tested the following trees: v4.20.5, v4.19.18, v4.14.96,
> >> v4.9.153, v4.4.172, v3.18.133.
> >>
> >> v4.20.5: Build OK!
> >> v4.19.18: Build OK!
> >> v4.14.96: Failed to apply! Possible dependencies:
> >>     0eecc636e5a2 ("bus: ti-sysc: Add minimal TI sysc interconnect target
> >>     driver")
> >>     49a0a3d805df ("bus: ti-sysc: Make omap_hwmod_sysc_fields into
> >>     sysc_regbits platform data")
> >>     566a9b05e1fa ("bus: ti-sysc: Handle module quirks based dts
> >>     configuration")
> >>     6396bb221514 ("treewide: kzalloc() -> kcalloc()")
> >>     695eea3d2c7f ("ARM: OMAP2+: Enable ti-sysc to use device tree data for
> >>     smartreflex")
> >>     70a65240efb1 ("bus: ti-sysc: Add register bits for interconnect target
> >>     modules")
> >>     a7199e2b91de ("bus: ti-sysc: Detect i2c interconnect target module
> >>     based on register layout")
> >>     bf8070522298 ("ARM: OMAP2+: Move all omap_hwmod_sysc_fields to
> >>     omap_hwmod_common_data.c")
> >>     c5a2de97fbd2 ("bus: ti-sysc: Add parsing of module capabilities")
> >>     d060b40523dc ("ARM: OMAP2+: Prepare to pass auxdata for smartreflex")
> >>     ef70b0bdeaf8 ("bus: ti-sysc: Add support for platform data callbacks")
> >>
> >> v4.9.153: Failed to apply! Possible dependencies:
> >>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup &
> >>     sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
> >>     1cec20f0ea0e ("dma-buf: Restart reservation_object_wait_timeout_rcu()
> >>     after writes")
> >>     555570d744f8 ("sched/clock: Update static_key usage")
> >>     78010cd9736e ("dma-buf/fence: add an lockdep_assert_held()")
> >>     983de5f97169 ("firmware: tegra: Add BPMP support")
> >>     9881b024b7d7 ("sched/clock: Delay switching sched_clock to stable")
> >>     acb04058de49 ("sched/clock: Fix hotplug crash")
> >>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies
> >>     before moving code to <uapi/linux/sched/types.h>")
> >>     b52992c06c90 ("drm/i915: Support asynchronous waits on struct fence
> >>     from i915_gem_request")
> >>     ca791d7f4256 ("firmware: tegra: Add IVC library")
> >>     e601757102cf ("sched/headers: Prepare for new header dependencies
> >>     before moving code to <linux/sched/clock.h>")
> >>     ea8b1c4a6019 ("drivers: psci: PSCI checker module")
> >>     f54d1867005c ("dma-buf: Rename struct fence to dma_fence")
> >>     fedf54132d24 ("dma-buf: Restart reservation_object_get_fences_rcu()
> >>     after writes")
> >>
> >> v4.4.172: Failed to apply! Possible dependencies:
> >>     0529900a01cb ("crypto: omap-aes - Support crypto engine framework")
> >>     0bc40be85f33 ("drm/i915: Rename intel_engine_cs function parameters")
> >>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup &
> >>     sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
> >>     2589ad84047f ("crypto: engine - move crypto engine to its own header")
> >>     4cba7cf025f3 ("crypto: engine - permit to enqueue ashash_request")
> >>     735d37b5424b ("crypto: engine - Introduce the block request crypto
> >>     engine framework")
> >>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies
> >>     before moving code to <uapi/linux/sched/types.h>")
> >>     c81d46138da6 ("drm/i915: Convert trace-irq to the breadcrumb waiter")
> >>     ca82580c9cea ("drm/i915: Do not call API requiring struct_mutex where
> >>     it is not available")
> >>     cbdc12a9fc9d ("drm/i915: make A0 wa's applied to A1")
> >>     e87a005d90c3 ("drm/i915: add helpers for platform specific revision id
> >>     range checks")
> >>     ea8b1c4a6019 ("drivers: psci: PSCI checker module")
> >>     ef712bb4b700 ("drm/i915: remove parens around revision ids")
> >>     f1b77aaca85a ("crypto: omap-des - Integrate with the crypto engine
> >>     framework")
> >>     fffda3f4fb49 ("drm/i915/bxt: add revision id for A1 stepping and use
> >>     it")
> >>
> >> v3.18.133: Failed to apply! Possible dependencies:
> >>     00aa37206e1a ("of/reconfig: Add debug output for OF_RECONFIG
> >>     notifiers")
> >>     0529900a01cb ("crypto: omap-aes - Support crypto engine framework")
> >>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup &
> >>     sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
> >>     2589ad84047f ("crypto: engine - move crypto engine to its own header")
> >>     310b0d55f030 ("crypto: omap-aes - Fix CTR mode")
> >>     4cba7cf025f3 ("crypto: engine - permit to enqueue ashash_request")
> >>     596103cf8fb0 ("crypto: drivers - Fix Kconfig selects")
> >>     6c5063434098 ("crypto: ccp - Add ACPI support")
> >>     7011a122383e ("crypto: nx - add NX-842 platform frontend driver")
> >>     735d37b5424b ("crypto: engine - Introduce the block request crypto
> >>     engine framework")
> >>     8c98ebd7a6ff ("crypto: img-hash - CRYPTO_DEV_IMGTEC_HASH should depend
> >>     on HAS_DMA")
> >>     a5bd093af0d1 ("crypto: ccp - Update CCP build support")
> >>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies
> >>     before moving code to <uapi/linux/sched/types.h>")
> >>     b53a2340d0d3 ("of/reconfig: Add of_reconfig_get_state_change() of
> >>     notifier helper.")
> >>     cfa8e7e70341 ("crypto: img-hash - Fix Kconfig selections")
> >>     d2e3ae6f3aba ("crypto: vmx - Enabling VMX module for PPC64")
> >>     d358f1abbf71 ("crypto: img-hash - Add Imagination Technologies hw hash
> >>     accelerator")
> >>     f1b77aaca85a ("crypto: omap-des - Integrate with the crypto engine
> >>     framework")
> >>     f5242e5a883b ("of/reconfig: Always use the same structure for
> >>     notifiers")
> >>     f6892d193fb9 ("of/reconfig: Add empty stubs for the of_reconfig
> >>     methods")
> >>     fdd05e4b9ae2 ("crypto: nx - rename nx-842.c to nx-842-pseries.c")
> >
> >FWIW the list of possible dependencies looks completely confused. If this
> >is not some interminnent bug, the way how it's generated should probably
> >be revisited.
> 
> It's actually somewhat accurate, but useless in this case :)
> 
> The reason for that mess is that on <=4.14 kernels, there is a
> dependency on 6396bb22151 ("treewide: kzalloc() -> kcalloc()"), but
> since that patch is a treewide change, to bring that in we need a big
> pile of seemingly random patches from all over the tree, which is the
> output you see.
> 
> --
> Thanks,
> Sasha