History

Input: ensure device is ready before delivering events

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.

Dmitry Torokhov committed 3 months ago

Commit 35f0a0d1 parent e07c5092 files · +74 −41Browse files

Unified · Split

2 files
  1. drivers/input/input.c +66 −37
  2. include/linux/input.h +8 −4
drivers/input/input.c+66−37
@@ -318,7 +318,7 @@ static int input_get_disposition(struct input_dev *dev, · 317 unmodified lines
318static void input_event_dispose(struct input_dev *dev, int disposition,318static void input_event_dispose(struct input_dev *dev, int disposition,
319 unsigned int type, unsigned int code, int value)319 unsigned int type, unsigned int code, int value)
320{320{
321 if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event)321 if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event && dev->ready)
322 dev->event(dev, type, code, value);322 dev->event(dev, type, code, value);
323 323
324 if (disposition & INPUT_PASS_TO_HANDLERS) {324 if (disposition & INPUT_PASS_TO_HANDLERS) {
@@ -568,6 +568,48 @@ void input_release_device(struct input_handle *handle) · 243 unmodified lines
568}568}
569EXPORT_SYMBOL(input_release_device);569EXPORT_SYMBOL(input_release_device);
570 570
571#define INPUT_DO_TOGGLE(dev, type, bits, on) \
572 do { \
573 int i; \
574 bool active; \
575 \
576 if (!test_bit(EV_##type, dev->evbit)) \
577 break; \
578 \
579 for_each_set_bit(i, dev->bits##bit, type##_CNT) { \
580 active = test_bit(i, dev->bits); \
581 if (!active && !on) \
582 continue; \
583 \
584 dev->event(dev, EV_##type, i, on ? active : 0); \
585 } \
586 } while (0)
587
588/*
589 * Iterate through the logical state of the input device (LEDs, sounds,
590 * auto-repeat) and explicitly push that state down to the hardware
591 * via dev->event() to match the current logical state (if activate is true),
592 * or forcibly turn off all feedback like LEDs and sounds during teardown
593 * or suspend (if activate is false).
594 *
595 * Primarily used as a state-replay mechanism after a device is opened
596 * or uninhibited, as events might have been dropped by the core while the
597 * hardware was not marked as ready.
598 */
599static void input_dev_toggle(struct input_dev *dev, bool activate)
600{
601 if (!dev->event || !dev->ready)
602 return;
603
604 INPUT_DO_TOGGLE(dev, LED, led, activate);
605 INPUT_DO_TOGGLE(dev, SND, snd, activate);
606
607 if (activate && test_bit(EV_REP, dev->evbit)) {
608 dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
609 dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
610 }
611}
612
571/**613/**
572 * input_open_device - open input device614 * input_open_device - open input device
573 * @handle: handle through which device is being accessed615 * @handle: handle through which device is being accessed
@@ -611,6 +653,11 @@ int input_open_device(struct input_handle *handle) · 37 unmodified lines
611 }653 }
612 }654 }
613 655
656 scoped_guard(spinlock_irq, &dev->event_lock) {
657 dev->ready = true;
658 input_dev_toggle(dev, true);
659 }
660
614 if (dev->poller)661 if (dev->poller)
615 input_dev_poller_start(dev->poller);662 input_dev_poller_start(dev->poller);
616 }663 }
@@ -651,6 +698,12 @@ void input_close_device(struct input_handle *handle) · 34 unmodified lines
651 if (!--dev->users && !dev->inhibited) {698 if (!--dev->users && !dev->inhibited) {
652 if (dev->poller)699 if (dev->poller)
653 input_dev_poller_stop(dev->poller);700 input_dev_poller_stop(dev->poller);
701
702 scoped_guard(spinlock_irq, &dev->event_lock) {
703 input_dev_toggle(dev, false);
704 dev->ready = false;
705 }
706
654 if (dev->close)707 if (dev->close)
655 dev->close(dev);708 dev->close(dev);
656 }709 }
@@ -1702,37 +1755,6 @@ static int input_dev_uevent(const struct device *device, struct kobj_uevent_env · 1045 unmodified lines
1702 return 0;1755 return 0;
1703}1756}
1704 1757
1705#define INPUT_DO_TOGGLE(dev, type, bits, on) \
1706 do { \
1707 int i; \
1708 bool active; \
1709 \
1710 if (!test_bit(EV_##type, dev->evbit)) \
1711 break; \
1712 \
1713 for_each_set_bit(i, dev->bits##bit, type##_CNT) { \
1714 active = test_bit(i, dev->bits); \
1715 if (!active && !on) \
1716 continue; \
1717 \
1718 dev->event(dev, EV_##type, i, on ? active : 0); \
1719 } \
1720 } while (0)
1721
1722static void input_dev_toggle(struct input_dev *dev, bool activate)
1723{
1724 if (!dev->event)
1725 return;
1726
1727 INPUT_DO_TOGGLE(dev, LED, led, activate);
1728 INPUT_DO_TOGGLE(dev, SND, snd, activate);
1729
1730 if (activate && test_bit(EV_REP, dev->evbit)) {
1731 dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
1732 dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
1733 }
1734}
1735
1736/**1758/**
1737 * input_reset_device() - reset/restore the state of input device1759 * input_reset_device() - reset/restore the state of input device
1738 * @dev: input device whose state needs to be reset1760 * @dev: input device whose state needs to be reset
@@ -1760,21 +1782,25 @@ static int input_inhibit_device(struct input_dev *dev) · 21 unmodified lines
1760 return 0;1782 return 0;
1761 1783
1762 if (dev->users) {1784 if (dev->users) {
1763 if (dev->close)
1764 dev->close(dev);
1765 if (dev->poller)1785 if (dev->poller)
1766 input_dev_poller_stop(dev->poller);1786 input_dev_poller_stop(dev->poller);
1787
1788 scoped_guard(spinlock_irq, &dev->event_lock) {
1789 input_dev_toggle(dev, false);
1790 dev->ready = false;
1791 }
1792
1793 if (dev->close)
1794 dev->close(dev);
1767 }1795 }
1768 1796
1769 scoped_guard(spinlock_irq, &dev->event_lock) {1797 scoped_guard(spinlock_irq, &dev->event_lock) {
1770 input_mt_release_slots(dev);1798 input_mt_release_slots(dev);
1771 input_dev_release_keys(dev);1799 input_dev_release_keys(dev);
1772 input_handle_event(dev, EV_SYN, SYN_REPORT, 1);1800 input_handle_event(dev, EV_SYN, SYN_REPORT, 1);
1773 input_dev_toggle(dev, false);1801 dev->inhibited = true;
1774 }1802 }
1775 1803
1776 dev->inhibited = true;
1777
1778 return 0;1804 return 0;
1779}1805}
1780 1806
@@ -1793,6 +1819,9 @@ static int input_uninhibit_device(struct input_dev *dev) · 12 unmodified lines
1793 if (error)1819 if (error)
1794 return error;1820 return error;
1795 }1821 }
1822 scoped_guard(spinlock_irq, &dev->event_lock)
1823 dev->ready = true;
1824
1796 if (dev->poller)1825 if (dev->poller)
1797 input_dev_poller_start(dev->poller);1826 input_dev_poller_start(dev->poller);
1798 }1827 }
include/linux/input.h+8−4
@@ -128,11 +128,14 @@ enum input_clock_type { · 127 unmodified lines
128 * @devres_managed: indicates that devices is managed with devres framework128 * @devres_managed: indicates that devices is managed with devres framework
129 * and needs not be explicitly unregistered or freed.129 * and needs not be explicitly unregistered or freed.
130 * @timestamp: storage for a timestamp set by input_set_timestamp called130 * @timestamp: storage for a timestamp set by input_set_timestamp called
131 * by a driver131 * by a driver
132 * @inhibited: indicates that the input device is inhibited. If that is132 * @inhibited: indicates that the input device is inhibited. If that is
133 * the case then input core ignores any events generated by the device.133 * the case then input core ignores any events generated by the device.
134 * Device's close() is called when it is being inhibited and its open()134 * Device's close() is called when it is being inhibited and its open()
135 * is called when it is being uninhibited.135 * is called when it is being uninhibited.
136 * @ready: indicates that the device has been successfully opened and is
137 * prepared to process events (like LEDs or sounds) sent from the
138 * input core.
136 */139 */
137struct input_dev {140struct input_dev {
138 const char *name;141 const char *name;
@@ -209,6 +212,7 @@ struct input_dev { · 70 unmodified lines
209 ktime_t timestamp[INPUT_CLK_MAX];212 ktime_t timestamp[INPUT_CLK_MAX];
210 213
211 bool inhibited;214 bool inhibited;
215 bool ready;
212};216};
213#define to_input_dev(d) container_of(d, struct input_dev, dev)217#define to_input_dev(d) container_of(d, struct input_dev, dev)
214 218

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t