All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Günther Noack" <gnoack@google.com>
To: Lee Jones <lee@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <bentiss@kernel.org>,
	linux-input@vger.kernel.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID
Date: Wed, 15 Apr 2026 15:30:52 +0200	[thread overview]
Message-ID: <ad-S8Riah-f6mEWm@google.com> (raw)
In-Reply-To: <20260414143238.1177080-1-lee@kernel.org>

Hello Lee!

On Tue, Apr 14, 2026 at 03:32:38PM +0100, Lee Jones wrote:
> It is currently possible for a malicious or misconfigured USB device to
> cause an out-of-bounds (OOB) read when submitting reports using
> DOUBLE_REPORT_ID by specifying a large report length and providing a
> smaller one.
> 
> Let's prevent that by comparing the specified report length with the
> actual size of the data read in from userspace.  If the actual data
> length ends up being smaller than specified, we'll politely warn the
> user and prevent any further processing.
> 
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>  drivers/hid/hid-magicmouse.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 91f621ceb924..5f44129e6dcc 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -490,6 +490,14 @@ static int magicmouse_raw_event(struct hid_device *hdev,
>  		/* Sometimes the trackpad sends two touch reports in one
>  		 * packet.
>  		 */
> +
> +		if (size < data[1] + 2) {
> +			hid_warn(hdev,
> +				 "received report length (%d) was smaller than specified (%d)",
> +				 size, data[1] + 2);
> +			return 0;
> +		}
> +
>  		magicmouse_raw_event(hdev, report, data + 2, data[1]);
>  		magicmouse_raw_event(hdev, report, data + 2 + data[1],
>  			size - 2 - data[1]);
> -- 
> 2.54.0.rc0.605.g598a273b03-goog

I am afraid this still looks buggy to me.

With your check, size can still be *equal* to data[1] + 2.  In that case, the
second recursive call becomes

  magicmouse_raw_event(hdev, report, data + size, 0);

so this points just after the original "data" buffer with zero size.

But the magicmouse_raw_event() function accesses data[0] directly at the
beginning.  It assumes that size is >= 1, but it does not check that before that
first access.

—Günther

      reply	other threads:[~2026-04-15 13:31 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-14 14:32 [PATCH 1/1] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID Lee Jones
2026-04-15 13:30 ` Günther Noack [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=ad-S8Riah-f6mEWm@google.com \
    --to=gnoack@google.com \
    --cc=bentiss@kernel.org \
    --cc=jikos@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.