Re: [linux-6.12.y 3/4] HID: core: introduce hid_safe_input_report()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The patch introduces a hard build failure by passing 9 arguments to a function expecting 8.

Pre-existing issues:
- [High] Integer underflow in `i2c_hid_get_input()` when computing the remaining buffer size for bogus HID descriptors.
--

commit d54718e72e548c203a2c42b46b69bf352c081a00
Author: Benjamin Tissoires <bentiss@kernel.org>

HID: core: introduce hid_safe_input_report()

This patch introduces hid_safe_input_report() to be used in transport
layers like usbhid and i2chid. This provides the buffer size allowing
the hid-core to make smarter decisions regarding incoming buffers.

> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 294a25330ed03..2c5583818421e 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
[ ... ]
> @@ -2141,6 +2142,30 @@ 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.
> + * Please use this function instead of the non safe version because we provide
> + * here the size of the buffer, allowing hid-core to make smarter decisions
> + * regarding the incoming buffer.
> + */
> +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 */);
> +}

[Severity: Low]
Does this cause a build failure? Looking at the definition of
__hid_input_report() in this file, it only accepts 8 arguments:

static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
                              u8 *data, u32 size, int interrupt, u64 source,
                              bool from_bpf, bool lock_already_taken)

This call passes 9 arguments, including the new bufsize parameter. Were some
prerequisite upstream patches updating the signature of __hid_input_report()
omitted from this backport?

> +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 cf8ae0df0cda9..8ce0535fc42d6 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -568,9 +568,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);

[Severity: High]
This is a pre-existing issue, but can this calculation underflow and pass a
massive size to the HID core?

If a device has the I2C_HID_QUIRK_BAD_INPUT_SIZE quirk and provides a bogus
descriptor with wMaxInputLength < 2, the size variable will be < 2, and the
code earlier in i2c_hid_get_input() forces ret_size = size.

When ret_size and ihid->bufsize are less than sizeof(__le16) (which is 2),
subtracting sizeof(__le16) will result in an unsigned integer underflow.
This large underflowed value would then be passed into the HID core here and
could lead to out-of-bounds reads in functions like hid_dump_report().

>  	}
>  
>  	return;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601083642.908433-3-lee@kernel.org?part=1