Re: [PATCH 8/8] HID: prodikeys: validate report size in raw_event callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] HID: prodikeys: pk_raw_event() drops 3-byte report 0x03 MIDI packets
--

commit 2a171eaec1ee7f4c5cfc52b5bb2b8e59c517ae46
Author: Tristan Madani <tristan@talencesecurity.com>

HID: prodikeys: validate report size in raw_event callback

This patch adds a minimum report size check at the beginning of
pk_raw_event() to prevent out-of-bounds reads on short reports.

> diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c
> index fba01e4fcab1c..c1419e6703777 100644
> --- a/drivers/hid/hid-prodikeys.c
> +++ b/drivers/hid/hid-prodikeys.c
> @@ -768,6 +768,9 @@ static int pk_raw_event(struct hid_device *hdev, struct hid_report *report,
>  	struct pcmidi_snd *pm = hid_get_drvdata(hdev);
>  	int ret = 0;
>  
> +	if (size < 4)
> +		return 0;
> +
>  	if (1 == pm->ifnum) {
>  		if (report->id == data[0])
>  			switch (report->id) {

[Severity: Medium]
Could checking size < 4 before inspecting report->id drop valid 3-byte
packets for report 0x03?

In drivers/hid/hid-prodikeys.c, pk_raw_event() calls pcmidi_handle_report(),
which dispatches three report IDs:

	switch (report_id) {
	case 0x01: /* midi keys (qwerty)*/
		ret = pcmidi_handle_report1(pm, data);
		break;
	case 0x03: /* midi keyboard (musical)*/
		ret = pcmidi_handle_report3(pm, data, size);
		break;
	case 0x04: /* multimedia/midi keys (qwerty)*/
		ret = pcmidi_handle_report4(pm, data);
		break;
	}

While pcmidi_handle_report1() and pcmidi_handle_report4() unconditionally
read a 24-bit bitmask from data[1..3] and require size >= 4,
pcmidi_handle_report3() computes the number of note/velocity pairs from
size:

	unsigned num_notes = (size-1)/2;
	for (j = 0; j < num_notes; j++)	{
		note = data[j*2+1];
		velocity = data[j*2+2];

A single-note report 0x03 has a 1-byte report ID followed by one 2-byte
note/velocity pair (size == 3), which pcmidi_handle_report3() already bounds
checks via num_notes.

Should the size < 4 check be restricted to reports 0x01 and 0x04 so that
3-byte report 0x03 MIDI events are not lost?

>
>
>

[]▷

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