Re: [PATCH v5 1/2] HID: haptic: reorder try_module_get/get_device before input_ff_create

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] input: missing EV_FF capability cleanup on failure leads to NULL deref
--

commit 58557bc733a3a68f242cf09872b1a593ec599fc2
Author: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

HID: haptic: reorder try_module_get/get_device before input_ff_create

This commit moves try_module_get() and get_device() before input_ff_create()
in hid_haptic_init() to acquire references early. It adjusts the error
handling to jump to stop_buffer_free directly on failure, removing the need
to restore handlers or call input_ff_destroy().

> diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
> index 8760eeb08b2cd..592b44e020d6b 100644
> --- a/drivers/hid/hid-haptic.c
> +++ b/drivers/hid/hid-haptic.c

[ ... ]

> @@ -499,11 +499,23 @@ int hid_haptic_init(struct hid_device *hdev,
>  
>  	input_set_capability(dev, EV_FF, FF_HAPTIC);
>  
> -	flush = dev->flush;
> -	event = dev->event;
> +	if (!try_module_get(THIS_MODULE)) {
> +		dev_err(&hdev->dev, "Failed to increase module count.\n");
> +		ret = -ENODEV;
> +		goto stop_buffer_free;
> +	}

[Severity: High]
Does this error path leave the EV_FF capability set on the input device?

Before this change, the failure paths for try_module_get() and get_device()
jumped to input_free, which called input_ff_destroy(dev).

By jumping to stop_buffer_free instead, the EV_FF capability remains set on
the device, but dev->ff is never initialized.

If hid_haptic_init() returns an error that is ignored by its caller, the
partially initialized input device could be registered.

When a userspace application opens the input event node and issues a force
feedback ioctl, could the input core's input_ff_upload() dereference a null
pointer?

For example, in drivers/input/ff-core.c:input_ff_upload():

        if (!test_bit(effect->type, ff->ffbit)) {

Because test_bit(EV_FF, dev->evbit) is true, the input core might assume
dev->ff is valid (where ff aliases dev->ff) and cause a kernel panic.

> +	if (!get_device(&hdev->dev)) {
> +		dev_err(&hdev->dev, "Failed to get hdev device.\n");
> +		ret = -ENODEV;
> +		module_put(THIS_MODULE);
> +		goto stop_buffer_free;
> +	}
> +
>  	ret = input_ff_create(dev, FF_MAX_EFFECTS);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016135516.2957381-1-qwe.aldo@gmail.com?part=1