Re: [PATCH v2 2/2] Input: aw8697 - Add Awinic AW8697 haptics driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] input: UAF in aw8697_play_work due to missing devm cancellation action
- [Low] i2c: missing named initializer for i2c_device_id array
--

commit 78f66ce5a94c007171954aaef47a26794e665c94
Author: Armandas Kvietkus <armandas.kvietkus@proton.me>

Input: aw8697 - Add Awinic AW8697 haptics driver

Adds a driver for the Awinic AW8697 LRA haptic driver IC. It supports
force feedback rumble and calibrates the LRA resonance at probe.

> diff --git a/drivers/input/misc/aw8697.c b/drivers/input/misc/aw8697.c
> new file mode 100644
> index 0000000000000..96c0205089fd0
> --- /dev/null
> +++ b/drivers/input/misc/aw8697.c

[ ... ]

> +static int aw8697_probe(struct i2c_client *client)
> +{

[ ... ]

> +	err = devm_request_threaded_irq(dev, client->irq, NULL, aw8697_irq,
> +					IRQF_ONESHOT, NULL, haptics);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to request irq\n");
> +
> +	INIT_WORK(&haptics->play_work, aw8697_play_work);
> +
> +	haptics->input_dev = devm_input_allocate_device(dev);

[Severity: High]
Could this lead to a use-after-free of the haptics structure if the driver
is unbound while an effect is active?

The workqueue is initialized here, but no devm action is registered to
guarantee cancellation after the input device is fully unregistered.

If the driver is unbound, devm automatically calls input_unregister_device(),
which triggers aw8697_close() and cancels the work. However, the
force-feedback memless timer is only stopped later in
input_unregister_device() when it calls the destroy callback.

In this narrow window, the timer can still fire and call aw8697_play_effect(),
which reschedules haptics->play_work. Because there is no devm action to
cancel the work queue after the input device is fully unregistered, the
rescheduled work persists after devm frees the haptics structure, causing a
use-after-free when the work executes.

> +	if (!haptics->input_dev)
> +		return -ENOMEM;

[ ... ]

> +static DEFINE_SIMPLE_DEV_PM_OPS(aw8697_pm_ops, aw8697_suspend, aw8697_resume);
> +
> +static const struct i2c_device_id aw8697_id[] = {
> +	{ "aw8697" },

[Severity: Low]
Does this array violate the I2C subsystem rule mandating named initializers?
It seems this should be written as { .name = "aw8697" }.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, aw8697_id);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-mainlining-v2-0-40bb1b3943db@proton.me?part=2