[PATCH] input: synaptics_i2c - cancel delayed work before freeing device

[PATCH] input: synaptics_i2c - cancel delayed work before freeing device

[Minseong Kim]

synaptics_i2c_irq() schedules touch->dwork via mod_delayed_work().
The delayed work performs I2C transactions and may still be running
(or get queued) when the device is removed.

synaptics_i2c_remove() currently frees 'touch' without canceling
touch->dwork. If removal happens while the work is pending/running,
the work handler may dereference freed memory, leading to a potential
use-after-free.

Cancel the delayed work synchronously before unregistering/freeing
the device.

Fixes: eef3e4cab72e Input: add driver for Synaptics I2C touchpad
Reported-by: Minseong Kim <ii4gsp@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Minseong Kim <ii4gsp@gmail.com>
---
 drivers/input/mouse/synaptics_i2c.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/mouse/synaptics_i2c.c b/drivers/input/mouse/synaptics_i2c.c
index a0d707e47d93..fe30bf9aea3a 100644
--- a/drivers/input/mouse/synaptics_i2c.c
+++ b/drivers/input/mouse/synaptics_i2c.c
@@ -593,6 +593,8 @@ static void synaptics_i2c_remove(struct i2c_client *client)
 	if (!polling_req)
 		free_irq(client->irq, touch);
 
+	cancel_delayed_work_sync(&touch->dwork);
+
 	input_unregister_device(touch->input);
 	kfree(touch);
 }
-- 
2.39.5

Re: [PATCH] input: synaptics_i2c - cancel delayed work before freeing device

[Dmitry Torokhov]

Hi Minseong,

On Wed, Dec 10, 2025 at 12:20:27PM +0900, Minseong Kim wrote:
> synaptics_i2c_irq() schedules touch->dwork via mod_delayed_work().
> The delayed work performs I2C transactions and may still be running
> (or get queued) when the device is removed.
> 
> synaptics_i2c_remove() currently frees 'touch' without canceling
> touch->dwork. If removal happens while the work is pending/running,
> the work handler may dereference freed memory, leading to a potential
> use-after-free.
> 
> Cancel the delayed work synchronously before unregistering/freeing
> the device.
> 
> Fixes: eef3e4cab72e Input: add driver for Synaptics I2C touchpad
> Reported-by: Minseong Kim <ii4gsp@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Minseong Kim <ii4gsp@gmail.com>
> ---
>  drivers/input/mouse/synaptics_i2c.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/input/mouse/synaptics_i2c.c b/drivers/input/mouse/synaptics_i2c.c
> index a0d707e47d93..fe30bf9aea3a 100644
> --- a/drivers/input/mouse/synaptics_i2c.c
> +++ b/drivers/input/mouse/synaptics_i2c.c
> @@ -593,6 +593,8 @@ static void synaptics_i2c_remove(struct i2c_client *client)
>  	if (!polling_req)
>  		free_irq(client->irq, touch);
>  
> +	cancel_delayed_work_sync(&touch->dwork);
> +

The call to cancel_delayed_work_sync() happens in the close() handler
for the device. I see that in resume we restart the polling without
checking if the device is opened, so if we want to fix it we should add
the checks there.

However support for the PXA board using in the device with this touch
controller (eXeda) was removed a while ago. Mike, you're one of the
authors, any objections to simply removing the driver? 

Thanks.

-- 
Dmitry

Re: [PATCH] input: synaptics_i2c - cancel delayed work before freeing device

[Minseong Kim]

Hi Dmitry,

Thanks for the review.

Understood that cancel_delayed_work_sync() is already called from the
close() handler, and that resume() can restart polling regardless of
open state. If we keep this driver, I can send a v2 that adds an open-state
guard in resume().

However, if this driver is no longer used and Mike confirms there are no
remaining users, I have no objections to removing it instead.

Thanks,
Minseong

Re: [PATCH] input: synaptics_i2c - cancel delayed work before freeing device

[Mike Rapoport]

Hi,

