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

[Bastien Nocera]

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.

Maybe hid_input_report() should also be marked as deprecated somehow,
to avoid new users?

Cheers

> + */
> +int hid_safe_input_report(struct hid_device *hid, enum
> hid_report_type type, u8 *data,
> +			  size_t bufsize, u32 size, int interrupt)
> +{
> +	return __hid_input_report(hid, type, data, bufsize, size,
> interrupt, 0,
> +				  false, /* from_bpf */
> +				  false /* lock_already_taken */);
> +}
> +EXPORT_SYMBOL_GPL(hid_safe_input_report);
> +
>  bool hid_match_one_id(const struct hid_device *hdev,
>  		      const struct hid_device_id *id)
>  {
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-
> hid/i2c-hid-core.c
> index 5a183af3d5c6..e0a302544cef 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -574,9 +574,10 @@ static void i2c_hid_get_input(struct i2c_hid
> *ihid)
>  		if (ihid->hid->group != HID_GROUP_RMI)
>  			pm_wakeup_event(&ihid->client->dev, 0);
>  
> -		hid_input_report(ihid->hid, HID_INPUT_REPORT,
> -				ihid->inbuf + sizeof(__le16),
> -				ret_size - sizeof(__le16), 1);
> +		hid_safe_input_report(ihid->hid, HID_INPUT_REPORT,
> +				      ihid->inbuf + sizeof(__le16),
> +				      ihid->bufsize -
> sizeof(__le16),
> +				      ret_size - sizeof(__le16), 1);
>  	}
>  
>  	return;
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-
> core.c
> index fbbfc0f60829..5af93b9b1fb5 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -283,9 +283,9 @@ static void hid_irq_in(struct urb *urb)
>  			break;
>  		usbhid_mark_busy(usbhid);
>  		if (!test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
> -			hid_input_report(urb->context,
> HID_INPUT_REPORT,
> -					 urb->transfer_buffer,
> -					 urb->actual_length, 1);
> +			hid_safe_input_report(urb->context,
> HID_INPUT_REPORT,
> +					      urb->transfer_buffer,
> urb->transfer_buffer_length,
> +					      urb->actual_length,
> 1);
>  			/*
>  			 * autosuspend refused while keys are
> pressed
>  			 * because most keyboards don't wake up when
> @@ -482,9 +482,10 @@ static void hid_ctrl(struct urb *urb)
>  	switch (status) {
>  	case 0:			/* success */
>  		if (usbhid->ctrl[usbhid->ctrltail].dir ==
> USB_DIR_IN)
> -			hid_input_report(urb->context,
> +			hid_safe_input_report(urb->context,
>  				usbhid->ctrl[usbhid-
> >ctrltail].report->type,
> -				urb->transfer_buffer, urb-
> >actual_length, 0);
> +				urb->transfer_buffer, urb-
> >transfer_buffer_length,
> +				urb->actual_length, 0);
>  		break;
>  	case -ESHUTDOWN:	/* unplug */
>  		unplug = 1;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index ac432a2ef415..bfb9859f391e 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -1030,6 +1030,8 @@ struct hid_field *hid_find_field(struct
> hid_device *hdev, unsigned int report_ty
>  int hid_set_field(struct hid_field *, unsigned, __s32);
>  int hid_input_report(struct hid_device *hid, enum hid_report_type
> type, u8 *data, u32 size,
>  		     int interrupt);
> +int hid_safe_input_report(struct hid_device *hid, enum
> hid_report_type type, u8 *data,
> +			  size_t bufsize, u32 size, int interrupt);
>  struct hid_field *hidinput_get_led_field(struct hid_device *hid);
>  unsigned int hidinput_count_leds(struct hid_device *hid);
>  __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16
> code);