Re: [PATCH v3 1/4] HID: pass the buffer size to hid_report_raw_event

Re: [PATCH v3 1/4] HID: pass the buffer size to hid_report_raw_event

[Greg Kroah-Hartman]

On Mon, May 04, 2026 at 10:47:22AM +0200, Benjamin Tissoires wrote:
> commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()") enforced the provided data to be at least the size of
> the declared buffer in the report descriptor to prevent a buffer
> overflow. However, we can try to be smarter by providing both the buffer
> size and the data size, meaning that hid_report_raw_event() can make
> better decision whether we should plaining reject the buffer (buffer
> overflow attempt) or if we can safely memset it to 0 and pass it to the
> rest of the stack.
> 
> Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v3 4/4] HID: wacom: use __free(kfree) to clean up temporary buffers

[Dmitry Torokhov]

Hi Benjamin,

On Mon, May 04, 2026 at 10:47:25AM +0200, Benjamin Tissoires wrote:
> @@ -386,10 +381,11 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>  	case WACOM_HID_WD_OFFSETRIGHT:
>  	case WACOM_HID_WD_OFFSETBOTTOM:
>  		/* read manually */
> -		n = hid_report_len(field->report);
> -		data = hid_alloc_report_buf(field->report, GFP_KERNEL);
> +		u8 *data __free(kfree) = hid_alloc_report_buf(field->report, GFP_KERNEL);
> +
>  		if (!data)
>  			break;
> +		n = hid_report_len(field->report);
>  		data[0] = field->report->id;
>  		ret = wacom_get_report(hdev, HID_FEATURE_REPORT,
>  					data, n, WAC_CMD_RETRIES);
> @@ -400,7 +396,6 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>  			hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
>  				 __func__);
>  		}
> -		kfree(data);
>  		break;
>  	}

I'd recommend establishing a new scope for the "data", otherwise it is
fragile. If there was another label below then this cleanup would
explode since current scope of "data" is from the declaration point
until the end of the switch statement.

Having a dedicated scope makes lifertime explicit.

Thanks.

-- 
Dmitry

Re: [PATCH v3 0/4] HID: Proper fix for OOM in hid-core

[Lee Jones]

On Mon, 04 May 2026, Benjamin Tissoires wrote:

> Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()") enforced the provided data to be at least the size of
> the declared buffer in the report descriptor to prevent a buffer
> overflow.
> 
> We only had corner cases of malicious devices exposing the OOM because
> in most cases, the buffer provided by the transport layer needs to be
> allocated at probe time and is large enough to handle all the possible
> reports.
> 
> However, the patch from above, which enforces the spec a little bit more
> introduced both regressions for devices not following the spec (not
> necesserally malicious), but also a stream of errors for those devices.
> 
> Let's revert to the old behavior by giving more information to HID core
> to be able to decide whether it can or not memset the rest of the buffer
> to 0 and continue the processing.
> 
> Note that the first commit makes an API change, but the callers are
> relatively limited, so it should be fine on its own. The second patch
> can't really make the same kind of API change because we have too many
> callers in various subsystems. We can switch them one by one to the safe
> approach when needed.
> 
> The last 2 patches are small cleanups I initially put together with the
> 2 first patches, but they can be applied on their own and don't need to
> be pulled in stable like the first 2.
> 
> Cheers,
> Benjamin
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> Changes in v3:
> - fixed ghib -> ghid in greybus
> - fixed i386 size_t debug size reported by kernel-bot
> - Link to v2: https://lore.kernel.org/r/20260416-wip-fix-core-v2-0-be92570e5627@kernel.org
> 
> Changes in v2:
> - added a small blurb explaining the difference between the safe and the
>   non safe version of hid_safe_input_report
> - Link to v1: https://lore.kernel.org/r/20260415-wip-fix-core-v1-0-ed3c4c823175@kernel.org
> 
> ---
> Benjamin Tissoires (4):
>       HID: pass the buffer size to hid_report_raw_event
>       HID: core: introduce hid_safe_input_report()
>       HID: multitouch: use __free(kfree) to clean up temporary buffers
>       HID: wacom: use __free(kfree) to clean up temporary buffers
> 
>  drivers/hid/bpf/hid_bpf_dispatch.c |  6 ++--
>  drivers/hid/hid-core.c             | 67 ++++++++++++++++++++++++++++++--------
>  drivers/hid/hid-gfrm.c             |  4 +--
>  drivers/hid/hid-logitech-hidpp.c   |  2 +-
>  drivers/hid/hid-multitouch.c       | 18 ++++------
>  drivers/hid/hid-primax.c           |  2 +-
>  drivers/hid/hid-vivaldi-common.c   |  2 +-
>  drivers/hid/i2c-hid/i2c-hid-core.c |  7 ++--
>  drivers/hid/usbhid/hid-core.c      | 11 ++++---
>  drivers/hid/wacom_sys.c            | 46 +++++++++-----------------
>  drivers/staging/greybus/hid.c      |  2 +-
>  include/linux/hid.h                |  6 ++--
>  include/linux/hid_bpf.h            | 14 +++++---
>  13 files changed, 109 insertions(+), 78 deletions(-)

