All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Input: fix potential use-after-free in input_devices_seq_show
@ 2026-09-28 17:11 Habil Eren Türker
  2026-09-28 17:21 ` sashiko-bot
  0 siblings, 1 reply; 5+ messages in thread
From: Habil Eren Türker @ 2026-09-28 17:11 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: linux-input, linux-kernel, syzbot+bc6b37960b1d13f68f9f,
	Habil Eren Türker

The input_devices_seq_show() function accesses the input_dev structure
while holding input_mutex. However, the device can still be freed
concurrently, leading to a use-after-free.

Fix this by taking a reference to the input device in
input_devices_seq_start() and dropping it in input_devices_seq_stop().

Reported-by: syzbot+bc6b37960b1d13f68f9f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=bc6b37960b1d13f68f9f
Tested-by: Habil Eren Türker <habilerenturker@hotmail.com>
Signed-off-by: Habil Eren Türker <habilerenturker@hotmail.com>
---
Changes in v2:
- Added Reported-by and Closes tags for the Syzbot report.
- No functional changes.

 drivers/input/input.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index cf6fecea7..14a95e816 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1051,6 +1051,7 @@ static __poll_t input_proc_devices_poll(struct file *file, poll_table *wait)
 static void *input_devices_seq_start(struct seq_file *seq, loff_t *pos)
 {
 	struct input_seq_state *state = seq->private;
+	void *v;
 	int error;
 
 	error = mutex_lock_interruptible(&input_mutex);
@@ -1061,7 +1062,11 @@ static void *input_devices_seq_start(struct seq_file *seq, loff_t *pos)
 
 	state->mutex_acquired = true;
 
-	return seq_list_start(&input_dev_list, *pos);
+	v = seq_list_start(&input_dev_list, *pos);
+	if (v)
+		input_get_device(container_of(v, struct input_dev, node));
+
+	return v;
 }
 
 static void *input_devices_seq_next(struct seq_file *seq, void *v, loff_t *pos)
@@ -1077,6 +1082,17 @@ static void input_seq_stop(struct seq_file *seq, void *v)
 		mutex_unlock(&input_mutex);
 }
 
+static void input_devices_seq_stop(struct seq_file *seq, void *v)
+{
+	struct input_seq_state *state = seq->private;
+
+	if (v)
+		input_put_device(container_of(v, struct input_dev, node));
+
+	if (state->mutex_acquired)
+		mutex_unlock(&input_mutex);
+}
+
 static void input_seq_print_bitmap(struct seq_file *seq, const char *name,
 				   unsigned long *bitmap, int max)
 {
@@ -1151,7 +1167,7 @@ static int input_devices_seq_show(struct seq_file *seq, void *v)
 static const struct seq_operations input_devices_seq_ops = {
 	.start	= input_devices_seq_start,
 	.next	= input_devices_seq_next,
-	.stop	= input_seq_stop,
+	.stop	= input_devices_seq_stop,
 	.show	= input_devices_seq_show,
 };
 
-- 
2.47.3


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

* Re: [PATCH v2] Input: fix potential use-after-free in input_devices_seq_show
  2026-09-28 17:11 [PATCH v2] Input: fix potential use-after-free in input_devices_seq_show Habil Eren Türker
@ 2026-09-28 17:21 ` sashiko-bot
  2026-09-28 18:15   ` Habil Eren Türker
  0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-09-28 17:21 UTC (permalink / raw)
  To: Habil Eren Türker; +Cc: linux-input, dmitry.torokhov

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] input: refcount leak and UAF during seq_file iteration
- [Critical] input: invalid pointer dereference in seq_stop on interrupt
--

commit aa10b9792f79b028b976fdcbe93a1680d11f5f5f
Author: Habil Eren Türker <habilerenturker@gmail.com>
Subject: Input: fix potential use-after-free in input_devices_seq_show

This commit attempts to fix a use-after-free in input_devices_seq_show()
by taking a reference to the input device in input_devices_seq_start() and
dropping it in input_devices_seq_stop().

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

> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 78c10eea7328d..6d21718a73ff8 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
[ ... ]
> @@ -1124,7 +1125,11 @@ static void *input_devices_seq_start(struct seq_file *seq, loff_t *pos)
>  
>  	state->mutex_acquired = true;
>  
> -	return seq_list_start(&input_dev_list, *pos);
> +	v = seq_list_start(&input_dev_list, *pos);
> +	if (v)
> +		input_get_device(container_of(v, struct input_dev, node));
> +
> +	return v;
>  }
>  
>  static void *input_devices_seq_next(struct seq_file *seq, void *v, loff_t *pos)

[Severity: Critical]
In input_devices_seq_start(), a reference is acquired on the first device. 
However, input_devices_seq_next() advances the iterator without acquiring a 
reference to the next device or dropping the reference to the previous one.

When iteration stops, input_devices_seq_stop() drops a reference on whatever 
device input_devices_seq_next() yielded last.

Can this cause a refcount underflow (and potential use-after-free) on the
last device, while permanently leaking the reference to the first device?

[ ... ]
> @@ -1140,6 +1145,17 @@ static void input_seq_stop(struct seq_file *seq, void *v)
>  		mutex_unlock(&input_mutex);
>  }
>  
> +static void input_devices_seq_stop(struct seq_file *seq, void *v)
> +{
> +	struct input_seq_state *state = seq->private;
> +
> +	if (v)
> +		input_put_device(container_of(v, struct input_dev, node));

[Severity: Critical]
If input_devices_seq_start() is interrupted by a signal while trying to
acquire input_mutex, it returns ERR_PTR(-EINTR). The seq_file core then
passes this error pointer to input_devices_seq_stop().

Because an error pointer is not NULL, the if (v) check will evaluate to true.

Does this pass an invalid, negative pointer to container_of() and
input_put_device(), potentially crashing the kernel? Should this logic verify
!IS_ERR(v) before dereferencing it?

> +
> +	if (state->mutex_acquired)
> +		mutex_unlock(&input_mutex);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928171306.59206-1-habilerenturker@hotmail.com?part=1

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

* Re: [PATCH v2] Input: fix potential use-after-free in input_devices_seq_show
  2026-09-28 17:21 ` sashiko-bot
