Re: [PATCH v7 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: FF_HAPTIC capability lost due to input_set_capability() reordering
--

commit 212e1315f3df5903b889706323b41c9477dd030e
Author: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

HID: haptic: reorder try_module_get/get_device before input_ff_create

Move try_module_get() and get_device() before input_ff_create() in
hid_haptic_init(). The original order placed them after input_ff_create(),
and failures went to input_free which called input_ff_destroy() without
ever setting ret.

> diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
> index 8760eeb08b2cd..9753bbff9d0d0 100644
> --- a/drivers/hid/hid-haptic.c
> +++ b/drivers/hid/hid-haptic.c
[ ... ]
> @@ -499,39 +497,36 @@ int hid_haptic_init(struct hid_device *hdev,
>  	fill_effect_buf(haptic, &stop_effect, &haptic->stop_effect,
>  			HID_HAPTIC_ORDINAL_WAVEFORMSTOP);
>  
> -	input_set_capability(dev, EV_FF, FF_HAPTIC);
> +	if (!try_module_get(THIS_MODULE)) {
> +		dev_err(&hdev->dev, "Failed to increase module count.\n");
> +		ret = -ENODEV;
> +		goto stop_buffer_free;
> +	}
> +	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;
> +	}
>  
> -	flush = dev->flush;
> -	event = dev->event;
>  	ret = input_ff_create(dev, FF_MAX_EFFECTS);
>  	if (ret) {
>  		dev_err(&hdev->dev, "Failed to create ff device.\n");
> +		put_device(&hdev->dev);
> +		module_put(THIS_MODULE);
>  		goto stop_buffer_free;
>  	}
>  
> +	input_set_capability(dev, EV_FF, FF_HAPTIC);

[Severity: High]
Does moving this call after input_ff_create() break the haptic functionality?

Looking at input_ff_create() in drivers/input/ff-core.c, it iterates over
dev->ffbit and copies the supported effects into the newly created ff device:

	/* Copy "true" bits into ff device bitmap */
	for_each_set_bit(i, dev->ffbit, FF_CNT)
		__set_bit(i, ff->ffbit);

If input_set_capability() is called after input_ff_create(), FF_HAPTIC won't be
present in dev->ffbit during the copy. This leaves the internal state incomplete
because ff->ffbit will lack the capability, causing the input core to reject
haptic effects when userspace attempts to upload them.

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