[PATCH] Input: maplemouse - fix NULL pointer dereference in open()

[PATCH] Input: maplemouse - fix NULL pointer dereference in open()

[Florian Fuchs]

Commit 555c765b0cc2 ("Input: mouse - drop unnecessary calls to
input_set_drvdata") dropped the input_set_drvdata() call in probe
because the data appeared to be unused. However, dc_mouse_open() and
dc_mouse_close() were using maple_get_drvdata(to_maple_dev(&dev->dev)).
This actually retrieves driver data from the input device's embedded
struct device. After input_set_drvdata() was removed, that lookup started
returning NULL and opening the input device dereferences mse->mdev.

Restore input_set_drvdata() and convert open() and close() to use
input_get_drvdata() so the dependency is no longer hidden.

Fixes: 555c765b0cc2 ("Input: mouse - drop unnecessary calls to input_set_drvdata")
Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
---
This fix was tested on the target platform. The following is the error I
get, when using the unpatched kernel:

BUG: unable to handle kernel NULL pointer dereference at 00000004
PC: [<8c26eec4>] dc_mouse_open+0xc/0x28
pgd = f700ee57
[00000004] *pgd=00000000
Oops: 0000 [#1]

CPU: 0 UID: 0 PID: 45 Comm: Xfbdev Not tainted 7.1.1 #84 PREEMPT
PC is at dc_mouse_open+0xc/0x28
PR is at input_open_device+0x7c/0xe0
PC  : 8c26eec4 SP  : 8c7bbd9c SR  : 40008100 TEA : 00000004
R0  : 8c26eeb8 R1  : 00000000 R2  : 00000001 R3  : 00000000
R4  : 8c6b0dc0 R5  : 8c26efa8 R6  : 8c7b64c0 R7  : 00000200
R8  : 00000000 R9  : 8c6b0d70 R10 : 8c6b0c00 R11 : 8c6ce604
R12 : 8c390a64 R13 : 8c6b0d3c R14 : 8c0e9ba0
MACH: 00000006 MACL: 8686868d GBR : 29609ff4 PR  : 8c265fc8

Call trace:
 [<8c265fc8>] input_open_device+0x7c/0xe0
 [<8c26b2d0>] mousedev_open_device+0x38/0x68
 [<8c26b77c>] mousedev_open+0xa4/0x110
 [<8c0e9cc6>] chrdev_open+0x112/0x15c
 [<8c0e2e42>] do_dentry_open+0x27e/0x2fc
 [<8c0e9bb4>] chrdev_open+0x0/0x15c
 [<8c0f32d2>] path_openat+0x1d2/0x7cc
 [<8c0f3956>] do_file_open+0x8a/0xf0
 [<8c0f3100>] path_openat+0x0/0x7cc
 [<8c1efeac>] strncpy_from_user+0x64/0xe4
 [<8c0ffc7e>] alloc_fd+0x106/0x124
 [<8c0e41ed>] sys_openat2+0xb9/0xbc
 [<8c0e3fc6>] do_sys_openat2+0x76/0xd4
 [<8c0e40ee>] do_sys_open+0x2a/0x54
 [<8c00e25a>] syscall_call+0x18/0x1e
 [<8c0e4118>] sys_open+0x0/0x10

 drivers/input/mouse/maplemouse.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/maplemouse.c b/drivers/input/mouse/maplemouse.c
index c99f7e234219..c41182766538 100644
--- a/drivers/input/mouse/maplemouse.c
+++ b/drivers/input/mouse/maplemouse.c
@@ -48,7 +48,7 @@ static void dc_mouse_callback(struct mapleq *mq)
 
 static int dc_mouse_open(struct input_dev *dev)
 {
-	struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
+	struct dc_mouse *mse = input_get_drvdata(dev);
 
 	maple_getcond_callback(mse->mdev, dc_mouse_callback, HZ/50,
 		MAPLE_FUNC_MOUSE);
@@ -58,7 +58,7 @@ static int dc_mouse_open(struct input_dev *dev)
 
 static void dc_mouse_close(struct input_dev *dev)
 {
-	struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
+	struct dc_mouse *mse = input_get_drvdata(dev);
 
 	maple_getcond_callback(mse->mdev, dc_mouse_callback, 0,
 		MAPLE_FUNC_MOUSE);
@@ -88,6 +88,7 @@ static int probe_maple_mouse(struct device *dev)
 	mse->dev = input_dev;
 	mse->mdev = mdev;
 
+	input_set_drvdata(input_dev, mse);
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
 	input_dev->keybit[BIT_WORD(BTN_MOUSE)] = BIT_MASK(BTN_LEFT) |
 		BIT_MASK(BTN_RIGHT) | BIT_MASK(BTN_MIDDLE);
-- 
2.43.0

Re: [PATCH] Input: maplemouse - fix NULL pointer dereference in open()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Initialization of maple bus driver data occurs after exposing the input device to userspace, leading to a potential NULL pointer dereference.
- [High] Teardown asymmetry causes a race condition where a background callback is inadvertently re-armed during device removal, leading to UAF or NULL pointer dereference.
--

commit 5babe5754528798c9e3476330c1a7834fe83388a
Author: Florian Fuchs <fuchsfl@gmail.com>

Input: maplemouse - fix NULL pointer dereference in open()

Commit 555c765b0cc2 dropped the input_set_drvdata() call in probe, which
caused a NULL pointer dereference in open() because maple_get_drvdata()
was still being used. This patch restores input_set_drvdata() and
converts open() and close() to use input_get_drvdata().

> diff --git a/drivers/input/mouse/maplemouse.c b/drivers/input/mouse/maplemouse.c
> index c99f7e2342191..c41182766538a 100644
> --- a/drivers/input/mouse/maplemouse.c
> +++ b/drivers/input/mouse/maplemouse.c

[ ... ]

> @@ -58,7 +58,7 @@ static int dc_mouse_open(struct input_dev *dev)
>  
>  static void dc_mouse_close(struct input_dev *dev)
>  {
> -	struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
> +	struct dc_mouse *mse = input_get_drvdata(dev);
>  
>  	maple_getcond_callback(mse->mdev, dc_mouse_callback, 0,
>  		MAPLE_FUNC_MOUSE);
>  }

[Severity: High]
This is a pre-existing issue, but does calling maple_getcond_callback() here
create a race condition during device removal?

During removal, remove_maple_mouse() clears the callback before unregistering
the device:

drivers/input/mouse/maplemouse.c:remove_maple_mouse() {
	...
	mdev->callback = NULL;
	input_unregister_device(mse->dev);
	...
}

If the device is open, input_unregister_device() synchronously calls
dc_mouse_close(). This unconditionally calls maple_getcond_callback(), which
re-arms mdev->callback back to dc_mouse_callback.

After input_unregister_device() returns, remove_maple_mouse() clears the
driver data and frees mse. If a delayed maple bus packet arrives after this,
dc_mouse_callback() will execute and attempt to use a freed mse pointer or a
NULL driver data pointer, causing a kernel panic.

Would it be safer to avoid re-arming the callback if the device is being
removed?

[ ... ]

> @@ -88,6 +88,7 @@ static int probe_maple_mouse(struct device *dev)
>  	mse->dev = input_dev;
>  	mse->mdev = mdev;
>  
> +	input_set_drvdata(input_dev, mse);
>  	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
>  	input_dev->keybit[BIT_WORD(BTN_MOUSE)] = BIT_MASK(BTN_LEFT) |
>  		BIT_MASK(BTN_RIGHT) | BIT_MASK(BTN_MIDDLE);

[Severity: High]
This is a pre-existing issue, but is it possible for a callback to fire
before driver data is fully initialized?

Later in probe_maple_mouse(), the device is registered:

drivers/input/mouse/maplemouse.c:probe_maple_mouse() {
	...
	error = input_register_device(input_dev);
	if (error)
		goto fail_register;

	mdev->driver = mdrv;
	maple_set_drvdata(mdev, mse);
	...
}

If userspace immediately opens the device upon registration, dc_mouse_open()
will schedule dc_mouse_callback(). If the callback executes before
maple_set_drvdata() completes, maple_get_drvdata(mapledev) inside the
callback will return NULL, leading to a NULL pointer dereference when
accessing mse->dev.

Could the maple_set_drvdata() call be moved before input_register_device()
to prevent this?

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