Linux Input/HID development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Sang-Heon Jeon" <ekffu200098@gmail.com>
Cc: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org
Subject: Re: [PATCH v2 3/3] Input: synaptics_i2c - return 0 explicitly on success
Date: Wed, 29 Jul 2026 17:16:50 +0000	[thread overview]
Message-ID: <20260729171651.15FDC1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260729171001.260698-4-ekffu200098@gmail.com>

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

Pre-existing issues:
- [High] The delayed work `touch->dwork` is not cancelled on driver unbind or probe failure, leading to a use-after-free.
--

--- Patch [3]: [PATCH v2 3/3] Input: synaptics_i2c - return 0 explicitly on success ---
commit 2cc2d0ff5c54be0c06b9f9251e169b1467e86f9b
Author: Sang-Heon Jeon <ekffu200098@gmail.com>

Input: synaptics_i2c - return 0 explicitly on success

error is always zero at the last return in synaptics_i2c_reg_set().
Explicitly return 0 on the success path instead of returning error,
which is the preferred way when there are multiple failure points.

> diff --git a/drivers/input/mouse/synaptics_i2c.c b/drivers/input/mouse/synaptics_i2c.c
> index d4cf982f12632..66e833974c6da 100644
> --- a/drivers/input/mouse/synaptics_i2c.c
> +++ b/drivers/input/mouse/synaptics_i2c.c
> @@ -261,7 +261,7 @@ static s32 synaptics_i2c_reg_set(struct i2c_client *client, u16 reg, u8 val)
>  	if (error)
>  		return error;
>  
> -	return error;
> +	return 0;
>  }
>  
>  static s32 synaptics_i2c_word_get(struct i2c_client *client, u16 reg)

[Severity: High]
This is a pre-existing issue, but I noticed that the delayed work touch->dwork
doesn't appear to be cancelled on driver unbind or probe failure.

In synaptics_i2c_probe(), interrupts are enabled and devm_request_irq() is
called before input_register_device(). The IRQ handler can immediately schedule
touch->dwork.

If input_register_device() fails:

drivers/input/mouse/synaptics_i2c.c:synaptics_i2c_probe() {
	...
	error = input_register_device(touch->input);
	if (error) {
		dev_err(dev, "Input device register failed: %d\n", error);
		return error;
	}
	...
}

The probe function returns the error directly. Since devres will automatically
free the IRQ and the touch structure, but the pending work on system_dfl_wq is
never cancelled, could the delayed work run later and access the freed touch
memory?

Furthermore, the driver lacks a remove callback entirely:

drivers/input/mouse/synaptics_i2c.c:
static struct i2c_driver synaptics_i2c_driver = {
	.driver = {
		.name	= DRIVER_NAME,
		.of_match_table = of_match_ptr(synaptics_i2c_of_match),
		.pm	= pm_sleep_ptr(&synaptics_i2c_pm),
	},

	.probe		= synaptics_i2c_probe,
	.id_table	= synaptics_i2c_id_table,
};

If the device is unbound via sysfs, devres will automatically tear down
resources. However, the delayed work is only explicitly cancelled if the input
device is currently open (via synaptics_i2c_close()).

If the device was never opened before it is unbound, does this mean the work
is never cancelled before the memory is freed?

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

      reply	other threads:[~2026-07-29 17:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29 17:09 [PATCH v2 0/3] Input: clean up conditional returns with no effect Sang-Heon Jeon
2026-07-29 17:09 ` [PATCH v2 1/3] Input: pmic8xxx-keypad - remove conditional return " Sang-Heon Jeon
2026-07-29 17:09 ` [PATCH v2 2/3] Input: rmi_smbus " Sang-Heon Jeon
2026-07-29 17:17   ` sashiko-bot
2026-07-29 17:09 ` [PATCH v2 3/3] Input: synaptics_i2c - return 0 explicitly on success Sang-Heon Jeon
2026-07-29 17:16   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260729171651.15FDC1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=ekffu200098@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox