[PATCH] Input: serio - fix O(n^2) complexity in serio_unregister_driver()

Mohamad Raizudeen posted 1 patch 2 months, 1 week ago
drivers/input/serio/serio.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
[PATCH] Input: serio - fix O(n^2) complexity in serio_unregister_driver()
Posted by Mohamad Raizudeen 2 months, 1 week ago
The current implementation restarts the scan from the beginning after
each disconnection(goto start_over) because serio_disconnect_port() may
delete the current list entry. This results in O(n^2) worst case
behaviour.

Replace with a two-phase approach:

	1.Collect only top-level ports bound to the driver(skip those whose parent is also bound to the same driver) into a
	  temporary list.

	2.Process each collected port once, moving it back to serio_list first to maintain invariants expected by serio_destroy_port().

This eliminates the restart loop, reduces complexity from O(n^2) to O(n)
and avoids use-after-free by never collecting child ports separately.

Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
---
 drivers/input/serio/serio.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 54dd26249b02..46b8faf1a46c 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -821,22 +821,27 @@ EXPORT_SYMBOL(__serio_register_driver);
 
 void serio_unregister_driver(struct serio_driver *drv)
 {
-	struct serio *serio;
+	struct serio *serio, *next;
+	LIST_HEAD(disconnect_list);
+
 
 	guard(mutex)(&serio_mutex);
 
 	drv->manual_bind = true;	/* so serio_find_driver ignores it */
 	serio_remove_pending_events(drv);
 
-start_over:
-	list_for_each_entry(serio, &serio_list, node) {
-		if (serio->drv == drv) {
-			serio_disconnect_port(serio);
-			serio_find_driver(serio);
-			/* we could've deleted some ports, restart */
-			goto start_over;
+	/*Collect all ports bound to this driver first*/
+	list_for_each_entry_safe(serio, next, &serio_list, node) {
+		if (serio->drv == drv && !(serio->parent && serio->parent->drv == drv)) {
+			list_move_tail(&serio->node, &disconnect_list);
 		}
 	}
+	
+	list_for_each_entry_safe(serio, next, &disconnect_list, node) {
+		list_move_tail(&serio->node, &serio_list);
+		serio_disconnect_port(serio);
+		serio_find_driver(serio);
+	}
 
 	driver_unregister(&drv->driver);
 }
-- 
2.43.0
Re: [PATCH] Input: serio - fix O(n^2) complexity in serio_unregister_driver()
Posted by Dmitry Torokhov 2 months, 1 week ago
Hi Mohamad,

On Wed, Apr 08, 2026 at 09:58:47PM +0530, Mohamad Raizudeen wrote:
> The current implementation restarts the scan from the beginning after
> each disconnection(goto start_over) because serio_disconnect_port() may
> delete the current list entry. This results in O(n^2) worst case
> behaviour.

The "big O" notation only makes sense when numbers are big. Here we are
dealing with 6 serio ports max (1 KBC, 4xMUX, 1 Synaptics pass-through)
unless you have a serial port expander and hooked up a few ports there.
But still, the number if very limited.

> 
> Replace with a two-phase approach:
> 
> 	1.Collect only top-level ports bound to the driver(skip those whose parent is also bound to the same driver) into a
> 	  temporary list.

We do not have such setups at the moment, but what about parent's
parent's parent?

> 
> 	2.Process each collected port once, moving it back to serio_list first to maintain invariants expected by serio_destroy_port().
> 
> This eliminates the restart loop, reduces complexity from O(n^2) to O(n)
> and avoids use-after-free by never collecting child ports separately.

Could you explain more about the use-after-free scenario?

Thanks.

-- 
Dmitry