[PATCH v2 1/4] Input: ensure device is ready before delivering events

[PATCH v2 1/4] Input: ensure device is ready before delivering events

[Dmitry Torokhov]

When a device is opened via input_open_device(), the driver's open()
callback is invoked. Some drivers, like cm109, submit URBs or perform
other hardware initialization in their open() callbacks.

However, the input core does not prevent dev->event() from being called
concurrently during the driver's open() execution. For instance, if a
console beep occurs, the kbd handler might inject an EV_SND event. This
can lead to double list_add BUGs if the driver submits the same URB in
both open() and event() paths without adequate synchronization.

To fix this, introduce a ready flag in the input_dev structure.
For complex devices (where dev->open is defined), this flag is set to true
only after the driver's open() method successfully completes. The core now
checks ready in input_event_dispose() and input_dev_toggle()
to prevent events from reaching the hardware before it is fully prepared.
For simple devices (no open callback), events are delivered immediately.

We also replay the logical state in input_open_device() by calling
input_dev_toggle() right after marking the device ready, ensuring no
events are permanently lost.

In the inhibit path, we ensure that physical feedback (LEDs/sounds) is
turned off before the device is closed, and we synchronize the inhibited
state transition under the event lock to prevent races with incoming events.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

v2:
- made the patch introducing "ready" flagi first in the series
- fixed up Sashiko's comments regarding not shutting off LEDs on close

 drivers/input/input.c | 103 +++++++++++++++++++++++++++---------------
 include/linux/input.h |  12 +++--
 2 files changed, 74 insertions(+), 41 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index cf6fecea79b8..e57d1023d262 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -318,7 +318,7 @@ static int input_get_disposition(struct input_dev *dev,
 static void input_event_dispose(struct input_dev *dev, int disposition,
 				unsigned int type, unsigned int code, int value)
 {
-	if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event)
+	if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event && dev->ready)
 		dev->event(dev, type, code, value);
 
 	if (disposition & INPUT_PASS_TO_HANDLERS) {
@@ -568,6 +568,48 @@ void input_release_device(struct input_handle *handle)
 }
 EXPORT_SYMBOL(input_release_device);
 
