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

Bastien Nocera
2026-04-16T09:36:28Z

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@vger.kernel.org
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
>  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);