Re: [PATCH 2/4] HID: core: introduce hid_safe_input_report()

[Benjamin Tissoires]

Hi Carlos,

On May 30 2026, Carlos Llamas wrote:
> On Thu, Apr 16, 2026 at 04:46:28PM +0200, Benjamin Tissoires wrote:
> > On Thu, Apr 16, 2026 at 11:41 AM Bastien Nocera <hadess@xxxxxxxxxx> wrote:
> > >
> > > On Wed, 2026-04-15 at 11:38 +0200, Benjamin Tissoires wrote:
> > > > hid_input_report() is used in too many places to have a commit that
> > > > doesn't cross subsystem borders. Instead of changing the API,
> > > > introduce
> > > > a new one when things matters in the transport layers:
> > > > - usbhid
> > > > - i2chid
> > > >
> > > > This effectively revert to the old behavior for those two transport
> > > > layers.
> > > >
> > > > Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > > > bogus memset()")
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Signed-off-by: Benjamin Tissoires <bentiss@xxxxxxxxxx>
> > > > ---
[...]
> 
> Hi Benjamin, our CI started failing with commit 0a3fe972a7cb ("HID:
> core: Mitigate potential OOB by removing bogus memset()"), so I was
> hoping your patchset would fix this.
> 
> However, I just realized our call path goes through uhid precisely,
> which still triggers the EINVAL error since uhid as not converted to
> hid_safe_input_report().
> 
> My vague understanding though, is that uhid_event uses a static buffer
> in ev->data[UHID_DATA_MAX], so maybe we can use that through
> uhid_dev_input{2}()?
> 
> I ran the following path through our CI and it fixed our issue, so I
> wanted to get your thoughts on this.

Oh, yes, you are correct. Sorry with all the back and forth on this
paritcular topic, my brain assumed that uhid was only allocating the
useful part of the payload and was not safe.

For the future me: the problem with uhid was that we were emultaing
devices that would trigger a bug elsewhere in the stack not in
uhid_dev_input*().

Patch looks good, please send it normally to the ML with your SoB :)

Cheers,
Benjamin

> 
> Carlos Llamas
> 
> ---
>  drivers/hid/uhid.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 524b53a3c87b..37b60c3aaf66 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -595,8 +595,8 @@ static int uhid_dev_input(struct uhid_device *uhid, struct uhid_event *ev)
>  	if (!READ_ONCE(uhid->running))
>  		return -EINVAL;
>  
> -	hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data,
> -			 min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
> +	hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data, UHID_DATA_MAX,
> +			      min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
>  
>  	return 0;
>  }
> @@ -606,8 +606,8 @@ static int uhid_dev_input2(struct uhid_device *uhid, struct uhid_event *ev)
>  	if (!READ_ONCE(uhid->running))
>  		return -EINVAL;
>  
> -	hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data,
> -			 min_t(size_t, ev->u.input2.size, UHID_DATA_MAX), 0);
> +	hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data, UHID_DATA_MAX,
> +			      min_t(size_t, ev->u.input2.size, UHID_DATA_MAX), 0);
>  
>  	return 0;
>  }
>