Re: [PATCH 6/8] HID: oxp: validate report size in raw_event callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] HID: oxp: unvalidated 64-byte read in oxp_hid_raw_event() dev_dbg()
--

commit 1f763db8177cf3b395303a0f7956ff3e6b99d798
Author: Tristan Madani <tristan@talencesecurity.com>

HID: oxp: validate report size in raw_event callbacks

This commit adds report size checks to oxp_hid_raw_event_gen_1() and
oxp_hid_raw_event_gen_2() before casting the report buffer to
struct oxp_gen_1_rgb_report and struct oxp_gen_2_rgb_report.

> diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
> index 1e691ebc1199e..af9f60d45bf29 100644
> --- a/drivers/hid/hid-oxp.c
> +++ b/drivers/hid/hid-oxp.c
> @@ -318,6 +318,9 @@ static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
>  	struct led_classdev_mc *led_mc = drvdata.led_mc;
>  	struct oxp_gen_1_rgb_report *rgb_rep;
>  
> +	if (size < sizeof(struct oxp_gen_1_rgb_report))
> +		return 0;
> +
>  	if (data[1] != OXP_FID_GEN1_RGB_REPLY)
>  		return 0;
>  
> @@ -391,6 +394,9 @@ static int oxp_hid_raw_event_gen_2(struct hid_device *hdev,
>  	struct led_classdev_mc *led_mc = drvdata.led_mc;
>  	struct oxp_gen_2_rgb_report *rgb_rep;
>  
> +	if (size < sizeof(struct oxp_gen_2_rgb_report))
> +		return 0;
> +
>  	if (data[0] != OXP_FID_GEN2_STATUS_EVENT)
>  		return 0;
>  

[Severity: Medium]
Can a short report still trigger an out-of-bounds read in the caller
oxp_hid_raw_event() before reaching these checks?

In drivers/hid/hid-oxp.c, __hid_input_report() invokes the driver's
raw_event callback oxp_hid_raw_event(), which runs dev_dbg() with
OXP_PACKET_SIZE (64) before dispatching to oxp_hid_raw_event_gen_1() or
oxp_hid_raw_event_gen_2():

drivers/hid/hid-oxp.c:oxp_hid_raw_event() {
	u16 up = get_usage_page(hdev);

	dev_dbg(&hdev->dev, "raw event data: [%*ph]\n", OXP_PACKET_SIZE, data);

	switch (up) {
	case GEN1_USAGE_PAGE:
		return oxp_hid_raw_event_gen_1(hdev, report, data, size);
	case GEN2_USAGE_PAGE:
		return oxp_hid_raw_event_gen_2(hdev, report, data, size);
	...
}

When a report with size < OXP_PACKET_SIZE is delivered (for example via a
short transfer, hid_bpf_input_report(), or __hid_request() where
hid_alloc_report_buf() allocates hid_report_len(report) + 7 +
(report->id == 0) bytes) and dynamic debugging or DEBUG is enabled, the
dev_dbg() call reads 64 bytes from data regardless of size.

Should the dev_dbg() call in oxp_hid_raw_event() pass size instead of
OXP_PACKET_SIZE (or validate size beforehand)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018213754.1951187-1-tristmd@gmail.com?part=6