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

[Carlos Llamas]

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>
> > > ---
> > >  drivers/hid/hid-core.c             | 21 +++++++++++++++++++++
> > >  drivers/hid/i2c-hid/i2c-hid-core.c |  7 ++++---
> > >  drivers/hid/usbhid/hid-core.c      | 11 ++++++-----
> > >  include/linux/hid.h                |  2 ++
> > >  4 files changed, 33 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > > index a806820df7e5..cb0ad99e7a0a 100644
> > > --- a/drivers/hid/hid-core.c
> > > +++ b/drivers/hid/hid-core.c
> > > @@ -2191,6 +2191,27 @@ int hid_input_report(struct hid_device *hid,
> > > enum hid_report_type type, u8 *data
> > >  }
> > >  EXPORT_SYMBOL_GPL(hid_input_report);
> > >
> > > +/**
> > > + * hid_safe_input_report - report data from lower layer (usb, bt...)
> > > + *
> > > + * @hid: hid device
> > > + * @type: HID report type (HID_*_REPORT)
> > > + * @data: report contents
> > > + * @bufsize: allocated size of the data buffer
> > > + * @size: useful size of data parameter
> > > + * @interrupt: distinguish between interrupt and control transfers
> > > + *
> > > + * This is data entry for lower layers.
> >
> > You probably want to explain why it should be used instead of
> > hid_input_report() in this doc blurb, and modify the hid_input_report()
> > docs to mention that this should be used.
> 
> Good point. Sending v2 ASAP.
> 
> >
> > Maybe hid_input_report() should also be marked as deprecated somehow,
> > to avoid new users?
> 
> Well, it's not entirely deprecated because, for instance, in uhid we
> only have the buffer with the provided size around. So we can't be
> less restrictive in that precise case, and then switching to _safe
> will not change a bit.
> 
> Cheers,
> Benjamin

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.

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;
 }