[PATCH] staging: greybus: hid: fix off-by-one in SET_REPORT allocation

[PATCH] staging: greybus: hid: fix off-by-one in SET_REPORT allocation

[Farhad Alemi]

gb_hid_set_report() sizes its request payload as sizeof(*request) + len -
1, but report[] in struct gb_hid_set_report_request is a flexible array
member that sizeof() already excludes. The buffer is therefore one byte too
small, so memcpy(request->report, buf, len) writes one byte past its end,
which KASAN reports as a slab-out-of-bounds write. Drop the stray - 1 so
the allocation covers the whole report.

Closes: https://lore.kernel.org/all/CA+0ovCgLrz4WhPKP5LGW5HZa8VOodgeo6pWuyQGgHE7UY57Oog@mail.gmail.com/
Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
---
The device was emulated.

--- a/drivers/staging/greybus/hid.c
+++ b/drivers/staging/greybus/hid.c
@@ -97,7 +97,8 @@ static int gb_hid_set_report(struct gb_hid *ghid, u8
report_type, u8 report_id,
 {
 	struct gb_hid_set_report_request *request;
 	struct gb_operation *operation;
-	int ret, size = sizeof(*request) + len - 1;
+	/* report[] is a flexible array, so sizeof() already excludes it. */
+	int ret, size = sizeof(*request) + len;

 	ret = gb_pm_runtime_get_sync(ghid->bundle);
 	if (ret)

Re: [PATCH] staging: greybus: hid: fix off-by-one in SET_REPORT allocation

[Greg KH]

On Thu, Sep 10, 2026 at 09:32:47PM +0000, Farhad Alemi wrote:
> gb_hid_set_report() sizes its request payload as sizeof(*request) + len -
> 1, but report[] in struct gb_hid_set_report_request is a flexible array
> member that sizeof() already excludes. The buffer is therefore one byte too
> small, so memcpy(request->report, buf, len) writes one byte past its end,
> which KASAN reports as a slab-out-of-bounds write. Drop the stray - 1 so
> the allocation covers the whole report.
> 
> Closes: https://lore.kernel.org/all/CA+0ovCgLrz4WhPKP5LGW5HZa8VOodgeo6pWuyQGgHE7UY57Oog@mail.gmail.com/
> Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>

Did you forget an Assisted-by: tag?

> ---
> The device was emulated.

emulated how?

thanks,

greg k-h

Re: [PATCH] staging: greybus: hid: fix off-by-one in SET_REPORT allocation

[Dan Carpenter]

Alex, Greg isn't accepting AI patches for emulated devices.  Is this
something you could approve?

On Thu, Sep 10, 2026 at 09:32:47PM +0000, Farhad Alemi wrote:
> gb_hid_set_report() sizes its request payload as sizeof(*request) + len -
> 1, but report[] in struct gb_hid_set_report_request is a flexible array
> member that sizeof() already excludes.

I can't really understand this sentence.  What is excluded?

> The buffer is therefore one byte too
> small, so memcpy(request->report, buf, len) writes one byte past its end,
> which KASAN reports as a slab-out-of-bounds write. Drop the stray - 1 so
> the allocation covers the whole report.

I think a better commit message is.

  This "sizeof(*request) + len - 1" calculation is wrong.  It's unclear
  where the "- 1" comes from.  Perhaps the request->report[] started as
  a one element array before the driver was published?  That is something
  that people used to do. Regardless, when we do the memcpy(),

  memcpy(request->report, buf, len);

  Then it will write one byte past the end of the buffer.

> 
> Closes: https://lore.kernel.org/all/CA+0ovCgLrz4WhPKP5LGW5HZa8VOodgeo6pWuyQGgHE7UY57Oog@mail.gmail.com/
> Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>

This needs a Fixes tag.

Fixes: 96eab779e198 ("greybus: hid: add HID class driver")

> ---
> The device was emulated.
> 
> --- a/drivers/staging/greybus/hid.c
> +++ b/drivers/staging/greybus/hid.c
> @@ -97,7 +97,8 @@ static int gb_hid_set_report(struct gb_hid *ghid, u8
> report_type, u8 report_id,

The patch is corrupt and doesn't apply.  Read the first couple paragraphs
of Documentation/process/email-clients.rst

>  {
>  	struct gb_hid_set_report_request *request;
>  	struct gb_operation *operation;
> -	int ret, size = sizeof(*request) + len - 1;
> +	/* report[] is a flexible array, so sizeof() already excludes it. */

AI always adds these pointless comments.  Only interesting lines of
code need comments.  Imagine if every line of the kernel had comments.
It would eventually turn into something like the Terms and Conditions
where it would take more than a human lifetime to read all the things
we agree to.  We need to create an AGENTS.md which tells AI this stuff.

regards,
dan carpenter

> +	int ret, size = sizeof(*request) + len;
> 
>  	ret = gb_pm_runtime_get_sync(ghid->bundle);
>  	if (ret)