+#define INPUT_DO_TOGGLE(dev, type, bits, on)				\
+	do {								\
+		int i;							\
+		bool active;						\
+									\
+		if (!test_bit(EV_##type, dev->evbit))			\
+			break;						\
+									\
+		for_each_set_bit(i, dev->bits##bit, type##_CNT) {	\
+			active = test_bit(i, dev->bits);		\
+			if (!active && !on)				\
+				continue;				\
+									\
+			dev->event(dev, EV_##type, i, on ? active : 0);	\
+		}							\
+	} while (0)
+
+/*
+ * Iterate through the logical state of the input device (LEDs, sounds,
+ * auto-repeat) and explicitly push that state down to the hardware
+ * via dev->event() to match the current logical state (if activate is true),
+ * or forcibly turn off all feedback like LEDs and sounds during teardown
+ * or suspend (if activate is false).
+ *
+ * Primarily used as a state-replay mechanism after a device is opened
+ * or uninhibited, as events might have been dropped by the core while the
+ * hardware was not marked as ready.
+ */
+static void input_dev_toggle(struct input_dev *dev, bool activate)
+{
+	if (!dev->event || !dev->ready)
+		return;
+
+	INPUT_DO_TOGGLE(dev, LED, led, activate);
+	INPUT_DO_TOGGLE(dev, SND, snd, activate);
+
+	if (activate && test_bit(EV_REP, dev->evbit)) {
+		dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
+		dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
+	}
+}
+
 /**
  * input_open_device - open input device
  * @handle: handle through which device is being accessed
@@ -611,6 +653,11 @@ int input_open_device(struct input_handle *handle)
 			}
 		}
 
+		scoped_guard(spinlock_irq, &dev->event_lock) {
+			dev->ready = true;
+			input_dev_toggle(dev, true);
+		}
+
 		if (dev->poller)
 			input_dev_poller_start(dev->poller);
 	}
@@ -651,6 +698,12 @@ void input_close_device(struct input_handle *handle)
 		if (!--dev->users && !dev->inhibited) {
 			if (dev->poller)
 				input_dev_poller_stop(dev->poller);
+
+			scoped_guard(spinlock_irq, &dev->event_lock) {
+				input_dev_toggle(dev, false);
+				dev->ready = false;
+			}
+
 			if (dev->close)
 				dev->close(dev);
 		}
@@ -1702,37 +1755,6 @@ static int input_dev_uevent(const struct device *device, struct kobj_uevent_env
 	return 0;
 }
 
-#define INPUT_DO_TOGGLE(dev, type, bits, on)				\
-	do {								\
-		int i;							\
-		bool active;						\
-									\
-		if (!test_bit(EV_##type, dev->evbit))			\
-			break;						\
-									\
-		for_each_set_bit(i, dev->bits##bit, type##_CNT) {	\
-			active = test_bit(i, dev->bits);		\
-			if (!active && !on)				\
-				continue;				\
-									\
-			dev->event(dev, EV_##type, i, on ? active : 0);	\
-		}							\
-	} while (0)
-
-static void input_dev_toggle(struct input_dev *dev, bool activate)
-{
-	if (!dev->event)
-		return;
-
-	INPUT_DO_TOGGLE(dev, LED, led, activate);
-	INPUT_DO_TOGGLE(dev, SND, snd, activate);
-
-	if (activate && test_bit(EV_REP, dev->evbit)) {
-		dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
-		dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
-	}
-}
-
 /**
  * input_reset_device() - reset/restore the state of input device
  * @dev: input device whose state needs to be reset
@@ -1760,21 +1782,25 @@ static int input_inhibit_device(struct input_dev *dev)
 		return 0;
 
 	if (dev->users) {
-		if (dev->close)
-			dev->close(dev);
 		if (dev->poller)
 			input_dev_poller_stop(dev->poller);
+
+		scoped_guard(spinlock_irq, &dev->event_lock) {
+			input_dev_toggle(dev, false);
+			dev->ready = false;
+		}
+
+		if (dev->close)
+			dev->close(dev);
 	}
 
 	scoped_guard(spinlock_irq, &dev->event_lock) {
 		input_mt_release_slots(dev);
 		input_dev_release_keys(dev);
 		input_handle_event(dev, EV_SYN, SYN_REPORT, 1);
-		input_dev_toggle(dev, false);
+		dev->inhibited = true;
 	}
 
-	dev->inhibited = true;
-
 	return 0;
 }
 
@@ -1793,6 +1819,9 @@ static int input_uninhibit_device(struct input_dev *dev)
 			if (error)
 				return error;
 		}
+		scoped_guard(spinlock_irq, &dev->event_lock)
+			dev->ready = true;
+
 		if (dev->poller)
 			input_dev_poller_start(dev->poller);
 	}
diff --git a/include/linux/input.h b/include/linux/input.h
index 76f7aa226202..f147d27e6d1d 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -128,11 +128,14 @@ enum input_clock_type {
  * @devres_managed: indicates that devices is managed with devres framework
  *	and needs not be explicitly unregistered or freed.
  * @timestamp: storage for a timestamp set by input_set_timestamp called
- *  by a driver
+ *	by a driver
  * @inhibited: indicates that the input device is inhibited. If that is
- * the case then input core ignores any events generated by the device.
- * Device's close() is called when it is being inhibited and its open()
- * is called when it is being uninhibited.
+ *	the case then input core ignores any events generated by the device.
+ *	Device's close() is called when it is being inhibited and its open()
+ *	is called when it is being uninhibited.
+ * @ready: indicates that the device has been successfully opened and is
+ *	prepared to process events (like LEDs or sounds) sent from the
+ *	input core.
  */
 struct input_dev {
 	const char *name;
@@ -209,6 +212,7 @@ struct input_dev {
 	ktime_t timestamp[INPUT_CLK_MAX];
 
 	bool inhibited;
+	bool ready;
 };
 #define to_input_dev(d) container_of(d, struct input_dev, dev)
 
-- 
2.55.0.508.g3f0d502094-goog

[PATCH v2 2/4] Input: clear inhibited flag before re-opening device on uninhibit

[Dmitry Torokhov]

When uninhibiting a device, we previously called dev->open() and started
the poller before clearing dev->inhibited. Since drivers (like
gpio_keys) often report initial state during open(), and pollers report
events immediately upon starting, these initial events were dropped by
input_get_disposition() because dev->inhibited was still true.

Fix this by clearing dev->inhibited before calling dev->open(), ensuring
initial events are delivered to handlers, and restoring dev->inhibited =
true if dev->open() fails.

Fixes: a181616487db ("Input: Add "inhibited" property")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index e57d1023d262..e4f8c2067b84 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1813,24 +1813,26 @@ static int input_uninhibit_device(struct input_dev *dev)
 	if (!dev->inhibited)
 		return 0;
 
+	dev->inhibited = false;
+
 	if (dev->users) {
 		if (dev->open) {
 			error = dev->open(dev);
-			if (error)
+			if (error) {
+				dev->inhibited = true;
 				return error;
+			}
 		}
 		scoped_guard(spinlock_irq, &dev->event_lock)
 			dev->ready = true;
-
-		if (dev->poller)
-			input_dev_poller_start(dev->poller);
 	}
 
-	dev->inhibited = false;
-
 	scoped_guard(spinlock_irq, &dev->event_lock)
 		input_dev_toggle(dev, true);
 
+	if (dev->users && dev->poller)
+		input_dev_poller_start(dev->poller);
+
 	return 0;
 }
 
-- 
2.55.0.508.g3f0d502094-goog

[PATCH v2 3/4] Input: call handler->start() when uninhibiting device

[Dmitry Torokhov]

When an input device is inhibited via input_inhibit_device(), the driver
is closed and physical feedback (like LEDs and sounds) is toggled off.
However, from the input core's perspective, the handles remain open.

When the device is later uninhibited, the driver is re-opened. While the
core restores simple LED states via input_dev_toggle(), complex handlers
(such as vt/keyboard) may need to re-synchronize their broader logical
state with the hardware.

Fixes: a181616487db ("Input: Add "inhibited" property")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index e4f8c2067b84..47886a394c6b 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1806,6 +1806,7 @@ static int input_inhibit_device(struct input_dev *dev)
 
 static int input_uninhibit_device(struct input_dev *dev)
 {
+	struct input_handle *handle;
 	int error;
 
 	guard(mutex)(&dev->mutex);
@@ -1833,6 +1834,11 @@ static int input_uninhibit_device(struct input_dev *dev)
 	if (dev->users && dev->poller)
 		input_dev_poller_start(dev->poller);
 
+	list_for_each_entry(handle, &dev->h_list, d_node) {
+		if (handle->open && handle->handler->start)
+			handle->handler->start(handle);
+	}
+
 	return 0;
 }
 
-- 
2.55.0.508.g3f0d502094-goog

[PATCH v2 4/4] Input: defer handler's start() until device is opened

[Dmitry Torokhov]

When registering an input handle, handler->start() is currently called
immediately. However, the input device might not be fully opened or
ready to process events at this stage, meaning any state synchronization
events (like setting LED states) injected by the handler's start method
might be dropped.

Move the handler->start() invocation to input_open_device(). If it is
the first handle opening the device, start() is called after the driver's
open() method has successfully completed and the device is fully prepared.

To facilitate this, factor out the device startup logic (calling driver's
open and starting polling) into input_start_device().

For passive observer handlers, their start() method is also deferred
until the handle is opened. Since opening a passive observer handle does
not start the underlying hardware device, their start() method is called
immediately upon opening, regardless of whether the device is active.

Fixes: c7e8dc6ee6d5 ("Input: add start() method to input handlers")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c | 55 ++++++++++++++++++++++++-------------------
 include/linux/input.h |  5 ++--
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 47886a394c6b..c9f480629099 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -610,6 +610,33 @@ static void input_dev_toggle(struct input_dev *dev, bool activate)
 	}
 }
 
+static int input_start_device(struct input_dev *dev)
+{
+	int error;
+
+	lockdep_assert_held(&dev->mutex);
+
+	if (dev->users++ == 0 && !dev->inhibited) {
+		if (dev->open) {
+			error = dev->open(dev);
+			if (error) {
+				dev->users--;
+				return error;
+			}
+		}
+
+		scoped_guard(spinlock_irq, &dev->event_lock) {
+			dev->ready = true;
+			input_dev_toggle(dev, true);
+		}
+
+		if (dev->poller)
+			input_dev_poller_start(dev->poller);
+	}
+
+	return 0;
+}
+
 /**
  * input_open_device - open input device
  * @handle: handle through which device is being accessed
@@ -628,21 +655,9 @@ int input_open_device(struct input_handle *handle)
 
 		handle->open++;
 
-		if (handle->handler->passive_observer)
-			return 0;
-
-		if (dev->users++ || dev->inhibited) {
-			/*
-			 * Device is already opened and/or inhibited,
-			 * so we can exit immediately and report success.
-			 */
-			return 0;
-		}
-
-		if (dev->open) {
-			error = dev->open(dev);
+		if (!handle->handler->passive_observer) {
+			error = input_start_device(dev);
 			if (error) {
-				dev->users--;
 				handle->open--;
 				/*
 				 * Make sure we are not delivering any more
@@ -653,13 +668,8 @@ int input_open_device(struct input_handle *handle)
 			}
 		}
 
-		scoped_guard(spinlock_irq, &dev->event_lock) {
-			dev->ready = true;
-			input_dev_toggle(dev, true);
-		}
-
-		if (dev->poller)
-			input_dev_poller_start(dev->poller);
+		if (handle->open == 1 && handle->handler->start)
+			handle->handler->start(handle);
 	}
 
 	return 0;
@@ -2692,9 +2702,6 @@ int input_register_handle(struct input_handle *handle)
 	 */
 	list_add_tail_rcu(&handle->h_node, &handler->h_list);
 
-	if (handler->start)
-		handler->start(handle);
-
 	return 0;
 }
 EXPORT_SYMBOL(input_register_handle);
diff --git a/include/linux/input.h b/include/linux/input.h
index f147d27e6d1d..0ee5f32de08a 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -288,8 +288,9 @@ struct input_handle;
  * @connect: called when attaching a handler to an input device
  * @disconnect: disconnects a handler from input device
  * @start: starts handler for given handle. This function is called by
- *	input core right after connect() method and also when a process
- *	that "grabbed" a device releases it
+ *	input core when device is open and ready to process events,
+ *	and also when device is uninhibited or when a process that "grabbed"
+ *	a device releases it
  * @passive_observer: set to %true by drivers only interested in observing
  *	data stream from devices if there are other users present. Such
  *	drivers will not result in starting underlying hardware device
-- 
2.55.0.508.g3f0d502094-goog

Re: [PATCH v2 2/4] Input: clear inhibited flag before re-opening device on uninhibit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] When `dev->open()` fails during uninhibiting, restoring `dev->inhibited = true` without holding `dev->event_lock` and without releasing active keys leaves events permanently stuck in userspace.
--

--- Patch [2]: [PATCH v2 2/4] Input: clear inhibited flag before re-opening device on uninhibit ---
commit f055731a6a02bc8698a040a5af7e51e9039340a7
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: clear inhibited flag before re-opening device on uninhibit

This patch clears dev->inhibited before calling dev->open() when
uninhibiting a device. It ensures initial events are delivered to handlers,
and restores dev->inhibited if dev->open() fails.

> diff --git a/drivers/input/input.c b/drivers/input/input.c
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1813,24 +1813,26 @@ static int input_uninhibit_device(struct input_dev *dev)
>  	if (!dev->inhibited)
>  		return 0;
>  
> +	dev->inhibited = false;
> +
>  	if (dev->users) {
>  		if (dev->open) {
>  			error = dev->open(dev);
> -			if (error)
> +			if (error) {
> +				dev->inhibited = true;

[Severity: Medium]
When dev->open() fails and dev->inhibited is restored to true, is there a
potential data race with concurrent input_handle_event() readers?

Since dev->inhibited was set to false earlier, the driver's open() method or
interrupts could have emitted events before the failure. Does restoring
dev->inhibited here without holding dev->event_lock and without synthesizing
key releases via input_dev_release_keys() risk leaving events permanently
stuck in userspace?

>  				return error;
> +			}
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803005210.1251102-1-dmitry.torokhov@gmail.com?part=2

Re: [PATCH v2 3/4] Input: call handler->start() when uninhibiting device

[sashiko-bot]

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

Pre-existing issues:
- [High] Sysfs uninhibiting a device can race with device unregistration, causing `dev->open()` to be called on a hardware device that is being destroyed.
--

--- Patch [3]: [PATCH v2 3/4] Input: call handler->start() when uninhibiting device ---
commit 7812e6dd22b71b48fc822b6545778a4eb30f8e35
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: call handler->start() when uninhibiting device

When an input device is uninhibited, the driver is re-opened. This patch
ensures that complex handlers are also notified to re-synchronize their
state by calling handler->start() for each open handle.

> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index e4f8c2067b84d..47886a394c6b8 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1806,6 +1806,7 @@ static int input_inhibit_device(struct input_dev *dev)
>  
>  static int input_uninhibit_device(struct input_dev *dev)
>  {
> +	struct input_handle *handle;
>  	int error;
>  
>  	guard(mutex)(&dev->mutex);

[Severity: High]
This is a pre-existing issue, but does this function open a race condition
with input_unregister_device() by missing a check for dev->going_away?

When a driver calls input_unregister_device(), the core sets
dev->going_away = true in input_disconnect_device() and releases
dev->mutex.

If a concurrent sysfs write to the inhibited attribute executes
input_uninhibit_device(), it acquires dev->mutex here.

Because it lacks a dev->going_away check (unlike input_open_device()), it
proceeds to set dev->inhibited = false and calls dev->open() on a dying
device.

Can this lead to a use-after-free or hardware state corruption if the
driver has already freed its IO mappings or IRQs during unregistration?

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

Re: [PATCH v2 2/4] Input: clear inhibited flag before re-opening device on uninhibit

[Dmitry Torokhov]

On Mon, Aug 03, 2026 at 01:18:32AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] When `dev->open()` fails during uninhibiting, restoring `dev->inhibited = true` without holding `dev->event_lock` and without releasing active keys leaves events permanently stuck in userspace.
> --
> 
> --- Patch [2]: [PATCH v2 2/4] Input: clear inhibited flag before re-opening device on uninhibit ---
> commit f055731a6a02bc8698a040a5af7e51e9039340a7
> Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> Input: clear inhibited flag before re-opening device on uninhibit
> 
> This patch clears dev->inhibited before calling dev->open() when
> uninhibiting a device. It ensures initial events are delivered to handlers,
> and restores dev->inhibited if dev->open() fails.
> 
> > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > --- a/drivers/input/input.c
> > +++ b/drivers/input/input.c
> > @@ -1813,24 +1813,26 @@ static int input_uninhibit_device(struct input_dev *dev)
> >  	if (!dev->inhibited)
> >  		return 0;
> >  
> > +	dev->inhibited = false;
> > +
> >  	if (dev->users) {
> >  		if (dev->open) {
> >  			error = dev->open(dev);
> > -			if (error)
> > +			if (error) {
> > +				dev->inhibited = true;
> 
> [Severity: Medium]
> When dev->open() fails and dev->inhibited is restored to true, is there a
> potential data race with concurrent input_handle_event() readers?
> 
> Since dev->inhibited was set to false earlier, the driver's open() method or
> interrupts could have emitted events before the failure. Does restoring
> dev->inhibited here without holding dev->event_lock and without synthesizing
> key releases via input_dev_release_keys() risk leaving events permanently
> stuck in userspace?

Userspace has to clean up its own state after a device disappears.

Thanks.

-- 
Dmitry