From: Dan Carpenter <error27@gmail.com>
To: Farhad Alemi <farhad.alemi@berkeley.edu>, Alex Elder <elder@kernel.org>
Cc: Viresh Kumar <vireshk@kernel.org>,
Johan Hovold <johan@kernel.org>,
falemi@asu.edu, greybus-dev@lists.linaro.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: greybus: hid: fix off-by-one in SET_REPORT allocation
Date: Fri, 11 Sep 2026 15:14:32 +0300 [thread overview]
Message-ID: <aqPwqFu0eFY4CSZ0@stanley.mountain> (raw)
In-Reply-To: <CA+0ovCirfyOhrXdxmM3ri9-cShD0mzPptO1qtsaboS1xscaM0Q@mail.gmail.com>
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)
prev parent reply other threads:[~2026-09-11 12:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-10 21:32 Farhad Alemi
2026-09-11 5:57 ` Greg KH
2026-09-11 12:14 ` Dan Carpenter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aqPwqFu0eFY4CSZ0@stanley.mountain \
--to=error27@gmail.com \
--cc=elder@kernel.org \
--cc=falemi@asu.edu \
--cc=farhad.alemi@berkeley.edu \
--cc=greybus-dev@lists.linaro.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=vireshk@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®