Re: [PATCH] Input: reject parent overrides on managed input devices

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Sat, Oct 10, 2026 at 04:04:42AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] input: newly added parent check breaks existing managed drivers
> - [High] input: NULL pointer dereference in input_free_device()
> --
> 
> commit 5c214b0cacbe95369ec05dede9ca4ab2c7e279f1
> Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> Input: reject parent overrides on managed input devices
> 
> This commit adds a strict check to ensure that the parent of a managed
> input device is not overridden or cleared before registration. It returns
> -EINVAL if the parent does not match the original device that allocated
> the managed input device. KUnit tests are also added to verify this
> behavior.
> 
> > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > index 78c10eea7328d..1cca6d48f5977 100644
> > --- a/drivers/input/input.c
> > +++ b/drivers/input/input.c
> > @@ -2396,6 +2396,14 @@ int input_register_device(struct input_dev *dev)
> >  	}
> >  
> >  	if (dev->devres_managed) {
> > +		if (!dev->dev.parent ||
> > +		    !devres_find(dev->dev.parent, devm_input_device_release,
> > +				 devm_input_device_match, dev)) {
> > +			dev_err(&dev->dev,
> > +				"Parent of managed input device was overridden, refusing to register\n");
> > +			return -EINVAL;
> 
> [Severity: High]
> Does this new strict check break existing in-tree drivers that legitimately
> reparent the input device during probe?
> 
> For example, looking at da7280_probe() in drivers/input/misc/da7280.c:
> 
> da7280_probe() {
>     input_dev = devm_input_allocate_device(dev);
>     ...
>     input_dev->dev.parent = client->dev.parent;
>     ...
>     error = input_register_device(input_dev);
> }
> 
> Another example is tps6507x_ts_probe() in drivers/input/touchscreen/tps6507x-ts.c:
> 
> tps6507x_ts_probe() {
>     poll_dev = devm_input_allocate_device(&i2c->dev);
>     ...
>     poll_dev->dev.parent = tsc->dev;
>     ...
>     error = input_register_device(poll_dev);
> }
> 
> Because these drivers explicitly override dev.parent before registration,
> wouldn't this new check unconditionally fail with -EINVAL and break their
> initialization sequence?

No drivers should be doing such overrides. I already posted patches
fixing it up.

> 
> [Severity: High]
> If this error path is triggered, can it lead to a NULL pointer dereference
> or double-free when the driver cleans up?
> 
> When a managed driver's probe fails here, it will typically call
> input_free_device() in its error path.  However, input_free_device() trusts
> dev->dev.parent unconditionally:
> 
> drivers/input/input.c:input_free_device() {
>     ...
>     if (dev->devres_managed)
>         WARN_ON(devres_destroy(dev->dev.parent,
>                                devm_input_device_release,
>                                devm_input_device_match,
>                                dev));
>     ...
> }
> 
> If dev.parent was cleared to NULL before input_register_device() was called,
> wouldn't passing NULL to devres_destroy() cause a crash when it tries to
> lock the devres list?
> 
> Alternatively, if the device was reparented to a different device,
> devres_destroy() would fail to find the devres on the new parent.  Wouldn't
> this fail to untrack the devres, leading to a double-free later when the
> original devm owner is unbound?

Given that drivers should not be doing it I am not worries about this
case. They'll get an error message first.

Thanks.

-- 
Dmitry