On Tue, Dec 09, 2025 at 08:40:54PM -0800, Dmitry Torokhov wrote:
> Hi Minseong,
> 
> On Wed, Dec 10, 2025 at 12:20:27PM +0900, Minseong Kim wrote:
> > synaptics_i2c_irq() schedules touch->dwork via mod_delayed_work().
> > The delayed work performs I2C transactions and may still be running
> > (or get queued) when the device is removed.
> > 
> > synaptics_i2c_remove() currently frees 'touch' without canceling
> > touch->dwork. If removal happens while the work is pending/running,
> > the work handler may dereference freed memory, leading to a potential
> > use-after-free.
> > 
> > Cancel the delayed work synchronously before unregistering/freeing
> > the device.
> > 
> > Fixes: eef3e4cab72e Input: add driver for Synaptics I2C touchpad
> > Reported-by: Minseong Kim <ii4gsp@gmail.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Minseong Kim <ii4gsp@gmail.com>
> > ---
> >  drivers/input/mouse/synaptics_i2c.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/input/mouse/synaptics_i2c.c b/drivers/input/mouse/synaptics_i2c.c
> > index a0d707e47d93..fe30bf9aea3a 100644
> > --- a/drivers/input/mouse/synaptics_i2c.c
> > +++ b/drivers/input/mouse/synaptics_i2c.c
> > @@ -593,6 +593,8 @@ static void synaptics_i2c_remove(struct i2c_client *client)
> >  	if (!polling_req)
> >  		free_irq(client->irq, touch);
> >  
> > +	cancel_delayed_work_sync(&touch->dwork);
> > +
> 
> The call to cancel_delayed_work_sync() happens in the close() handler
> for the device. I see that in resume we restart the polling without
> checking if the device is opened, so if we want to fix it we should add
> the checks there.
> 
> However support for the PXA board using in the device with this touch
> controller (eXeda) was removed a while ago. Mike, you're one of the
> authors, any objections to simply removing the driver? 
 
No objections from my side.

> Thanks.
> 
> -- 
> Dmitry

-- 
Sincerely yours,
Mike.

Re: [PATCH] input: synaptics_i2c - cancel delayed work before freeing device

[Dmitry Torokhov]

On Wed, Dec 10, 2025 at 09:25:38PM +0900, Mike Rapoport wrote:
> Hi,
> 
> On Tue, Dec 09, 2025 at 08:40:54PM -0800, Dmitry Torokhov wrote:
> > Hi Minseong,
> > 
> > On Wed, Dec 10, 2025 at 12:20:27PM +0900, Minseong Kim wrote:
> > > synaptics_i2c_irq() schedules touch->dwork via mod_delayed_work().
> > > The delayed work performs I2C transactions and may still be running
> > > (or get queued) when the device is removed.
> > > 
> > > synaptics_i2c_remove() currently frees 'touch' without canceling
> > > touch->dwork. If removal happens while the work is pending/running,
> > > the work handler may dereference freed memory, leading to a potential
> > > use-after-free.
> > > 
> > > Cancel the delayed work synchronously before unregistering/freeing
> > > the device.
> > > 
> > > Fixes: eef3e4cab72e Input: add driver for Synaptics I2C touchpad
> > > Reported-by: Minseong Kim <ii4gsp@gmail.com>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Minseong Kim <ii4gsp@gmail.com>
> > > ---
> > >  drivers/input/mouse/synaptics_i2c.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/input/mouse/synaptics_i2c.c b/drivers/input/mouse/synaptics_i2c.c
> > > index a0d707e47d93..fe30bf9aea3a 100644
> > > --- a/drivers/input/mouse/synaptics_i2c.c
> > > +++ b/drivers/input/mouse/synaptics_i2c.c
> > > @@ -593,6 +593,8 @@ static void synaptics_i2c_remove(struct i2c_client *client)
> > >  	if (!polling_req)
> > >  		free_irq(client->irq, touch);
> > >  
> > > +	cancel_delayed_work_sync(&touch->dwork);
> > > +
> > 
> > The call to cancel_delayed_work_sync() happens in the close() handler
> > for the device. I see that in resume we restart the polling without
> > checking if the device is opened, so if we want to fix it we should add
> > the checks there.
> > 
> > However support for the PXA board using in the device with this touch
> > controller (eXeda) was removed a while ago. Mike, you're one of the
> > authors, any objections to simply removing the driver? 
>  
> No objections from my side.

Hmm, it looks like it is still referenced from
arch/arm/boot/dts/nxp/mxs/imx23-sansa.dts

Marek, is this device still relevant?

Thanks.

-- 
Dmitry