Re: [PATCH] HID: saitek: don't dereference a NULL hidinput in saitek_event()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid: UAF in saitek_event() due to dangling field->hidinput on init failure
--

commit c05cbb2256bb191c18315a19fe3d041ae082e349
Author: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

    HID: saitek: don't dereference a NULL hidinput in saitek_event()

    hid_process_event() calls the driver's ->event() for every usage of a
    report, including fields that hidinput did not map to any input device,
    e.g. a constant input item. For those fields field->hidinput is NULL,
    and saitek_event() loads field->hidinput->input unconditionally:

Closes: https://syzkaller.appspot.com/bug?extid=eba2b3201bd25a97cddf

> diff --git a/drivers/hid/hid-saitek.c b/drivers/hid/hid-saitek.c
> index 6fe7c087c594e..2a9e07a4fc77f 100644
> --- a/drivers/hid/hid-saitek.c
> +++ b/drivers/hid/hid-saitek.c
> @@ -151,7 +151,12 @@ static int saitek_event(struct hid_device *hdev, struct hid_field *field,
>  		struct hid_usage *usage, __s32 value)
>  {
>  	struct saitek_sc *ssc = hid_get_drvdata(hdev);
> -	struct input_dev *input = field->hidinput->input;
> +	struct input_dev *input;
> +
> +	if (!field->hidinput)
> +		return 0;
> +
> +	input = field->hidinput->input;

[Severity: High]
Can this still cause a use-after-free if hidinput_connect() fails during
probe?

If hidinput_connect() fails (for example, if input_register_device() fails),
it calls hidinput_disconnect() which frees the hid_input structures:

drivers/hid/hid-input.c:hidinput_disconnect() {
    ...
	list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
		list_del(&hidinput->list);
		...
		kfree(hidinput);
	}
    ...
}

However, it does not clear the field->hidinput pointers in the report fields.

Because the Saitek driver defines a ->raw_event callback, hid_connect()
tolerates the input connection failure and returns success. The probe sequence
completes successfully and the device becomes active.

When a report arrives, __hid_input_report() processes it and calls the driver's
->event() callback (saitek_event). The newly added check if (!field->hidinput)
will evaluate to false because the pointer is non-NULL but dangling, leading
to a use-after-free when field->hidinput->input is dereferenced.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015143433.129525-1-ngocthang2710.1999@gmail.com?part=1