@ 2026-09-28 18:15   ` Habil Eren Türker
  2026-09-30 18:15     ` Dmitry Torokhov
  0 siblings, 1 reply; 5+ messages in thread
From: Habil Eren Türker @ 2026-09-28 18:15 UTC (permalink / raw)
  To: sashiko-bot; +Cc: dmitry.torokhov, linux-input, sashiko-reviews

This patch is withdrawn.

Sashiko's assessment is correct. input_mutex already protects the device
during seq_file iteration, so the explicit refcounting is unnecessary.
Additionally, the patch had two bugs: a refcount imbalance in next() and
a missing IS_ERR() check in stop(), which could lead to a kernel panic
or use-after-free.

I will investigate the actual root cause further.

Regards,
Türker

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

* Re: [PATCH v2] Input: fix potential use-after-free in input_devices_seq_show
  2026-09-28 18:15   ` Habil Eren Türker
@ 2026-09-30 18:15     ` Dmitry Torokhov
  2026-10-01 16:04       ` Habil Eren Türker
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2026-09-30 18:15 UTC (permalink / raw)
  To: Habil Eren Türker
  Cc: Habil Eren Türker, sashiko-bot, linux-input, linux-kernel,
	sashiko-reviews

Hi Türker,

On Mon, Sep 28, 2026 at 09:15:24PM +0300, Habil Eren Türker wrote:
> This patch is withdrawn.
>
> Sashiko's assessment is correct. input_mutex already protects the device
> during seq_file iteration, so the explicit refcounting is unnecessary.
> Additionally, the patch had two bugs: a refcount imbalance in next() and
> a missing IS_ERR() check in stop(), which could lead to a kernel panic
> or use-after-free.
>
> I will investigate the actual root cause further.

To save you some time digging through the syzbot reports I had an LLM
scan them and here are the findings:

It is not the struct input_dev itself that is being freed while on
input_dev_list, but rather the strings pointed to by dev->name or
dev->phys when a driver either fails to unregister its input device
before freeing its private data, or leaks an input_dev instance.

Because syzbot groups KASAN crashes by the top frame (string_nocheck() ->
string() -> vsnprintf() -> seq_printf()), the 63 crash reports under
https://syzkaller.appspot.com/bug?extid=bc6b37960b1d13f68f9f actually
belong to three separate bugs:

1. 25 of the 63 reports crash in irq_seq_show() (kernel/irq/proc.c:601)
   when reading /proc/interrupts, which is unrelated to input.

2. 29 of the reports (including the initial syzbot report [1]) crash in
   input_devices_seq_show() on line 1178 when formatting dev->name:

     seq_printf(seq, "N: Name=\"%s\"\n", dev->name ? dev->name : "");

   In all 29 reports, the bad read is at offset 0x758 (1880 bytes) inside
   a kmalloc-2k object. In most runs that slab slot had already exited
   the KASAN quarantine and been reallocated (for example by netlink
   __alloc_skb() or sk_prot_alloc()), masking the original owner.
   However, one report [2] (log [3]) caught the object before reuse:

     Allocated by: redrat3_dev_probe() (drivers/media/rc/redrat3.c:1023)
     Freed by:     redrat3_delete() <- redrat3_dev_probe() (redrat3.c:1124)

   redrat3_init_rc_dev() registered the rc_dev (and its input_dev, with
   input_dev->name pointing to rr3->name at offset 0x758 of struct
   redrat3_dev), and when redrat3_enable_detector() subsequently failed,
   the error path freed rr3 without calling rc_unregister_device(),
   leaving the input_dev registered with a dangling dev->name pointer.
   This is already fixed in linux-next by commit af452b9e0133 ("media:
   redrat3: fix UAF in probe error path leaving rc device registered").

3. The remaining 9 reports crash in input_devices_seq_show() on the next
   line (drivers/input/input.c:1179) when formatting dev->phys:

     seq_printf(seq, "P: Phys=%s\n", dev->phys ? dev->phys : "");

   (dev->name does not fault there because xpad->name points to a static
   string literal in xpad_device[].) All 9 reports read offset 0x220
   (544 bytes) inside a kmalloc-1k object (offsetof(struct usb_xpad,
   phys)). Two reports [4] (log [5]) and [6] caught the struct usb_xpad
   object before slab reuse:

     Allocated by: xpad_probe() (drivers/input/joystick/xpad.c:2052)
     Freed by:     xpad_disconnect() (drivers/input/joystick/xpad.c:2236)
     Last work:    xpad360w_process_packet() <- xpad_irq_in()

   In xpad360w_process_packet(), xpad->pad_present is updated in URB
   completion context and schedules xpad->work (xpad_presence_work()).
   If presence packets toggle xpad->pad_present (true -> false -> true)
   before xpad_presence_work() runs, xpad_presence_work() sees
   xpad->pad_present == true and calls xpad_init_input() again even
   though xpad->input_created is already true. That overwrites xpad->dev
   (and xpad->led) and leaks the old input_dev on input_dev_list (in [5]
   it registers input69 through input79 on the same USB interface). When
   xpad_disconnect() later runs, xpad_deinit_input() only unregisters
   the last xpad->dev and frees xpad, leaving the leaked input_dev
   instances on input_dev_list with dev->phys pointing into freed
   xpad->phys.

[1] https://syzkaller.appspot.com/text?tag=CrashReport&x=12bef8c9580000
[2] https://syzkaller.appspot.com/text?tag=CrashReport&x=15d46e79580000
[3] https://syzkaller.appspot.com/text?tag=CrashLog&x=16cc5679580000
[4] https://syzkaller.appspot.com/text?tag=CrashReport&x=115fd67e580000
[5] https://syzkaller.appspot.com/text?tag=CrashLog&x=17f40456580000
[6] https://syzkaller.appspot.com/text?tag=CrashReport&x=11131092580000

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2] Input: fix potential use-after-free in input_devices_seq_show
  2026-09-30 18:15     ` Dmitry Torokhov
@ 2026-10-01 16:04       ` Habil Eren Türker
  0 siblings, 0 replies; 5+ messages in thread
From: Habil Eren Türker @ 2026-10-01 16:04 UTC (permalink / raw)
  To: dmitry.torokhov@gmail.com
  Cc: habilerenturker@gmail.com, Habil Eren Türker,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	sashiko-bot@kernel.org, sashiko-reviews@lists.linux.dev

Hi Dmitry,

Thank you for taking the time to analyze this and for the detailed
breakdown. I really appreciate it.

Regards,
Türker

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

end of thread, other threads:[~2026-10-01 16:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-28 17:11 [PATCH v2] Input: fix potential use-after-free in input_devices_seq_show Habil Eren Türker
2026-09-28 17:21 ` sashiko-bot
2026-09-28 18:15   ` Habil Eren Türker
2026-09-30 18:15     ` Dmitry Torokhov
2026-10-01 16:04       ` Habil Eren Türker

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.