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

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

[Dmitry Torokhov]

When an input device is allocated with devm_input_allocate_device(), the
owner struct device is set as dev->dev.parent and holds the
devm_input_device_release resource. Later, input_register_device()
attaches the devm_input_device_unregister resource to dev->dev.parent.

Clearing or overriding dev->dev.parent before registration splits devres
ownership across different devices, which breaks managed lifetime
assumptions.

Verify via devres_find() that dev->dev.parent is non-NULL and still
holds the matching devm_input_device_release entry before registering a
managed input device, rejecting registration with -EINVAL otherwise.

Also add KUnit test coverage in input_test.c verifying that reparented
and NULL-parent managed input devices are rejected while preserving the
owner parent succeeds.

Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c            |  8 ++++++++
 drivers/input/tests/input_test.c | 34 ++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index ac0369414687..76ed9ff40584 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -2417,6 +2417,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;
+		}
+
 		devres = devres_alloc(devm_input_device_unregister,
 				      sizeof(*devres), GFP_KERNEL);
 		if (!devres)
diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
index e105ce71a920..b8c8f04c041b 100644
--- a/drivers/input/tests/input_test.c
+++ b/drivers/input/tests/input_test.c
@@ -8,6 +8,7 @@
 #include <linux/delay.h>
 #include <linux/input.h>
 
+#include <kunit/device.h>
 #include <kunit/test.h>
 
 #define POLL_INTERVAL 100
@@ -161,11 +162,44 @@ static void input_test_grab(struct kunit *test)
 	input_put_device(input_dev);
 }
 
+static void input_test_managed_parent_override(struct kunit *test)
+{
+	struct device *owner, *other_dev;
+	struct input_dev *input_dev;
+
+	owner = kunit_device_register(test, "input-test-owner");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, owner);
+
+	other_dev = kunit_device_register(test, "input-test-other");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, other_dev);
+
+	/* Overriding dev.parent to another device must be rejected */
+	input_dev = devm_input_allocate_device(owner);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, input_dev);
+	input_dev->name = "Managed input device (reparented)";
+	input_dev->dev.parent = other_dev;
+	KUNIT_EXPECT_EQ(test, input_register_device(input_dev), -EINVAL);
+
+	/* Clearing dev.parent to NULL must be rejected */
+	input_dev = devm_input_allocate_device(owner);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, input_dev);
+	input_dev->name = "Managed input device (null parent)";
+	input_dev->dev.parent = NULL;
+	KUNIT_EXPECT_EQ(test, input_register_device(input_dev), -EINVAL);
+
+	/* Preserving original owner as dev.parent succeeds */
+	input_dev = devm_input_allocate_device(owner);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, input_dev);
+	input_dev->name = "Managed input device (valid parent)";
+	KUNIT_EXPECT_EQ(test, input_register_device(input_dev), 0);
+}
+
 static struct kunit_case input_tests[] = {
 	KUNIT_CASE(input_test_polling),
 	KUNIT_CASE(input_test_timestamp),
 	KUNIT_CASE(input_test_match_device_id),
 	KUNIT_CASE(input_test_grab),
+	KUNIT_CASE(input_test_managed_parent_override),
 	{ /* sentinel */ }
 };
 
-- 
2.56.0.385.gd3acb90ef8-goog


-- 
Dmitry

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

[sashiko-bot]

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?

[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?

> +		}
> +
>  		devres = devres_alloc(devm_input_device_unregister,
>  				      sizeof(*devres), GFP_KERNEL);
>  		if (!devres)

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/asmCc1VxVPLIMgY6@google.com?part=1

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

[Dmitry Torokhov]

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

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

[Christian Lamparter]

On Sat, Oct 10, 2026 at 5:54 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> When an input device is allocated with devm_input_allocate_device(), the
> owner struct device is set as dev->dev.parent and holds the
> devm_input_device_release resource. Later, input_register_device()
> attaches the devm_input_device_unregister resource to dev->dev.parent.

Ok, why would this be a problem? If something other than the devres
garbage collection
callback access the data after it was unregistered... what else is
supposed to happen
other than a crash.

for what's worth: this patch seems to be a "spite" response to this:
https://lore.kernel.org/linux-wireless/20261008-carl9170-reverts-v2-0-3ecb12089797@gmail.com/

I think someone else is working on a separate fix:
https://lwn.net/Articles/1038523/: "Revocable references for transient devices"
With todays Patch-Series V14 on: https://lkml.org/lkml/2026/10/11/475

From what I can tell, this should have been what devres was trying to solve.

Cheers,
Christian

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

[Dmitry Torokhov]

On Sun, Oct 11, 2026 at 08:12:47PM +0200, Christian Lamparter wrote:
> On Sat, Oct 10, 2026 at 5:54 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > When an input device is allocated with devm_input_allocate_device(), the
> > owner struct device is set as dev->dev.parent and holds the
> > devm_input_device_release resource. Later, input_register_device()
> > attaches the devm_input_device_unregister resource to dev->dev.parent.
> 
> Ok, why would this be a problem? If something other than the devres
> garbage collection
> callback access the data after it was unregistered... what else is
> supposed to happen
> other than a crash.

If the owner is overridden one possibility is that input device stays
registered while the rest of resources have already been released. Quite
often it is "name" and "phys" pointers. Since the input device stays
registered, userspace can access it, which results in UAF scenarios.
They are reported by syzbot and may serve as an attack vector on a
device.

> 
> for what's worth: this patch seems to be a "spite" response to this:
> https://lore.kernel.org/linux-wireless/20261008-carl9170-reverts-v2-0-3ecb12089797@gmail.com/

Heh, no. I was simply tracking a syzbot report and I have identified 5
drivers that do the wrong thing. 4 of them are input subsystem and I
posted patches and will pick them up through my tree, and one is that
wireless driver that I posed patches (reverts) fixing as well.

I want to make sure we do not introduce any more such cases, so I am
adding a runtime check that will fire if someone will try to add another
such case.

Frankly I do not really care how you will fix your driver. I posted the
patches that should resolve the issue, you are free to take them or come
up with your own. This patch will take care of the issue from my end. 

> 
> I think someone else is working on a separate fix:
> https://lwn.net/Articles/1038523/: "Revocable references for transient devices"
> With todays Patch-Series V14 on: https://lkml.org/lkml/2026/10/11/475
> 
> From what I can tell, this should have been what devres was trying to solve.

No, not really. It solves different use case altogether.

Thanks.

-- 
Dmitry