mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] HID: debug: check length before copy_to_user()
@ 2018-07-02 23:59 Daniel Rosenberg
  2018-07-03  7:50 ` Benjamin Tissoires
  2018-07-05  0:47 ` Al Viro
  0 siblings, 2 replies; 4+ messages in thread
From: Daniel Rosenberg @ 2018-07-02 23:59 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input
  Cc: linux-kernel, kernel-team, Daniel Rosenberg, stable

If our length is greater than the size of the buffer, we
overflow the buffer

Signed-off-by: Daniel Rosenberg <drosen@google.com>
Cc: stable@vger.kernel.org
---
 drivers/hid/hid-debug.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 8469b6964ff64..b48100236df89 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -1154,6 +1154,8 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
 			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;
@@ -1163,6 +1165,8 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
 			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;
@@ -1170,7 +1174,9 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
 			}
 			list->head = 0;
 			ret += len;
-			goto copy_rest;
+			count -= len;
+			if (count > 0)
+				goto copy_rest;
 		}
 
 	}
-- 
2.18.0.399.gad0ab374a1-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] HID: debug: check length before copy_to_user()
  2018-07-02 23:59 [PATCH] HID: debug: check length before copy_to_user() Daniel Rosenberg
@ 2018-07-03  7:50 ` Benjamin Tissoires
  2018-07-03  9:55   ` Jiri Kosina
  2018-07-05  0:47 ` Al Viro
  1 sibling, 1 reply; 4+ messages in thread
From: Benjamin Tissoires @ 2018-07-03  7:50 UTC (permalink / raw)
  To: Daniel Rosenberg
  Cc: Jiri Kosina, open list:HID CORE LAYER, lkml, kernel-team, 3.8+

On Tue, Jul 3, 2018 at 1:59 AM, Daniel Rosenberg <drosen@google.com> wrote:
> If our length is greater than the size of the buffer, we
> overflow the buffer
>
> Signed-off-by: Daniel Rosenberg <drosen@google.com>
> Cc: stable@vger.kernel.org
> ---

Looks good:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

>  drivers/hid/hid-debug.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index 8469b6964ff64..b48100236df89 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -1154,6 +1154,8 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
>                         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;
> @@ -1163,6 +1165,8 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
>                         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;
> @@ -1170,7 +1174,9 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
>                         }
>                         list->head = 0;
>                         ret += len;
> -                       goto copy_rest;
> +                       count -= len;
> +                       if (count > 0)
> +                               goto copy_rest;
>                 }
>
>         }
> --
> 2.18.0.399.gad0ab374a1-goog
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] HID: debug: check length before copy_to_user()
  2018-07-03  7:50 ` Benjamin Tissoires
@ 2018-07-03  9:55   ` Jiri Kosina
  0 siblings, 0 replies; 4+ messages in thread
From: Jiri Kosina @ 2018-07-03  9:55 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Daniel Rosenberg, open list:HID CORE LAYER, lkml, kernel-team, 3.8+

On Tue, 3 Jul 2018, Benjamin Tissoires wrote:

> > If our length is greater than the size of the buffer, we
> > overflow the buffer
> >
> > Signed-off-by: Daniel Rosenberg <drosen@google.com>
> > Cc: stable@vger.kernel.org
> > ---
> 
> Looks good:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Indeed, thanks a lot for fixing this. Now queued, I'll push it for 4.18.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] HID: debug: check length before copy_to_user()
  2018-07-02 23:59 [PATCH] HID: debug: check length before copy_to_user() Daniel Rosenberg
  2018-07-03  7:50 ` Benjamin Tissoires
@ 2018-07-05  0:47 ` Al Viro
  1 sibling, 0 replies; 4+ messages in thread
From: Al Viro @ 2018-07-05  0:47 UTC (permalink / raw)
  To: Daniel Rosenberg
  Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
	kernel-team, stable

On Mon, Jul 02, 2018 at 04:59:37PM -0700, Daniel Rosenberg wrote:
> If our length is greater than the size of the buffer, we
> overflow the buffer

Hmm...

How about this:
	buf = list->hid_debug_buf;
	if (list->tail < list->head) {
		ret = simple_read_from_buffer(buffer, count, &list->head,
					buf, HID_DEBUG_BUFSIZE);
		if (ret < 0)
			break;
		if (list->head != buf + HID_DEBUG_BUFSIZE)
			break;
		list->head = 0;
	}
	n = simple_read_from_buffer(buffer + ret, count - ret, &list->head,
					buf, list->tail);
	if (n >= 0)
		ret += n;
	if (list->head == buf + HID_DEBUG_BUFSIZE)
		list->head = 0;
instead?

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-07-05  0:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-02 23:59 [PATCH] HID: debug: check length before copy_to_user() Daniel Rosenberg
2018-07-03  7:50 ` Benjamin Tissoires
2018-07-03  9:55   ` Jiri Kosina
2018-07-05  0:47 ` Al Viro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®