What's the plan for this set Benjamin? -rcs or -next?

-- 
Lee Jones

Re: [PATCH v3 0/4] HID: Proper fix for OOM in hid-core

[Lee Jones]

On Wed, 06 May 2026, Lee Jones wrote:

> On Mon, 04 May 2026, Benjamin Tissoires wrote:
> 
> > Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > bogus memset()") enforced the provided data to be at least the size of
> > the declared buffer in the report descriptor to prevent a buffer
> > overflow.
> > 
> > We only had corner cases of malicious devices exposing the OOM because
> > in most cases, the buffer provided by the transport layer needs to be
> > allocated at probe time and is large enough to handle all the possible
> > reports.
> > 
> > However, the patch from above, which enforces the spec a little bit more
> > introduced both regressions for devices not following the spec (not
> > necesserally malicious), but also a stream of errors for those devices.
> > 
> > Let's revert to the old behavior by giving more information to HID core
> > to be able to decide whether it can or not memset the rest of the buffer
> > to 0 and continue the processing.
> > 
> > Note that the first commit makes an API change, but the callers are
> > relatively limited, so it should be fine on its own. The second patch
> > can't really make the same kind of API change because we have too many
> > callers in various subsystems. We can switch them one by one to the safe
> > approach when needed.
> > 
> > The last 2 patches are small cleanups I initially put together with the
> > 2 first patches, but they can be applied on their own and don't need to
> > be pulled in stable like the first 2.
> > 
> > Cheers,
> > Benjamin
> > 
> > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> > Changes in v3:
> > - fixed ghib -> ghid in greybus
> > - fixed i386 size_t debug size reported by kernel-bot
> > - Link to v2: https://lore.kernel.org/r/20260416-wip-fix-core-v2-0-be92570e5627@kernel.org
> > 
> > Changes in v2:
> > - added a small blurb explaining the difference between the safe and the
> >   non safe version of hid_safe_input_report
> > - Link to v1: https://lore.kernel.org/r/20260415-wip-fix-core-v1-0-ed3c4c823175@kernel.org
> > 
> > ---
> > Benjamin Tissoires (4):
> >       HID: pass the buffer size to hid_report_raw_event
> >       HID: core: introduce hid_safe_input_report()
> >       HID: multitouch: use __free(kfree) to clean up temporary buffers
> >       HID: wacom: use __free(kfree) to clean up temporary buffers
> > 
> >  drivers/hid/bpf/hid_bpf_dispatch.c |  6 ++--
> >  drivers/hid/hid-core.c             | 67 ++++++++++++++++++++++++++++++--------
> >  drivers/hid/hid-gfrm.c             |  4 +--
> >  drivers/hid/hid-logitech-hidpp.c   |  2 +-
> >  drivers/hid/hid-multitouch.c       | 18 ++++------
> >  drivers/hid/hid-primax.c           |  2 +-
> >  drivers/hid/hid-vivaldi-common.c   |  2 +-
> >  drivers/hid/i2c-hid/i2c-hid-core.c |  7 ++--
> >  drivers/hid/usbhid/hid-core.c      | 11 ++++---
> >  drivers/hid/wacom_sys.c            | 46 +++++++++-----------------
> >  drivers/staging/greybus/hid.c      |  2 +-
> >  include/linux/hid.h                |  6 ++--
> >  include/linux/hid_bpf.h            | 14 +++++---
> >  13 files changed, 109 insertions(+), 78 deletions(-)
> 
> What's the plan for this set Benjamin? -rcs or -next?

Are there any updates on this set please?

FYI, this set is still important to us.

Ideally, if all is well, it would go into the -rcs for v7.1.

-- 
Lee Jones

Re: [PATCH v3 0/4] HID: Proper fix for OOM in hid-core

[Jiri Kosina]

On Mon, 4 May 2026, Benjamin Tissoires wrote:

> Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()") enforced the provided data to be at least the size of
> the declared buffer in the report descriptor to prevent a buffer
> overflow.
> 
> We only had corner cases of malicious devices exposing the OOM because
> in most cases, the buffer provided by the transport layer needs to be
> allocated at probe time and is large enough to handle all the possible
> reports.
> 
> However, the patch from above, which enforces the spec a little bit more
> introduced both regressions for devices not following the spec (not
> necesserally malicious), but also a stream of errors for those devices.
> 
> Let's revert to the old behavior by giving more information to HID core
> to be able to decide whether it can or not memset the rest of the buffer
> to 0 and continue the processing.
> 
> Note that the first commit makes an API change, but the callers are
> relatively limited, so it should be fine on its own. The second patch
> can't really make the same kind of API change because we have too many
> callers in various subsystems. We can switch them one by one to the safe
> approach when needed.
> 
> The last 2 patches are small cleanups I initially put together with the
> 2 first patches, but they can be applied on their own and don't need to
> be pulled in stable like the first 2.
> 
> Cheers,
> Benjamin
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

I have now queued the first two in hid.git#for-7.1/upstream-fixes.

I expect the remaining two will be applied once respun with Dmitry's 
suggestion on proper guarding.

Thanks,

-- 
Jiri Kosina
SUSE Labs