Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] Input: synaptics-rmi4 - fix GPF in suspend and resume when unbound
@ 2026-08-06  5:44 Dmitry Torokhov
  2026-08-06  5:57 ` sashiko-bot
  2026-08-06 18:39 ` Dmitry Torokhov
  0 siblings, 2 replies; 3+ messages in thread
From: Dmitry Torokhov @ 2026-08-06  5:44 UTC (permalink / raw)
  To: linux-input; +Cc: Andrew Duggan, linux-kernel

Transport drivers (such as rmi_i2c and rmi_spi) invoke
rmi_driver_suspend() and rmi_driver_resume() on their child rmi_dev
device during system power management events. However, transport drivers
are fully registered and operational even if the physical RMI driver
failed to bind or probe the rmi_dev device.

When rmi_driver_suspend() or rmi_driver_resume() is called on an unbound
rmi_dev, dev_get_drvdata() returns NULL. Calling rmi_disable_irq() or
rmi_enable_irq() without driver data attached causes a NULL pointer
dereference and General Protection Fault when attempting to lock
data->enabled_mutex.

Fix this by checking if driver data is attached to rmi_dev in
rmi_driver_suspend() and rmi_driver_resume(), exiting early if
no driver data is present.

Fixes: 2b6a321da9a2 ("Input: synaptics-rmi4 - add support for Synaptics RMI4 devices")
Reported-by: syzbot+09103639e39c989e3ed3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=09103639e39c989e3ed3
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.6-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_driver.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 5d49a9021c7d..a349dfd17519 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -991,6 +991,15 @@ int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
 {
 	int retval;
 
+	/*
+	 * Transport driver will try to suspend RMI device even if physical
+	 * driver did not bind to the RMI device, because transport device
+	 * (I2C, SPI) is fully registered and operational. Exit early if
+	 * there is no driver data attached to the RMI device.
+	 */
+	if (!dev_get_drvdata(&rmi_dev->dev))
+		return 0;
+
 	retval = rmi_suspend_functions(rmi_dev);
 	if (retval)
 		dev_warn(&rmi_dev->dev, "Failed to suspend functions: %d\n",
@@ -1005,6 +1014,10 @@ int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake)
 {
 	int retval;
 
+	/* Skip if not fully bound to RMI driver */
+	if (!dev_get_drvdata(&rmi_dev->dev))
+		return 0;
+
 	rmi_enable_irq(rmi_dev, clear_wake);
 
 	retval = rmi_resume_functions(rmi_dev);
-- 
2.55.0.679.g6767b8d81c-goog


-- 
Dmitry

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - fix GPF in suspend and resume when unbound
  2026-08-06  5:44 [PATCH] Input: synaptics-rmi4 - fix GPF in suspend and resume when unbound Dmitry Torokhov
@ 2026-08-06  5:57 ` sashiko-bot
  2026-08-06 18:39 ` Dmitry Torokhov
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-06  5:57 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: dmitry.torokhov, linux-input

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

Pre-existing issues:
- [High] The exported function `rmi_set_attn_data()` dereferences `drvdata` without a NULL check, leading to a potential NULL pointer dereference if the physical device probe fails.
- [High] If `rmi_driver_probe()` fails after `rmi_f34_create_sysfs()` succeeds, the error path fails to call `rmi_f34_remove_sysfs()`, leaking the sysfs group. Accessing the leaked sysfs files causes a NULL pointer dereference.
- [Medium] If `rmi_suspend_functions()` fails, `rmi_driver_suspend()` and `rmi_i2c_suspend()` disable IRQs and regulators, but propagate the error to the PM core. The PM core aborts suspend and does not call `resume` for this device, leaving it permanently powered off.
- [Low] Misleading copy-paste error messages in suspend and resume paths.
--

--- Patch [1]: [PATCH] Input: synaptics-rmi4 - fix GPF in suspend and resume when unbound ---
commit 19d89c520c8ddb566dee79a7cfbffc9acc8839a2
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: synaptics-rmi4 - fix GPF in suspend and resume when unbound

Transport drivers invoke rmi_driver_suspend() and rmi_driver_resume()
on their child rmi_dev device during power management events. This
prevents a GPF by checking if driver data is attached, exiting early
if not present.

Closes: https://syzkaller.appspot.com/bug?extid=09103639e39c989e3ed3

> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 5d49a9021c7d0..a349dfd17519c 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -991,6 +991,15 @@ int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
>  {
>  	int retval;
>  
> +	/*
> +	 * Transport driver will try to suspend RMI device even if physical
> +	 * driver did not bind to the RMI device, because transport device
> +	 * (I2C, SPI) is fully registered and operational. Exit early if
> +	 * there is no driver data attached to the RMI device.
> +	 */
> +	if (!dev_get_drvdata(&rmi_dev->dev))
> +		return 0;
> +
>  	retval = rmi_suspend_functions(rmi_dev);

[Severity: Medium]
This isn't a bug introduced by this patch, but looking at the rest of
rmi_driver_suspend():

rmi_driver_suspend() {
    ...
    retval = rmi_suspend_functions(rmi_dev);
    if (retval)
        dev_warn(&rmi_dev->dev, "Failed to suspend functions: %d\n", retval);
    
    rmi_disable_irq(rmi_dev, enable_wake);
    return retval;
}

If rmi_suspend_functions() fails, the error path still disables the IRQ
and returns the error to the PM core. The I2C transport driver then
unconditionally disables regulators in rmi_i2c_suspend() and also
propagates the error.

Since the PM core aborts suspend and skips calling resume for devices
that return an error, does this leave the device permanently disabled and
unresponsive?

> @@ -1005,6 +1014,10 @@ int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake)
>  {
>  	int retval;
>  
> +	/* Skip if not fully bound to RMI driver */
> +	if (!dev_get_drvdata(&rmi_dev->dev))
> +		return 0;
> +
>  	rmi_enable_irq(rmi_dev, clear_wake);
>  
>  	retval = rmi_resume_functions(rmi_dev);

[Severity: Low]
This is a pre-existing issue, but further down in rmi_driver_resume() there
appears to be a copy-paste error in the warning message:

rmi_driver_resume() {
    ...
    retval = rmi_resume_functions(rmi_dev);
    if (retval)
        dev_warn(&rmi_dev->dev, "Failed to suspend functions: %d\n",
            retval);
    ...
}

Should this warning say "Failed to resume functions"? Similarly, in
rmi_i2c_suspend(), there is a warning that says "Failed to resume device"
when suspend fails.

[Severity: High]
This is a pre-existing issue, but since this patch addresses NULL pointer
dereferences from unbound devices, is there a similar vulnerability in
rmi_set_attn_data()?

rmi_set_attn_data() {
    struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
    
    if (!drvdata->enabled)
        return;
    ...
}

If rmi_driver_probe() fails, dev_get_drvdata() is cleared to NULL. However,
the HID transport driver (hid-rmi) sets the RMI_STARTED bit after successful
transport registration, independent of the physical driver probe.

If hardware sends events or malicious data is provided after a probe failure,
can rmi_input_event() bypass the RMI_STARTED check and call
rmi_set_attn_data(), leading to a NULL pointer dereference when accessing
drvdata->enabled?

[Severity: High]
This is another pre-existing issue, but looking at the error handling in
rmi_driver_probe():

rmi_driver_probe() {
    ...
    err_destroy_functions:
        rmi_free_function_list(rmi_dev);
    err:
        return retval;
}

If rmi_driver_probe() fails after successfully calling
rmi_f34_create_sysfs(), it jumps to err_destroy_functions but misses a
call to rmi_f34_remove_sysfs().

Does this leak the sysfs group? If the sysfs group is leaked and the driver
core clears drvdata, could a later user-space read to the world-readable
update_fw_status sysfs attribute trigger a NULL pointer dereference in
rmi_driver_update_fw_status_show() when it checks data->f34_container?

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - fix GPF in suspend and resume when unbound
  2026-08-06  5:44 [PATCH] Input: synaptics-rmi4 - fix GPF in suspend and resume when unbound Dmitry Torokhov
  2026-08-06  5:57 ` sashiko-bot
@ 2026-08-06 18:39 ` Dmitry Torokhov
  1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Torokhov @ 2026-08-06 18:39 UTC (permalink / raw)
  To: linux-input; +Cc: Andrew Duggan, linux-kernel

On Wed, Aug 05, 2026 at 10:44:23PM -0700, Dmitry Torokhov wrote:
> Transport drivers (such as rmi_i2c and rmi_spi) invoke
> rmi_driver_suspend() and rmi_driver_resume() on their child rmi_dev
> device during system power management events. However, transport drivers
> are fully registered and operational even if the physical RMI driver
> failed to bind or probe the rmi_dev device.
> 
> When rmi_driver_suspend() or rmi_driver_resume() is called on an unbound
> rmi_dev, dev_get_drvdata() returns NULL. Calling rmi_disable_irq() or
> rmi_enable_irq() without driver data attached causes a NULL pointer
> dereference and General Protection Fault when attempting to lock
> data->enabled_mutex.
> 
> Fix this by checking if driver data is attached to rmi_dev in
> rmi_driver_suspend() and rmi_driver_resume(), exiting early if
> no driver data is present.

Note that better fix would be to have RMI driver implement PM methods
(suspend, resume) but that requires more careful planning, especially
around RMI-HID.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-06 18:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06  5:44 [PATCH] Input: synaptics-rmi4 - fix GPF in suspend and resume when unbound Dmitry Torokhov
2026-08-06  5:57 ` sashiko-bot
2026-08-06 18:39 ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox