* [PATCH v4 4/5] HID: hid-oxp: Add Button Mapping Interface
From: Derek J. Clark @ 2026-04-19 4:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Lambert Fan, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260419042624.625746-1-derekjohn.clark@gmail.com>
Adds button mapping interface for second generation OneXPlayer
configuration HID interfaces. This interface allows the MCU to swap
button mappings at the hardware level. The current state cannot be
retrieved, and the mappings may have been modified in Windows prior, so
we reset the button mapping at init and expose an attribute to allow
userspace to do this again at any time.
The interface requires two pages of button mapping data to be sent
before the settings will take place. Since the MCU requires a 200ms
delay after each message (total 400ms for these attributes) use the same
debounce work queue method we used for RGB. This will allow for
userspace or udev rules to rapidly map all buttons. The values will
be cached before the final write is finally sent to the device.
Reviewed-by: Zhouwang Huang <honjow311@gmail.com>
Tested-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v4:
- Make oxp_btn_queue delayed work struct part of drvdata, add cancel
delayed work during remove.
v3:
- Ensure default button map is properly init during probe.
v2:
- Add detection of post-suspend MCU init to trigger setting the button
map again.
---
drivers/hid/hid-oxp.c | 568 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 568 insertions(+)
diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
index 2504b56b8f8a..52002d4cbd0b 100644
--- a/drivers/hid/hid-oxp.c
+++ b/drivers/hid/hid-oxp.c
@@ -34,11 +34,147 @@ enum oxp_function_index {
OXP_FID_GEN1_RGB_SET = 0x07,
OXP_FID_GEN1_RGB_REPLY = 0x0f,
OXP_FID_GEN2_TOGGLE_MODE = 0xb2,
+ OXP_FID_GEN2_KEY_STATE = 0xb4,
OXP_FID_GEN2_STATUS_EVENT = 0xb8,
};
+#define OXP_MAPPING_GAMEPAD 0x01
+#define OXP_MAPPING_KEYBOARD 0x02
+
+struct oxp_button_data {
+ u8 mode;
+ u8 index;
+ u8 key_id;
+ u8 padding[2];
+} __packed;
+
+struct oxp_button_entry {
+ struct oxp_button_data data;
+ const char *name;
+};
+
+static const struct oxp_button_entry oxp_button_table[] = {
+ /* Gamepad Buttons */
+ { { OXP_MAPPING_GAMEPAD, 0x01 }, "BTN_A" },
+ { { OXP_MAPPING_GAMEPAD, 0x02 }, "BTN_B" },
+ { { OXP_MAPPING_GAMEPAD, 0x03 }, "BTN_X" },
+ { { OXP_MAPPING_GAMEPAD, 0x04 }, "BTN_Y" },
+ { { OXP_MAPPING_GAMEPAD, 0x05 }, "BTN_LB" },
+ { { OXP_MAPPING_GAMEPAD, 0x06 }, "BTN_RB" },
+ { { OXP_MAPPING_GAMEPAD, 0x07 }, "BTN_LT" },
+ { { OXP_MAPPING_GAMEPAD, 0x08 }, "BTN_RT" },
+ { { OXP_MAPPING_GAMEPAD, 0x09 }, "BTN_START" },
+ { { OXP_MAPPING_GAMEPAD, 0x0a }, "BTN_SELECT" },
+ { { OXP_MAPPING_GAMEPAD, 0x0b }, "BTN_L3" },
+ { { OXP_MAPPING_GAMEPAD, 0x0c }, "BTN_R3" },
+ { { OXP_MAPPING_GAMEPAD, 0x0d }, "DPAD_UP" },
+ { { OXP_MAPPING_GAMEPAD, 0x0e }, "DPAD_DOWN" },
+ { { OXP_MAPPING_GAMEPAD, 0x0f }, "DPAD_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x10 }, "DPAD_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x11 }, "JOY_L_UP" },
+ { { OXP_MAPPING_GAMEPAD, 0x12 }, "JOY_L_UP_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x13 }, "JOY_L_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x14 }, "JOY_L_DOWN_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x15 }, "JOY_L_DOWN" },
+ { { OXP_MAPPING_GAMEPAD, 0x16 }, "JOY_L_DOWN_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x17 }, "JOY_L_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x18 }, "JOY_L_UP_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x19 }, "JOY_R_UP" },
+ { { OXP_MAPPING_GAMEPAD, 0x1a }, "JOY_R_UP_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x1b }, "JOY_R_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x1c }, "JOY_R_DOWN_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x1d }, "JOY_R_DOWN" },
+ { { OXP_MAPPING_GAMEPAD, 0x1e }, "JOY_R_DOWN_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x1f }, "JOY_R_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x20 }, "JOY_R_UP_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x22 }, "BTN_GUIDE" },
+ /* Keyboard Keys */
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5a }, "KEY_F1" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5b }, "KEY_F2" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5c }, "KEY_F3" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5d }, "KEY_F4" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5e }, "KEY_F5" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5f }, "KEY_F6" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x60 }, "KEY_F7" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x61 }, "KEY_F8" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x62 }, "KEY_F9" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x63 }, "KEY_F10" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x64 }, "KEY_F11" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x65 }, "KEY_F12" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x66 }, "KEY_F13" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x67 }, "KEY_F14" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x68 }, "KEY_F15" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x69 }, "KEY_F16" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6a }, "KEY_F17" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6b }, "KEY_F18" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6c }, "KEY_F19" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6d }, "KEY_F20" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6e }, "KEY_F21" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6f }, "KEY_F22" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x70 }, "KEY_F23" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x71 }, "KEY_F24" },
+};
+
+enum oxp_joybutton_index {
+ BUTTON_A = 0x01,
+ BUTTON_B,
+ BUTTON_X,
+ BUTTON_Y,
+ BUTTON_LB,
+ BUTTON_RB,
+ BUTTON_LT,
+ BUTTON_RT,
+ BUTTON_START,
+ BUTTON_SELECT,
+ BUTTON_L3,
+ BUTTON_R3,
+ BUTTON_DUP,
+ BUTTON_DDOWN,
+ BUTTON_DLEFT,
+ BUTTON_DRIGHT,
+ BUTTON_M1 = 0x22,
+ BUTTON_M2,
+ /* These are unused currently, reserved for future devices */
+ BUTTON_M3,
+ BUTTON_M4,
+ BUTTON_M5,
+ BUTTON_M6,
+};
+
+struct oxp_button_idx {
+ enum oxp_joybutton_index button_idx;
+ u8 mapping_idx;
+} __packed;
+
+struct oxp_bmap_page_1 {
+ struct oxp_button_idx btn_a;
+ struct oxp_button_idx btn_b;
+ struct oxp_button_idx btn_x;
+ struct oxp_button_idx btn_y;
+ struct oxp_button_idx btn_lb;
+ struct oxp_button_idx btn_rb;
+ struct oxp_button_idx btn_lt;
+ struct oxp_button_idx btn_rt;
+ struct oxp_button_idx btn_start;
+} __packed;
+
+struct oxp_bmap_page_2 {
+ struct oxp_button_idx btn_select;
+ struct oxp_button_idx btn_l3;
+ struct oxp_button_idx btn_r3;
+ struct oxp_button_idx btn_dup;
+ struct oxp_button_idx btn_ddown;
+ struct oxp_button_idx btn_dleft;
+ struct oxp_button_idx btn_dright;
+ struct oxp_button_idx btn_m1;
+ struct oxp_button_idx btn_m2;
+} __packed;
+
static struct oxp_hid_cfg {
struct delayed_work oxp_rgb_queue;
+ struct delayed_work oxp_btn_queue;
+ struct oxp_bmap_page_1 *bmap_1;
+ struct oxp_bmap_page_2 *bmap_2;
struct delayed_work oxp_mcu_init;
struct led_classdev_mc *led_mc;
struct hid_device *hdev;
@@ -50,6 +186,10 @@ static struct oxp_hid_cfg {
u8 rgb_en;
} drvdata;
+#define OXP_FILL_PAGE_SLOT(page, btn) \
+ { .button_idx = (page)->btn.button_idx, \
+ .mapping_idx = (page)->btn.mapping_idx }
+
enum oxp_gamepad_mode_index {
OXP_GP_MODE_XINPUT = 0x00,
OXP_GP_MODE_DEBUG = 0x03,
@@ -155,6 +295,10 @@ struct oxp_gen_2_rgb_report {
u8 effect;
} __packed;
+struct oxp_attr {
+ u8 index;
+};
+
static u16 get_usage_page(struct hid_device *hdev)
{
return hdev->collection[0].usage >> 16;
@@ -196,12 +340,19 @@ static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
}
static int oxp_gen_2_property_out(enum oxp_function_index fid, u8 *data, u8 data_size);
+static int oxp_set_buttons(void);
static void oxp_mcu_init_fn(struct work_struct *work)
{
u8 gp_mode_data[3] = { OXP_GP_MODE_DEBUG, 0x01, 0x02 };
int ret;
+ /* Re-apply the button mapping */
+ ret = oxp_set_buttons();
+ if (ret)
+ dev_err(&drvdata.hdev->dev,
+ "Error: Failed to set button mapping: %i\n", ret);
+
/* Cycle the gamepad mode */
ret = oxp_gen_2_property_out(OXP_FID_GEN2_TOGGLE_MODE, gp_mode_data, 3);
if (ret)
@@ -395,9 +546,408 @@ static ssize_t gamepad_mode_index_show(struct device *dev,
}
static DEVICE_ATTR_RO(gamepad_mode_index);
+static void oxp_set_defaults_bmap_1(struct oxp_bmap_page_1 *bmap)
+{
+ bmap->btn_a.button_idx = BUTTON_A;
+ bmap->btn_a.mapping_idx = 0;
+ bmap->btn_b.button_idx = BUTTON_B;
+ bmap->btn_b.mapping_idx = 1;
+ bmap->btn_x.button_idx = BUTTON_X;
+ bmap->btn_x.mapping_idx = 2;
+ bmap->btn_y.button_idx = BUTTON_Y;
+ bmap->btn_y.mapping_idx = 3;
+ bmap->btn_lb.button_idx = BUTTON_LB;
+ bmap->btn_lb.mapping_idx = 4;
+ bmap->btn_rb.button_idx = BUTTON_RB;
+ bmap->btn_rb.mapping_idx = 5;
+ bmap->btn_lt.button_idx = BUTTON_LT;
+ bmap->btn_lt.mapping_idx = 6;
+ bmap->btn_rt.button_idx = BUTTON_RT;
+ bmap->btn_rt.mapping_idx = 7;
+ bmap->btn_start.button_idx = BUTTON_START;
+ bmap->btn_start.mapping_idx = 8;
+}
+
+static void oxp_set_defaults_bmap_2(struct oxp_bmap_page_2 *bmap)
+{
+ bmap->btn_select.button_idx = BUTTON_SELECT;
+ bmap->btn_select.mapping_idx = 9;
+ bmap->btn_l3.button_idx = BUTTON_L3;
+ bmap->btn_l3.mapping_idx = 10;
+ bmap->btn_r3.button_idx = BUTTON_R3;
+ bmap->btn_r3.mapping_idx = 11;
+ bmap->btn_dup.button_idx = BUTTON_DUP;
+ bmap->btn_dup.mapping_idx = 12;
+ bmap->btn_ddown.button_idx = BUTTON_DDOWN;
+ bmap->btn_ddown.mapping_idx = 13;
+ bmap->btn_dleft.button_idx = BUTTON_DLEFT;
+ bmap->btn_dleft.mapping_idx = 14;
+ bmap->btn_dright.button_idx = BUTTON_DRIGHT;
+ bmap->btn_dright.mapping_idx = 15;
+ bmap->btn_m1.button_idx = BUTTON_M1;
+ bmap->btn_m1.mapping_idx = 48; /* KEY_F15 */
+ bmap->btn_m2.button_idx = BUTTON_M2;
+ bmap->btn_m2.mapping_idx = 49; /* KEY_F16 */
+}
+
+static void oxp_page_fill_data(char *buf, const struct oxp_button_idx *buttons,
+ size_t len)
+{
+ size_t offset_increment = sizeof(u8) + sizeof(struct oxp_button_idx);
+ size_t offset = 5;
+ unsigned int i;
+
+ for (i = 0; i < len; i++, offset += offset_increment) {
+ buf[offset] = (u8)buttons[i].button_idx;
+ memcpy(buf + offset + 1,
+ &oxp_button_table[buttons[i].mapping_idx].data,
+ sizeof(struct oxp_button_data));
+ }
+}
+
+static int oxp_set_buttons(void)
+{
+ u8 page_1[59] = { 0x02, 0x38, 0x20, 0x01, 0x01 };
+ u8 page_2[59] = { 0x02, 0x38, 0x20, 0x02, 0x01 };
+ u16 up = get_usage_page(drvdata.hdev);
+ int ret;
+
+ if (up != GEN2_USAGE_PAGE)
+ return -EINVAL;
+
+ const struct oxp_button_idx p1[] = {
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_a),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_b),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_x),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_y),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_lb),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_rb),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_lt),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_rt),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_start),
+ };
+
+ const struct oxp_button_idx p2[] = {
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_select),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_l3),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_r3),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_dup),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_ddown),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_dleft),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_dright),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_m1),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_m2),
+ };
+
+ oxp_page_fill_data(page_1, p1, ARRAY_SIZE(p1));
+ oxp_page_fill_data(page_2, p2, ARRAY_SIZE(p2));
+
+ ret = oxp_gen_2_property_out(OXP_FID_GEN2_KEY_STATE, page_1, ARRAY_SIZE(page_1));
+ if (ret)
+ return ret;
+
+ return oxp_gen_2_property_out(OXP_FID_GEN2_KEY_STATE, page_2, ARRAY_SIZE(page_2));
+}
+
+static void oxp_reset_buttons(void)
+{
+ oxp_set_defaults_bmap_1(drvdata.bmap_1);
+ oxp_set_defaults_bmap_2(drvdata.bmap_2);
+}
+
+static ssize_t reset_buttons_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ int val, ret;
+
+ ret = kstrtoint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val != 1)
+ return -EINVAL;
+
+ oxp_reset_buttons();
+ ret = oxp_set_buttons();
+ if (ret)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_WO(reset_buttons);
+
+static void oxp_btn_queue_fn(struct work_struct *work)
+{
+ int ret;
+
+ ret = oxp_set_buttons();
+ if (ret)
+ dev_err(&drvdata.hdev->dev,
+ "Error: Failed to write button mapping: %i\n", ret);
+}
+
+static int oxp_button_idx_from_str(const char *buf)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(oxp_button_table); i++)
+ if (sysfs_streq(buf, oxp_button_table[i].name))
+ return i;
+
+ return -EINVAL;
+}
+
+static ssize_t map_button_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count, u8 index)
+{
+ int idx;
+
+ idx = oxp_button_idx_from_str(buf);
+ if (idx < 0)
+ return idx;
+
+ switch (index) {
+ case BUTTON_A:
+ drvdata.bmap_1->btn_a.mapping_idx = idx;
+ break;
+ case BUTTON_B:
+ drvdata.bmap_1->btn_b.mapping_idx = idx;
+ break;
+ case BUTTON_X:
+ drvdata.bmap_1->btn_x.mapping_idx = idx;
+ break;
+ case BUTTON_Y:
+ drvdata.bmap_1->btn_y.mapping_idx = idx;
+ break;
+ case BUTTON_LB:
+ drvdata.bmap_1->btn_lb.mapping_idx = idx;
+ break;
+ case BUTTON_RB:
+ drvdata.bmap_1->btn_rb.mapping_idx = idx;
+ break;
+ case BUTTON_LT:
+ drvdata.bmap_1->btn_lt.mapping_idx = idx;
+ break;
+ case BUTTON_RT:
+ drvdata.bmap_1->btn_rt.mapping_idx = idx;
+ break;
+ case BUTTON_START:
+ drvdata.bmap_1->btn_start.mapping_idx = idx;
+ break;
+ case BUTTON_SELECT:
+ drvdata.bmap_2->btn_select.mapping_idx = idx;
+ break;
+ case BUTTON_L3:
+ drvdata.bmap_2->btn_l3.mapping_idx = idx;
+ break;
+ case BUTTON_R3:
+ drvdata.bmap_2->btn_r3.mapping_idx = idx;
+ break;
+ case BUTTON_DUP:
+ drvdata.bmap_2->btn_dup.mapping_idx = idx;
+ break;
+ case BUTTON_DDOWN:
+ drvdata.bmap_2->btn_ddown.mapping_idx = idx;
+ break;
+ case BUTTON_DLEFT:
+ drvdata.bmap_2->btn_dleft.mapping_idx = idx;
+ break;
+ case BUTTON_DRIGHT:
+ drvdata.bmap_2->btn_dright.mapping_idx = idx;
+ break;
+ case BUTTON_M1:
+ drvdata.bmap_2->btn_m1.mapping_idx = idx;
+ break;
+ case BUTTON_M2:
+ drvdata.bmap_2->btn_m2.mapping_idx = idx;
+ break;
+ default:
+ return -EINVAL;
+ }
+ mod_delayed_work(system_wq, &drvdata.oxp_btn_queue, msecs_to_jiffies(50));
+ return count;
+}
+
+static ssize_t map_button_show(struct device *dev,
+ struct device_attribute *attr, char *buf,
+ u8 index)
+{
+ u8 i;
+
+ switch (index) {
+ case BUTTON_A:
+ i = drvdata.bmap_1->btn_a.mapping_idx;
+ break;
+ case BUTTON_B:
+ i = drvdata.bmap_1->btn_b.mapping_idx;
+ break;
+ case BUTTON_X:
+ i = drvdata.bmap_1->btn_x.mapping_idx;
+ break;
+ case BUTTON_Y:
+ i = drvdata.bmap_1->btn_y.mapping_idx;
+ break;
+ case BUTTON_LB:
+ i = drvdata.bmap_1->btn_lb.mapping_idx;
+ break;
+ case BUTTON_RB:
+ i = drvdata.bmap_1->btn_rb.mapping_idx;
+ break;
+ case BUTTON_LT:
+ i = drvdata.bmap_1->btn_lt.mapping_idx;
+ break;
+ case BUTTON_RT:
+ i = drvdata.bmap_1->btn_rt.mapping_idx;
+ break;
+ case BUTTON_START:
+ i = drvdata.bmap_1->btn_start.mapping_idx;
+ break;
+ case BUTTON_SELECT:
+ i = drvdata.bmap_2->btn_select.mapping_idx;
+ break;
+ case BUTTON_L3:
+ i = drvdata.bmap_2->btn_l3.mapping_idx;
+ break;
+ case BUTTON_R3:
+ i = drvdata.bmap_2->btn_r3.mapping_idx;
+ break;
+ case BUTTON_DUP:
+ i = drvdata.bmap_2->btn_dup.mapping_idx;
+ break;
+ case BUTTON_DDOWN:
+ i = drvdata.bmap_2->btn_ddown.mapping_idx;
+ break;
+ case BUTTON_DLEFT:
+ i = drvdata.bmap_2->btn_dleft.mapping_idx;
+ break;
+ case BUTTON_DRIGHT:
+ i = drvdata.bmap_2->btn_dright.mapping_idx;
+ break;
+ case BUTTON_M1:
+ i = drvdata.bmap_2->btn_m1.mapping_idx;
+ break;
+ case BUTTON_M2:
+ i = drvdata.bmap_2->btn_m2.mapping_idx;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (i >= ARRAY_SIZE(oxp_button_table))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", oxp_button_table[i].name);
+}
+
+static ssize_t button_mapping_options_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ssize_t count = 0;
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(oxp_button_table); i++)
+ count += sysfs_emit_at(buf, count, "%s ", oxp_button_table[i].name);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(button_mapping_options);
+
+#define OXP_DEVICE_ATTR_RW(_name, _group) \
+ static ssize_t _name##_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ return _group##_store(dev, attr, buf, count, _name.index); \
+ } \
+ static ssize_t _name##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+ { \
+ return _group##_show(dev, attr, buf, _name.index); \
+ } \
+ static DEVICE_ATTR_RW(_name)
+
+static struct oxp_attr button_a = { BUTTON_A };
+OXP_DEVICE_ATTR_RW(button_a, map_button);
+
+static struct oxp_attr button_b = { BUTTON_B };
+OXP_DEVICE_ATTR_RW(button_b, map_button);
+
+static struct oxp_attr button_x = { BUTTON_X };
+OXP_DEVICE_ATTR_RW(button_x, map_button);
+
+static struct oxp_attr button_y = { BUTTON_Y };
+OXP_DEVICE_ATTR_RW(button_y, map_button);
+
+static struct oxp_attr button_lb = { BUTTON_LB };
+OXP_DEVICE_ATTR_RW(button_lb, map_button);
+
+static struct oxp_attr button_rb = { BUTTON_RB };
+OXP_DEVICE_ATTR_RW(button_rb, map_button);
+
+static struct oxp_attr button_lt = { BUTTON_LT };
+OXP_DEVICE_ATTR_RW(button_lt, map_button);
+
+static struct oxp_attr button_rt = { BUTTON_RT };
+OXP_DEVICE_ATTR_RW(button_rt, map_button);
+
+static struct oxp_attr button_start = { BUTTON_START };
+OXP_DEVICE_ATTR_RW(button_start, map_button);
+
+static struct oxp_attr button_select = { BUTTON_SELECT };
+OXP_DEVICE_ATTR_RW(button_select, map_button);
+
+static struct oxp_attr button_l3 = { BUTTON_L3 };
+OXP_DEVICE_ATTR_RW(button_l3, map_button);
+
+static struct oxp_attr button_r3 = { BUTTON_R3 };
+OXP_DEVICE_ATTR_RW(button_r3, map_button);
+
+static struct oxp_attr button_d_up = { BUTTON_DUP };
+OXP_DEVICE_ATTR_RW(button_d_up, map_button);
+
+static struct oxp_attr button_d_down = { BUTTON_DDOWN };
+OXP_DEVICE_ATTR_RW(button_d_down, map_button);
+
+static struct oxp_attr button_d_left = { BUTTON_DLEFT };
+OXP_DEVICE_ATTR_RW(button_d_left, map_button);
+
+static struct oxp_attr button_d_right = { BUTTON_DRIGHT };
+OXP_DEVICE_ATTR_RW(button_d_right, map_button);
+
+static struct oxp_attr button_m1 = { BUTTON_M1 };
+OXP_DEVICE_ATTR_RW(button_m1, map_button);
+
+static struct oxp_attr button_m2 = { BUTTON_M2 };
+OXP_DEVICE_ATTR_RW(button_m2, map_button);
+
static struct attribute *oxp_cfg_attrs[] = {
+ &dev_attr_button_a.attr,
+ &dev_attr_button_b.attr,
+ &dev_attr_button_d_down.attr,
+ &dev_attr_button_d_left.attr,
+ &dev_attr_button_d_right.attr,
+ &dev_attr_button_d_up.attr,
+ &dev_attr_button_l3.attr,
+ &dev_attr_button_lb.attr,
+ &dev_attr_button_lt.attr,
+ &dev_attr_button_m1.attr,
+ &dev_attr_button_m2.attr,
+ &dev_attr_button_mapping_options.attr,
+ &dev_attr_button_r3.attr,
+ &dev_attr_button_rb.attr,
+ &dev_attr_button_rt.attr,
+ &dev_attr_button_select.attr,
+ &dev_attr_button_start.attr,
+ &dev_attr_button_x.attr,
+ &dev_attr_button_y.attr,
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
+ &dev_attr_reset_buttons.attr,
NULL,
};
@@ -821,6 +1371,8 @@ static bool oxp_hybrid_mcu_device(void)
static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
{
+ struct oxp_bmap_page_1 *bmap_1;
+ struct oxp_bmap_page_2 *bmap_2;
int ret;
hid_set_drvdata(hdev, &drvdata);
@@ -854,6 +1406,21 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
return 0;
skip_rgb:
+ bmap_1 = devm_kzalloc(&hdev->dev, sizeof(struct oxp_bmap_page_1), GFP_KERNEL);
+ if (!bmap_1)
+ return dev_err_probe(&hdev->dev, -ENOMEM,
+ "Unable to allocate button map page 1\n");
+
+ bmap_2 = devm_kzalloc(&hdev->dev, sizeof(struct oxp_bmap_page_2), GFP_KERNEL);
+ if (!bmap_2)
+ return dev_err_probe(&hdev->dev, -ENOMEM,
+ "Unable to allocate button map page 2\n");
+
+ drvdata.bmap_1 = bmap_1;
+ drvdata.bmap_2 = bmap_2;
+ oxp_reset_buttons();
+ INIT_DELAYED_WORK(&drvdata.oxp_btn_queue, oxp_btn_queue_fn);
+
drvdata.gamepad_mode = OXP_GP_MODE_XINPUT;
INIT_DELAYED_WORK(&drvdata.oxp_mcu_init, oxp_mcu_init_fn);
@@ -908,6 +1475,7 @@ static int oxp_hid_probe(struct hid_device *hdev,
static void oxp_hid_remove(struct hid_device *hdev)
{
cancel_delayed_work(&drvdata.oxp_rgb_queue);
+ cancel_delayed_work(&drvdata.oxp_btn_queue);
cancel_delayed_work(&drvdata.oxp_mcu_init);
hid_hw_close(hdev);
hid_hw_stop(hdev);
--
2.53.0
^ permalink raw reply related
* [PATCH v4 5/5] HID: hid-oxp: Add Vibration Intensity Attribute
From: Derek J. Clark @ 2026-04-19 4:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Lambert Fan, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260419042624.625746-1-derekjohn.clark@gmail.com>
Adds attribute for setting the rumble intensity level. This setting must
be re-applied after the gamepad mode is set as doing so resets this to
the default value.
Reviewed-by: Zhouwang Huang <honjow311@gmail.com>
Tested-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
drivers/hid/hid-oxp.c | 78 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
index 52002d4cbd0b..20a54f337220 100644
--- a/drivers/hid/hid-oxp.c
+++ b/drivers/hid/hid-oxp.c
@@ -34,6 +34,7 @@ enum oxp_function_index {
OXP_FID_GEN1_RGB_SET = 0x07,
OXP_FID_GEN1_RGB_REPLY = 0x0f,
OXP_FID_GEN2_TOGGLE_MODE = 0xb2,
+ OXP_FID_GEN2_RUMBLE_SET = 0xb3,
OXP_FID_GEN2_KEY_STATE = 0xb4,
OXP_FID_GEN2_STATUS_EVENT = 0xb8,
};
@@ -181,6 +182,7 @@ static struct oxp_hid_cfg {
struct mutex cfg_mutex; /*ensure single synchronous output report*/
u8 rgb_brightness;
u8 gamepad_mode;
+ u8 rumble_intensity;
u8 rgb_effect;
u8 rgb_speed;
u8 rgb_en;
@@ -266,6 +268,11 @@ static const char *const oxp_rgb_effect_text[] = {
[OXP_EFFECT_MONO_LIST] = "monocolor",
};
+enum oxp_rumble_side_index {
+ OXP_RUMBLE_LEFT = 0x00,
+ OXP_RUMBLE_RIGHT,
+};
+
struct oxp_gen_1_rgb_report {
u8 report_id;
u8 message_id;
@@ -341,6 +348,7 @@ static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
static int oxp_gen_2_property_out(enum oxp_function_index fid, u8 *data, u8 data_size);
static int oxp_set_buttons(void);
+static int oxp_rumble_intensity_set(u8 intensity);
static void oxp_mcu_init_fn(struct work_struct *work)
{
@@ -368,6 +376,12 @@ static void oxp_mcu_init_fn(struct work_struct *work)
if (ret)
dev_err(&drvdata.hdev->dev,
"Error: Failed to set gamepad mode: %i\n", ret);
+
+ /* Set vibration level */
+ ret = oxp_rumble_intensity_set(drvdata.rumble_intensity);
+ if (ret)
+ dev_err(&drvdata.hdev->dev,
+ "Error: Failed to set rumble intensity: %i\n", ret);
}
static int oxp_hid_raw_event_gen_2(struct hid_device *hdev,
@@ -514,6 +528,14 @@ static ssize_t gamepad_mode_store(struct device *dev,
drvdata.gamepad_mode = data[0];
+ if (drvdata.gamepad_mode == OXP_GP_MODE_DEBUG)
+ return count;
+
+ /* Re-apply rumble settings as switching gamepad mode will override */
+ ret = oxp_rumble_intensity_set(drvdata.rumble_intensity);
+ if (ret)
+ return ret;
+
return count;
}
@@ -857,6 +879,59 @@ static ssize_t button_mapping_options_show(struct device *dev,
}
static DEVICE_ATTR_RO(button_mapping_options);
+static int oxp_rumble_intensity_set(u8 intensity)
+{
+ u8 header[15] = { 0x02, 0x38, 0x02, 0xe3, 0x39, 0xe3, 0x39, 0xe3,
+ 0x39, 0x01, intensity, 0x05, 0xe3, 0x39, 0xe3 };
+ u8 footer[9] = { 0x39, 0xe3, 0x39, 0xe3, 0xe3, 0x02, 0x04, 0x39, 0x39 };
+ size_t footer_size = ARRAY_SIZE(footer);
+ size_t header_size = ARRAY_SIZE(header);
+ u8 data[59] = { 0x0 };
+ size_t data_size = ARRAY_SIZE(data);
+
+ memcpy(data, header, header_size);
+ memcpy(data + data_size - footer_size, footer, footer_size);
+
+ return oxp_gen_2_property_out(OXP_FID_GEN2_RUMBLE_SET, data, data_size);
+}
+
+static ssize_t rumble_intensity_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ int ret;
+ u8 val;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val < 0 || val > 5)
+ return -EINVAL;
+
+ ret = oxp_rumble_intensity_set(val);
+ if (ret)
+ return ret;
+
+ drvdata.rumble_intensity = val;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%i\n", drvdata.rumble_intensity);
+}
+static DEVICE_ATTR_RW(rumble_intensity);
+
+static ssize_t rumble_intensity_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0-5\n");
+}
+static DEVICE_ATTR_RO(rumble_intensity_range);
+
#define OXP_DEVICE_ATTR_RW(_name, _group) \
static ssize_t _name##_store(struct device *dev, \
struct device_attribute *attr, \
@@ -948,6 +1023,8 @@ static struct attribute *oxp_cfg_attrs[] = {
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
&dev_attr_reset_buttons.attr,
+ &dev_attr_rumble_intensity.attr,
+ &dev_attr_rumble_intensity_range.attr,
NULL,
};
@@ -1422,6 +1499,7 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
INIT_DELAYED_WORK(&drvdata.oxp_btn_queue, oxp_btn_queue_fn);
drvdata.gamepad_mode = OXP_GP_MODE_XINPUT;
+ drvdata.rumble_intensity = 5;
INIT_DELAYED_WORK(&drvdata.oxp_mcu_init, oxp_mcu_init_fn);
mod_delayed_work(system_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 0/7] iio: HID: Add helper method hid_sensor_adjust_channel_bit_mask()
From: Jonathan Cameron @ 2026-04-19 13:10 UTC (permalink / raw)
To: Natália Salvino André
Cc: andy, bentiss, dlechner, jikos, nuno.sa, srinivas.pandruvada,
linux-iio, linux-input
In-Reply-To: <20260417225959.16688-1-natalia.andre@ime.usp.br>
On Fri, 17 Apr 2026 19:58:43 -0300
Natália Salvino André <natalia.andre@ime.usp.br> wrote:
> This patch series introduces a generic helper function to handle channel bit mask adjustments
> for HID sensors. Currently, multiple drivers implement identical logic for this task
>
Other than the stuff David already raised, all looks fine to me.
> Natália Salvino André (7):
> iio: HID: Add helper method hid_sensor_adjust_channel_bit_mask()
> iio: accel: HID: Replace method accel_3d_adjust_channel_bit_mask()
> iio: gyro: HID: Replace method gyro_3d_adjust_channel_bit_mask()
> iio: light: HID: Replace method als_adjust_channel_bit_mask()
> iio: light: HID: Replace method prox_adjust_channel_bit_mask()
> iio: magnetometer: HID: Replace method
> magn_3d_adjust_channel_bit_mask()
> iio: pressure: HID: Replace method press_adjust_channel_bit_mask()
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 13 +------------
> .../iio/common/hid-sensors/hid-sensor-attributes.c | 11 +++++++++++
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 13 +------------
> drivers/iio/light/hid-sensor-als.c | 13 +------------
> drivers/iio/light/hid-sensor-prox.c | 13 +------------
> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 13 +------------
> drivers/iio/pressure/hid-sensor-press.c | 13 +------------
> include/linux/hid-sensor-hub.h | 3 +++
> 8 files changed, 20 insertions(+), 72 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v2 00/19] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Vineeth Remanan Pillai @ 2026-04-19 13:14 UTC (permalink / raw)
To: Steven Rostedt
Cc: Peter Zijlstra, Dmitry Ilvokhin, Masami Hiramatsu,
Mathieu Desnoyers, Ingo Molnar, Jens Axboe, io-uring,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner,
Xin Long, Jon Maloy, Aaron Conole, Eelco Chaudron, Ilya Maximets,
netdev, bpf, linux-sctp, tipc-discussion, dev, Jiri Pirko,
Oded Gabbay, Koby Elbaz, dri-devel, Rafael J. Wysocki,
Viresh Kumar, Gautham R. Shenoy, Huang Rui, Mario Limonciello,
Len Brown, Srinivas Pandruvada, linux-pm, MyungJoo Ham,
Kyungmin Park, Chanwoo Choi, Christian König, Sumit Semwal,
linaro-mm-sig, Eddie James, Andrew Jeffery, Joel Stanley,
linux-fsi, David Airlie, Simona Vetter, Alex Deucher,
Danilo Krummrich, Matthew Brost, Philipp Stanner, Harry Wentland,
Leo Li, amd-gfx, Jiri Kosina, Benjamin Tissoires, linux-input,
Wolfram Sang, linux-i2c, Mark Brown, Michael Hennerich,
Nuno Sá, linux-spi, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, Chris Mason, David Sterba, linux-btrfs,
Thomas Gleixner, Andrew Morton, SeongJae Park, linux-mm,
Borislav Petkov, Dave Hansen, x86, linux-trace-kernel,
linux-kernel
In-Reply-To: <20260418190456.631df6f3@fedora>
On Sat, Apr 18, 2026 at 7:05 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon, 23 Mar 2026 12:00:19 -0400
> "Vineeth Pillai (Google)" <vineeth@bitbyteword.org> wrote:
>
> > if (trace_foo_enabled() && cond)
> > trace_call__foo(args); /* calls __do_trace_foo() directly */
>
> Hi Vineeth,
>
> Could you rebase this series on top of 7.1-rc1 when it comes out?
> Several of these patches were accepted already. Obviously drop those.
> They were the patches that added the feature, and any where the
> maintainer acked the patch.
>
> Now that the feature has been accepted, if you post the patch series
> again after 7.1-rc1 with all the patches that haven't been accepted
> yet, then the maintainers can simply take them directly. As the feature
> is now accepted, there's no dependency on it, and they don't need to go
> through the tracing tree.
>
Sure, will do. Thanks for merging this feature.
Thanks,
Vineeth
^ permalink raw reply
* [PATCH v2 0/2] Input/omap1: fix touchscreen functionality on Nokia 770
From: Aaro Koskinen @ 2026-04-19 16:18 UTC (permalink / raw)
To: Dmitry Torokhov, Oleksij Rempel, Janusz Krzysztofik,
Tony Lindgren, Linus Walleij, linux-input
Cc: linux-kernel, linux-omap, Aaro Koskinen
Hi,
Another attempt to make 770 touchscreen work again.
v2: Unrelated fbdev patch dropped (already merged)
GPIO setup fixed by deleting the conflicting descriptor.
v1: https://lore.kernel.org/linux-input/20250102181953.1020878-1-aaro.koskinen@iki.fi/
Aaro Koskinen (2):
Input: ads7846 - restore half-duplex support
Input: ads7846 - fix up the pendown GPIO setup on Nokia 770
arch/arm/mach-omap1/board-nokia770.c | 11 --
drivers/input/touchscreen/ads7846.c | 180 +++++++++++++++++++++++++--
2 files changed, 173 insertions(+), 18 deletions(-)
--
2.39.2
^ permalink raw reply
* [PATCH v2 1/2] Input: ads7846 - restore half-duplex support
From: Aaro Koskinen @ 2026-04-19 16:18 UTC (permalink / raw)
To: Dmitry Torokhov, Oleksij Rempel, Janusz Krzysztofik,
Tony Lindgren, Linus Walleij, linux-input
Cc: linux-kernel, linux-omap, Aaro Koskinen
In-Reply-To: <20260419161848.825831-1-aaro.koskinen@iki.fi>
On some boards, the SPI controller is limited to half-duplex and the driver
fails spamming "ads7846 spi2.1: spi_sync --> -22". Restore half-duplex
support with multiple SPI transfers.
Fixes: 9c9509717b53 ("Input: ads7846 - convert to full duplex")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
drivers/input/touchscreen/ads7846.c | 168 +++++++++++++++++++++++++++-
1 file changed, 166 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 0963b1a78a0c..4f8cc450e779 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -134,6 +134,9 @@ struct ads7846 {
bool disabled; /* P: lock */
bool suspended; /* P: lock */
+ int (*setup_spi_msg)(struct ads7846 *ts,
+ const struct ads7846_platform_data *pdata);
+ void (*read_state)(struct ads7846 *ts);
int (*filter)(void *data, int data_idx, int *val);
void *filter_data;
int (*get_pendown_state)(void);
@@ -797,6 +800,22 @@ static int ads7846_filter(struct ads7846 *ts)
return 0;
}
+static int ads7846_filter_one(struct ads7846 *ts, unsigned int cmd_idx)
+{
+ struct ads7846_packet *packet = ts->packet;
+ struct ads7846_buf_layout *l = &packet->l[cmd_idx];
+ int action, val;
+
+ val = ads7846_get_value(&packet->rx[l->offset + l->count - 1]);
+ action = ts->filter(ts->filter_data, cmd_idx, &val);
+ if (action == ADS7846_FILTER_REPEAT)
+ return -EAGAIN;
+ else if (action != ADS7846_FILTER_OK)
+ return -EIO;
+ ads7846_set_cmd_val(ts, cmd_idx, val);
+ return 0;
+}
+
static void ads7846_wait_for_hsync(struct ads7846 *ts)
{
if (ts->wait_for_sync) {
@@ -819,6 +838,45 @@ static void ads7846_wait_for_hsync(struct ads7846 *ts)
cpu_relax();
}
+static void ads7846_halfd_read_state(struct ads7846 *ts)
+{
+ struct ads7846_packet *packet = ts->packet;
+ int msg_idx = 0;
+
+ packet->ignore = false;
+
+ while (msg_idx < ts->msg_count) {
+ int error;
+
+ ads7846_wait_for_hsync(ts);
+
+ error = spi_sync(ts->spi, &ts->msg[msg_idx]);
+ if (error) {
+ dev_err_ratelimited(&ts->spi->dev, "spi_sync --> %d\n",
+ error);
+ packet->ignore = true;
+ return;
+ }
+
+ /*
+ * Last message is power down request, no need to convert
+ * or filter the value.
+ */
+ if (msg_idx == ts->msg_count - 1)
+ break;
+
+ error = ads7846_filter_one(ts, msg_idx);
+ if (error == -EAGAIN) {
+ continue;
+ } else if (error) {
+ packet->ignore = true;
+ msg_idx = ts->msg_count - 1;
+ } else {
+ msg_idx++;
+ }
+ }
+}
+
static void ads7846_read_state(struct ads7846 *ts)
{
struct ads7846_packet *packet = ts->packet;
@@ -947,7 +1005,7 @@ static irqreturn_t ads7846_irq(int irq, void *handle)
while (!ts->stopped && get_pendown_state(ts)) {
/* pen is down, continue with the measurement */
- ads7846_read_state(ts);
+ ts->read_state(ts);
if (!ts->stopped)
ads7846_report_state(ts);
@@ -1034,6 +1092,102 @@ static int ads7846_setup_pendown(struct spi_device *spi,
return 0;
}
+/*
+ * Set up the transfers to read touchscreen state; this assumes we
+ * use formula #2 for pressure, not #3.
+ */
+static int ads7846_halfd_spi_msg(struct ads7846 *ts,
+ const struct ads7846_platform_data *pdata)
+{
+ struct spi_message *m = ts->msg;
+ struct spi_transfer *x = ts->xfer;
+ struct ads7846_packet *packet = ts->packet;
+ int vref = pdata->keep_vref_on;
+ unsigned int offset = 0;
+ unsigned int cmd_idx, b;
+ size_t size = 0;
+
+ if (pdata->settle_delay_usecs)
+ packet->count = 2;
+ else
+ packet->count = 1;
+
+ if (ts->model == 7846)
+ packet->cmds = 5; /* x, y, z1, z2, pwdown */
+ else
+ packet->cmds = 3; /* x, y, pwdown */
+
+ for (cmd_idx = 0; cmd_idx < packet->cmds; cmd_idx++) {
+ struct ads7846_buf_layout *l = &packet->l[cmd_idx];
+ unsigned int max_count;
+
+ if (cmd_idx == packet->cmds - 1) {
+ cmd_idx = ADS7846_PWDOWN;
+ max_count = 1;
+ } else {
+ max_count = packet->count;
+ }
+
+ l->offset = offset;
+ offset += max_count;
+ l->count = max_count;
+ l->skip = 0;
+ size += sizeof(*packet->rx) * max_count;
+ }
+
+ /* We use two transfers per command. */
+ if (ARRAY_SIZE(ts->xfer) < offset * 2)
+ return -ENOMEM;
+
+ packet->rx = devm_kzalloc(&ts->spi->dev, size, GFP_KERNEL);
+ if (!packet->rx)
+ return -ENOMEM;
+
+ if (ts->model == 7873) {
+ /*
+ * The AD7873 is almost identical to the ADS7846
+ * keep VREF off during differential/ratiometric
+ * conversion modes.
+ */
+ ts->model = 7846;
+ vref = 0;
+ }
+
+ ts->msg_count = 0;
+
+ for (cmd_idx = 0; cmd_idx < packet->cmds; cmd_idx++) {
+ struct ads7846_buf_layout *l = &packet->l[cmd_idx];
+ u8 cmd;
+
+ ts->msg_count++;
+ spi_message_init(m);
+ m->context = ts;
+
+ if (cmd_idx == packet->cmds - 1)
+ cmd_idx = ADS7846_PWDOWN;
+
+ cmd = ads7846_get_cmd(cmd_idx, vref);
+
+ for (b = 0; b < l->count; b++) {
+ packet->rx[l->offset + b].cmd = cmd;
+ x->tx_buf = &packet->rx[l->offset + b].cmd;
+ x->len = 1;
+ spi_message_add_tail(x, m);
+ x++;
+ x->rx_buf = &packet->rx[l->offset + b].data;
+ x->len = 2;
+ if (b < l->count - 1 && l->count > 1) {
+ x->delay.value = pdata->settle_delay_usecs;
+ x->delay.unit = SPI_DELAY_UNIT_USECS;
+ }
+ spi_message_add_tail(x, m);
+ x++;
+ }
+ m++;
+ }
+ return 0;
+}
+
/*
* Set up the transfers to read touchscreen state; this assumes we
* use formula #2 for pressure, not #3.
@@ -1248,6 +1402,14 @@ static int ads7846_probe(struct spi_device *spi)
if (!ts)
return -ENOMEM;
+ if (spi->controller->flags & SPI_CONTROLLER_HALF_DUPLEX) {
+ ts->setup_spi_msg = ads7846_halfd_spi_msg;
+ ts->read_state = ads7846_halfd_read_state;
+ } else {
+ ts->setup_spi_msg = ads7846_setup_spi_msg;
+ ts->read_state = ads7846_read_state;
+ }
+
packet = devm_kzalloc(dev, sizeof(struct ads7846_packet), GFP_KERNEL);
if (!packet)
return -ENOMEM;
@@ -1342,7 +1504,9 @@ static int ads7846_probe(struct spi_device *spi)
ts->core_prop.swap_x_y = true;
}
- ads7846_setup_spi_msg(ts, pdata);
+ err = ts->setup_spi_msg(ts, pdata);
+ if (err)
+ return err;
ts->reg = devm_regulator_get(dev, "vcc");
if (IS_ERR(ts->reg)) {
--
2.39.2
^ permalink raw reply related
* [PATCH v2 2/2] Input: ads7846 - fix up the pendown GPIO setup on Nokia 770
From: Aaro Koskinen @ 2026-04-19 16:18 UTC (permalink / raw)
To: Dmitry Torokhov, Oleksij Rempel, Janusz Krzysztofik,
Tony Lindgren, Linus Walleij, linux-input
Cc: linux-kernel, linux-omap, Aaro Koskinen
In-Reply-To: <20260419161848.825831-1-aaro.koskinen@iki.fi>
Commit 767d83361aaa6 ("Input: ads7846 - Convert to use software nodes")
added gpiod set up for the IRQ in the 770 board file, then another in
the touchscreen driver for reading the pen state. This will make the probe
fail:
[ 1.347381] ads7846 spi2.0: failed to request pendown GPIO
[ 1.361846] ads7846: probe of spi2.0 failed with error -16
I originally tried to fix it using non-exclusive flag, but that was not
found acceptable. Instead, just use a single gpiod.
Fixes: 767d83361aaa6 ("Input: ads7846 - Convert to use software nodes")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
arch/arm/mach-omap1/board-nokia770.c | 11 -----------
drivers/input/touchscreen/ads7846.c | 12 +++++++-----
2 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
index a5bf5554800f..8b8013ab4590 100644
--- a/arch/arm/mach-omap1/board-nokia770.c
+++ b/arch/arm/mach-omap1/board-nokia770.c
@@ -285,9 +285,6 @@ static void __init nokia770_cbus_init(void)
static struct gpiod_lookup_table nokia770_irq_gpio_table = {
.dev_id = NULL,
.table = {
- /* GPIO used by SPI device 1 */
- GPIO_LOOKUP("gpio-0-15", 15, "ads7846_irq",
- GPIO_ACTIVE_HIGH),
/* GPIO used for retu IRQ */
GPIO_LOOKUP("gpio-48-63", 14, "retu_irq",
GPIO_ACTIVE_HIGH),
@@ -307,8 +304,6 @@ static struct gpiod_lookup_table nokia770_irq_gpio_table = {
static void __init omap_nokia770_init(void)
{
- struct gpio_desc *d;
-
/* On Nokia 770, the SleepX signal is masked with an
* MPUIO line by default. It has to be unmasked for it
* to become functional */
@@ -322,12 +317,6 @@ static void __init omap_nokia770_init(void)
platform_add_devices(nokia770_devices, ARRAY_SIZE(nokia770_devices));
gpiod_add_lookup_table(&nokia770_irq_gpio_table);
- d = gpiod_get(NULL, "ads7846_irq", GPIOD_IN);
- if (IS_ERR(d))
- pr_err("Unable to get ADS7846 IRQ GPIO descriptor\n");
- else
- nokia770_spi_board_info[1].irq = gpiod_to_irq(d);
-
spi_register_board_info(nokia770_spi_board_info,
ARRAY_SIZE(nokia770_spi_board_info));
omap_serial_init();
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 4f8cc450e779..ca7dbd3afe29 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -1084,6 +1084,8 @@ static int ads7846_setup_pendown(struct spi_device *spi,
dev_err(&spi->dev, "failed to request pendown GPIO\n");
return PTR_ERR(ts->gpio_pendown);
}
+ if (!spi->irq)
+ spi->irq = gpiod_to_irq(ts->gpio_pendown);
if (pdata->gpio_pendown_debounce)
gpiod_set_debounce(ts->gpio_pendown,
pdata->gpio_pendown_debounce);
@@ -1374,11 +1376,6 @@ static int ads7846_probe(struct spi_device *spi)
unsigned long irq_flags;
int err;
- if (!spi->irq) {
- dev_dbg(dev, "no IRQ?\n");
- return -EINVAL;
- }
-
/* don't exceed max specified sample rate */
if (spi->max_speed_hz > (125000 * SAMPLE_BITS)) {
dev_err(dev, "f(sample) %d KHz?\n",
@@ -1455,6 +1452,11 @@ static int ads7846_probe(struct spi_device *spi)
if (err)
return err;
+ if (!spi->irq) {
+ dev_dbg(dev, "no IRQ?\n");
+ return -EINVAL;
+ }
+
if (pdata->penirq_recheck_delay_usecs)
ts->penirq_recheck_delay_usecs =
pdata->penirq_recheck_delay_usecs;
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v2 1/4] HID: pass the buffer size to hid_report_raw_event
From: Icenowy Zheng @ 2026-04-19 16:26 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina, Filipe Laíns,
Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, Lee Jones
Cc: linux-input, linux-kernel, greybus-dev, linux-staging, linux-usb,
stable
In-Reply-To: <20260416-wip-fix-core-v2-1-be92570e5627@kernel.org>
在 2026-04-16四的 16:48 +0200，Benjamin Tissoires写道：
> commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()") enforced the provided data to be at least the size
> of
> the declared buffer in the report descriptor to prevent a buffer
> overflow. However, we can try to be smarter by providing both the
> buffer
> size and the data size, meaning that hid_report_raw_event() can make
> better decision whether we should plaining reject the buffer (buffer
> overflow attempt) or if we can safely memset it to 0 and pass it to
> the
> rest of the stack.
>
> Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> drivers/hid/bpf/hid_bpf_dispatch.c | 6 ++++--
> drivers/hid/hid-core.c | 42 +++++++++++++++++++++++++---
> ----------
> drivers/hid/hid-gfrm.c | 4 ++--
> drivers/hid/hid-logitech-hidpp.c | 2 +-
> drivers/hid/hid-multitouch.c | 2 +-
> drivers/hid/hid-primax.c | 2 +-
> drivers/hid/hid-vivaldi-common.c | 2 +-
> drivers/hid/wacom_sys.c | 6 +++---
> drivers/staging/greybus/hid.c | 2 +-
> include/linux/hid.h | 4 ++--
> include/linux/hid_bpf.h | 14 ++++++++-----
> 11 files changed, 53 insertions(+), 33 deletions(-)
============ 8< ===================
> diff --git a/drivers/staging/greybus/hid.c
> b/drivers/staging/greybus/hid.c
> index 1f58c907c036..37e8605c6767 100644
> --- a/drivers/staging/greybus/hid.c
> +++ b/drivers/staging/greybus/hid.c
> @@ -201,7 +201,7 @@ static void gb_hid_init_report(struct gb_hid
> *ghid, struct hid_report *report)
> * we just need to setup the input fields, so using
> * hid_report_raw_event is safe.
> */
> - hid_report_raw_event(ghid->hid, report->type, ghid->inbuf,
> size, 1);
> + hid_report_raw_event(ghid->hid, report->type, ghid->inbuf,
> ghib->bufsize, size, 1);
Oops, "ghid" is misspelled here...
Found this when building some gaint kernel with this patchset.
Thanks,
Icenowy
> }
>
> static void gb_hid_init_reports(struct gb_hid *ghid)
^ permalink raw reply
* [PATCH v2] iio: magnetometer: hid-sensor-magn-3d: prefer 'unsigned int'
From: Joshua Crofts @ 2026-04-19 18:05 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada
Cc: dlechner, nuno.sa, andy, linux-input, linux-iio, linux-kernel,
Joshua Crofts
Use 'u32' instead of bare 'unsigned' to resolve checkpatch.pl warnings
and correct type use as defined in the hid_sensor_hub_callbacks struct.
No functional change.
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
v2:
- changed 'unsigned int' to 'u32' per struct definition
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index c673f9323e..b01dd53eb1 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -280,7 +280,7 @@ static const struct iio_info magn_3d_info = {
/* Callback handler to send event after all samples are received and captured */
static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -302,7 +302,7 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
size_t raw_len, char *raw_data,
void *priv)
{
@@ -350,7 +350,7 @@ static int magn_3d_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec **channels,
int *chan_count,
- unsigned usage_id,
+ u32 usage_id,
struct magn_3d_state *st)
{
int i;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 2/3] Input: adafruit-seesaw - add interrupt support
From: Charles Dias @ 2026-04-19 19:24 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Anshul Dalal, Shuah Khan, Brigham Campbell, linux-input,
linux-kernel, Charles Dias
In-Reply-To: <CAHua+AMP-gwgFCX67rdDrfS8RUGpiw2msKYRn07AVWy+u_vTwQ@mail.gmail.com>
On Tue, Mar 24, 2026 at 10:59 AM Charles Dias
<charles.embedded@gmail.com> wrote:
>
> On Mon, Mar 23, 2026 at 2:12 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Charles,
> >
> > On Sat, Mar 21, 2026 at 05:24:45PM -0300, charles.embedded@gmail.com wrote:
> > > @@ -289,6 +341,19 @@ static int seesaw_probe(struct i2c_client *client)
> > > input_set_max_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MAX);
> > > input_set_min_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MIN);
> > >
> > > + if (client->irq) {
> > > + err = seesaw_register_write_u32(client, SEESAW_GPIO_INTENSET, SEESAW_BUTTON_MASK);
> > > + if (err)
> > > + return dev_err_probe(&client->dev, err,
> > > + "failed to enable hardware interrupts\n");
> >
> > Maybe this should be in seesaw_open()?
> >
> > Thanks.
> >
> > --
> > Dmitry
>
> Hi Dmitry. Thank you for your review!
>
> Since this is a one-time setup, I believe it should remain as is
> within the seesaw_probe() function, similar to other pin
> configurations.
>
> Please let me know if I'm missing any point here.
>
> Best regards,
> Charles Dias
Hi Dmitry,
I just wanted to follow up on my previous response regarding the initialization.
Please let me know if there is anything else you'd like me to address
or if there are any further changes needed on my end. Thank you!
Best regards,
Charles Dias
^ permalink raw reply
* Re: [PATCH v2 0/2] Input/omap1: fix touchscreen functionality on Nokia 770
From: Linus Walleij @ 2026-04-19 20:10 UTC (permalink / raw)
To: Aaro Koskinen
Cc: Dmitry Torokhov, Oleksij Rempel, Janusz Krzysztofik,
Tony Lindgren, linux-input, linux-kernel, linux-omap
In-Reply-To: <20260419161848.825831-1-aaro.koskinen@iki.fi>
On Sun, Apr 19, 2026 at 6:21 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Another attempt to make 770 touchscreen work again.
>
> v2: Unrelated fbdev patch dropped (already merged)
> GPIO setup fixed by deleting the conflicting descriptor.
>
> v1: https://lore.kernel.org/linux-input/20250102181953.1020878-1-aaro.koskinen@iki.fi/
>
> Aaro Koskinen (2):
> Input: ads7846 - restore half-duplex support
> Input: ads7846 - fix up the pendown GPIO setup on Nokia 770
Patches look good to me!
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: ads7846 - fix up the pendown GPIO setup on Nokia 770
From: Dmitry Torokhov @ 2026-04-20 0:06 UTC (permalink / raw)
To: Aaro Koskinen
Cc: Oleksij Rempel, Janusz Krzysztofik, Tony Lindgren, Linus Walleij,
linux-input, linux-kernel, linux-omap
In-Reply-To: <20260419161848.825831-3-aaro.koskinen@iki.fi>
Hi Aaro,
On Sun, Apr 19, 2026 at 07:18:48PM +0300, Aaro Koskinen wrote:
> Commit 767d83361aaa6 ("Input: ads7846 - Convert to use software nodes")
> added gpiod set up for the IRQ in the 770 board file, then another in
> the touchscreen driver for reading the pen state. This will make the probe
> fail:
>
> [ 1.347381] ads7846 spi2.0: failed to request pendown GPIO
> [ 1.361846] ads7846: probe of spi2.0 failed with error -16
>
> I originally tried to fix it using non-exclusive flag, but that was not
> found acceptable. Instead, just use a single gpiod.
>
> Fixes: 767d83361aaa6 ("Input: ads7846 - Convert to use software nodes")
> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> ---
> arch/arm/mach-omap1/board-nokia770.c | 11 -----------
> drivers/input/touchscreen/ads7846.c | 12 +++++++-----
> 2 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
> index a5bf5554800f..8b8013ab4590 100644
> --- a/arch/arm/mach-omap1/board-nokia770.c
> +++ b/arch/arm/mach-omap1/board-nokia770.c
> @@ -285,9 +285,6 @@ static void __init nokia770_cbus_init(void)
> static struct gpiod_lookup_table nokia770_irq_gpio_table = {
> .dev_id = NULL,
> .table = {
> - /* GPIO used by SPI device 1 */
> - GPIO_LOOKUP("gpio-0-15", 15, "ads7846_irq",
> - GPIO_ACTIVE_HIGH),
> /* GPIO used for retu IRQ */
> GPIO_LOOKUP("gpio-48-63", 14, "retu_irq",
> GPIO_ACTIVE_HIGH),
> @@ -307,8 +304,6 @@ static struct gpiod_lookup_table nokia770_irq_gpio_table = {
>
> static void __init omap_nokia770_init(void)
> {
> - struct gpio_desc *d;
> -
> /* On Nokia 770, the SleepX signal is masked with an
> * MPUIO line by default. It has to be unmasked for it
> * to become functional */
> @@ -322,12 +317,6 @@ static void __init omap_nokia770_init(void)
> platform_add_devices(nokia770_devices, ARRAY_SIZE(nokia770_devices));
>
> gpiod_add_lookup_table(&nokia770_irq_gpio_table);
> - d = gpiod_get(NULL, "ads7846_irq", GPIOD_IN);
> - if (IS_ERR(d))
> - pr_err("Unable to get ADS7846 IRQ GPIO descriptor\n");
> - else
> - nokia770_spi_board_info[1].irq = gpiod_to_irq(d);
No, I think what we need here is a simple gpiod_put(). The mapping is
not going to change unless someone tries to unload gpiochip, but then
the device is not going to work anyway.
Longer term we need to figure out how to describe interrupts with
software nodes (or wait long enough for everything to get converted to
device tree).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: ads7846 - restore half-duplex support
From: Dmitry Torokhov @ 2026-04-20 0:13 UTC (permalink / raw)
To: Aaro Koskinen
Cc: Oleksij Rempel, Janusz Krzysztofik, Tony Lindgren, Linus Walleij,
linux-input, linux-kernel, linux-omap
In-Reply-To: <20260419161848.825831-2-aaro.koskinen@iki.fi>
Hi Aaro,
On Sun, Apr 19, 2026 at 07:18:47PM +0300, Aaro Koskinen wrote:
> +static void ads7846_halfd_read_state(struct ads7846 *ts)
> +{
> + struct ads7846_packet *packet = ts->packet;
> + int msg_idx = 0;
> +
> + packet->ignore = false;
> +
> + while (msg_idx < ts->msg_count) {
> + int error;
> +
> + ads7846_wait_for_hsync(ts);
> +
> + error = spi_sync(ts->spi, &ts->msg[msg_idx]);
> + if (error) {
> + dev_err_ratelimited(&ts->spi->dev, "spi_sync --> %d\n",
> + error);
> + packet->ignore = true;
> + return;
Sashiko recommends trying to power down ADC on errors, what do you
think?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: touchscreen: tsc2007: Reduce I2C transactions for Z2 read
From: Dmitry Torokhov @ 2026-04-20 0:18 UTC (permalink / raw)
To: Yuki Horii; +Cc: clamor95, johannes.kirchmair, linux-input, Andreas Kemnade
In-Reply-To: <20260410074100.1660-1-horiiyuk@ishida.co.jp>
Hi Yuki,
On Fri, Apr 10, 2026 at 04:41:00PM +0900, Yuki Horii wrote:
> From: Yuki Horii <yuuki198708@gmail.com>
>
> The current implementation sends a separate power-down command
> after reading the Z2 value, resulting in an extra I2C
> transaction per measurement cycle.
>
> The TSC2007 command byte contains a 2-bit power-down mode
> selection field. By selecting the power-down state in the Z2
> measurement command, the device powers down after the Z2 A/D
> conversion completes, eliminating the subsequent power-down
> transaction.
>
> This reduces the number of I2C transactions by one per touch
> measurement cycle, decreasing I2C bus overhead and improving
> touch sampling performance.
>
> Signed-off-by: Yuki Horii <yuuki198708@gmail.com>
> ---
> drivers/input/touchscreen/tsc2007_core.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/touchscreen/tsc2007_core.c b/drivers/input/touchscreen/tsc2007_core.c
> index 948935de894b..ff60245baa96 100644
> --- a/drivers/input/touchscreen/tsc2007_core.c
> +++ b/drivers/input/touchscreen/tsc2007_core.c
> @@ -61,10 +61,8 @@ static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
>
> /* turn y+ off, x- on; we'll use formula #1 */
> tc->z1 = tsc2007_xfer(tsc, READ_Z1);
> - tc->z2 = tsc2007_xfer(tsc, READ_Z2);
> -
> - /* Prepare for next touch reading - power down ADC, enable PENIRQ */
> - tsc2007_xfer(tsc, PWRDOWN);
> + /* Read Z2 and power down ADC after A/D conversion, enable PENIRQ */
> + tc->z2 = tsc2007_xfer(tsc, (TSC2007_POWER_OFF_IRQ_EN | TSC2007_MEASURE_Z2));
> }
>
> u32 tsc2007_calculate_resistance(struct tsc2007 *tsc, struct ts_event *tc)
Thank you for the patch.
I'd like people using this part to chime in (CCed).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: imx_keypad - Fix spelling mistake "Colums" -> "Columns"
From: Frank Li @ 2026-04-20 1:57 UTC (permalink / raw)
To: Ethan Carter Edwards
Cc: Dmitry Torokhov, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, linux-input, imx, linux-arm-kernel, linux-kernel,
kernel-janitors
In-Reply-To: <20260418-imx-typo-v1-1-2a15e54ad4e7@ethancedwards.com>
On Sat, Apr 18, 2026 at 08:58:32PM -0400, Ethan Carter Edwards wrote:
> There is a spelling mistake in a two comments. Fix them.
>
> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/input/keyboard/imx_keypad.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index 069c1d6376e1..ccde60cd6bb3 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -324,7 +324,7 @@ static void imx_keypad_config(struct imx_keypad *keypad)
> reg_val |= (keypad->cols_en_mask & 0xff) << 8; /* cols */
> writew(reg_val, keypad->mmio_base + KPCR);
>
> - /* Write 0's to KPDR[15:8] (Colums) */
> + /* Write 0's to KPDR[15:8] (Columns) */
> reg_val = readw(keypad->mmio_base + KPDR);
> reg_val &= 0x00ff;
> writew(reg_val, keypad->mmio_base + KPDR);
> @@ -357,7 +357,7 @@ static void imx_keypad_inhibit(struct imx_keypad *keypad)
> reg_val |= KBD_STAT_KPKR | KBD_STAT_KPKD;
> writew(reg_val, keypad->mmio_base + KPSR);
>
> - /* Colums as open drain and disable all rows */
> + /* Columns as open drain and disable all rows */
> reg_val = (keypad->cols_en_mask & 0xff) << 8;
> writew(reg_val, keypad->mmio_base + KPCR);
> }
>
> ---
> base-commit: c7275b05bc428c7373d97aa2da02d3a7fa6b9f66
> change-id: 20260418-imx-typo-14370bd2ce47
>
> Best regards,
> --
> Ethan Carter Edwards <ethan@ethancedwards.com>
>
^ permalink raw reply
* Re: [PATCH v5 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad
From: Dmitry Torokhov @ 2026-04-20 4:47 UTC (permalink / raw)
To: Hugo Villeneuve
Cc: robin, andy, geert, robh, krzk+dt, conor+dt, hvilleneuve,
mkorpershoek, matthias.bgg, angelogioacchino.delregno, lee,
alexander.sverdlin, marek.vasut, akurz, devicetree, linux-kernel,
linux-input, linux-arm-kernel, linux-mediatek
In-Reply-To: <20260312180304.3865850-5-hugo@hugovil.com>
Hi Hugo,
On Thu, Mar 12, 2026 at 02:00:58PM -0400, Hugo Villeneuve wrote:
> +
> +static void charlieplex_keypad_report_key(struct input_dev *input)
> +{
> + struct charlieplex_keypad *keypad = input_get_drvdata(input);
> + const unsigned short *keycodes = input->keycode;
> +
> + if (keypad->current_code > 0) {
> + input_event(input, EV_MSC, MSC_SCAN, keypad->current_code);
> + input_report_key(input, keycodes[keypad->current_code], 0);
This needs input_sync() as otherwise userspace is free to only recognize
the last MSC_SCAN event.
> + }
> +
> + if (keypad->debounce_code) {
> + input_event(input, EV_MSC, MSC_SCAN, keypad->debounce_code);
> + input_report_key(input, keycodes[keypad->debounce_code], 1);
> + }
> +
> + input_sync(input);
> + keypad->current_code = keypad->debounce_code;
> +}
> +
> +static void charlieplex_keypad_check_switch_change(struct input_dev *input,
> + int code)
> +{
> + struct charlieplex_keypad *keypad = input_get_drvdata(input);
> +
> + if (code != keypad->debounce_code) {
> + keypad->debounce_count = 0;
> + keypad->debounce_code = code;
> + } else if (keypad->debounce_count < keypad->debounce_threshold) {
This does not work if debouncing is disabled (debounce threshold is 0).
> + keypad->debounce_count++;
> +
> + if (keypad->debounce_count >= keypad->debounce_threshold &&
> + keypad->debounce_code != keypad->current_code)
> + charlieplex_keypad_report_key(input);
> + }
> +}
> +
> +static void charlieplex_keypad_poll(struct input_dev *input)
> +{
> + struct charlieplex_keypad *keypad = input_get_drvdata(input);
> + int code;
> +
> + code = 0;
> + for (unsigned int oline = 0; oline < keypad->nlines; oline++) {
> + DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
> + int err;
> +
> + /* Activate only one line as output at a time. */
> + gpiod_direction_output(keypad->line_gpios->desc[oline], 1);
> +
> + if (keypad->settling_time_us)
> + fsleep(keypad->settling_time_us);
> +
> + /* Read input on all other lines. */
> + err = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
> + keypad->line_gpios->desc,
> + keypad->line_gpios->info, values);
> + if (err)
> + return;
We need to deactivate the line on error too.
> +
> + for (unsigned int iline = 0; iline < keypad->nlines; iline++) {
> + if (iline == oline)
> + continue; /* Do not read active output line. */
> +
> + /* Check if GPIO is asserted. */
> + if (test_bit(iline, values)) {
> + code = MATRIX_SCAN_CODE(oline, iline,
> + get_count_order(keypad->nlines));
> + /*
> + * Exit loop immediately since we cannot detect
> + * more than one key press at a time.
> + */
> + break;
> + }
> + }
> +
> + gpiod_direction_input(keypad->line_gpios->desc[oline]);
> +
> + if (code)
> + break;
> + }
> +
> + charlieplex_keypad_check_switch_change(input, code);
> +}
> +
> +static int charlieplex_keypad_init_gpio(struct platform_device *pdev,
> + struct charlieplex_keypad *keypad)
> +{
> + char **pin_names;
> + char label[32];
> +
> + snprintf(label, sizeof(label), "%s-pin", pdev->name);
> +
> + keypad->line_gpios = devm_gpiod_get_array(&pdev->dev, "line", GPIOD_IN);
> + if (IS_ERR(keypad->line_gpios))
> + return PTR_ERR(keypad->line_gpios);
> +
> + keypad->nlines = keypad->line_gpios->ndescs;
> +
> + if (keypad->nlines > MATRIX_MAX_ROWS)
> + return -EINVAL;
> +
> + pin_names = devm_kasprintf_strarray(&pdev->dev, label, keypad->nlines);
> + if (IS_ERR(pin_names))
> + return PTR_ERR(pin_names);
> +
> + for (unsigned int i = 0; i < keypad->line_gpios->ndescs; i++)
> + gpiod_set_consumer_name(keypad->line_gpios->desc[i], pin_names[i]);
> +
> + return 0;
> +}
> +
> +static int charlieplex_keypad_probe(struct platform_device *pdev)
> +{
> + struct charlieplex_keypad *keypad;
> + unsigned int debounce_interval_ms;
> + unsigned int poll_interval_ms;
> + struct input_dev *input_dev;
> + int err;
> +
> + keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> + if (!keypad)
> + return -ENOMEM;
> +
> + input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!input_dev)
> + return -ENOMEM;
> +
> + keypad->input_dev = input_dev;
> +
> + device_property_read_u32(&pdev->dev, "poll-interval", &poll_interval_ms);
> + device_property_read_u32(&pdev->dev, "debounce-delay-ms", &debounce_interval_ms);
> + device_property_read_u32(&pdev->dev, "settling-time-us", &keypad->settling_time_us);
Not all of these are required properties. If they are missing the driver
will operate on garbage values.
> +
> + keypad->current_code = -1;
> + keypad->debounce_code = -1;
> + keypad->debounce_threshold = DIV_ROUND_UP(debounce_interval_ms, poll_interval_ms);
This will bomb if poll interval is 0.
> +
> + err = charlieplex_keypad_init_gpio(pdev, keypad);
> + if (err)
> + return err;
> +
> + input_dev->name = pdev->name;
> + input_dev->id.bustype = BUS_HOST;
> +
> + err = matrix_keypad_build_keymap(NULL, NULL, keypad->nlines,
> + keypad->nlines, NULL, input_dev);
> + if (err)
> + dev_err_probe(&pdev->dev, -ENOMEM, "failed to build keymap\n");
Missing "return".
> +
> + if (device_property_read_bool(&pdev->dev, "autorepeat"))
> + __set_bit(EV_REP, input_dev->evbit);
> +
> + input_set_capability(input_dev, EV_MSC, MSC_SCAN);
> +
> + err = input_setup_polling(input_dev, charlieplex_keypad_poll);
> + if (err)
> + dev_err_probe(&pdev->dev, err, "unable to set up polling\n");
Missing "return".
I fixed it up and applied, please take a look in my 'next' branch and
tell me if I messed up.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer
From: Sangyun Kim @ 2026-04-20 5:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: qasdev00, gargaditya08, linux-input, linux-kernel
This series addresses two defects in hid-appletb-kbd's inactivity
timer subsystem. The two patches target different bugs and are
logically independent; they are sent together because they touch the
same tear-down code and because the same maintainer will review both.
Patch 1 fixes a slab use-after-free with two related tear-down windows
introduced by commit 38224c472a03 ("HID: appletb-kbd: fix slab
use-after-free bug in appletb_kbd_probe"):
A) Within "if (kbd->backlight_dev)" the order was
put_device() then timer_delete_sync(). A concurrent
hid_appletb_bl unbind between those two calls can drop the last
devm reference and free the backlight_device; the still-armed
inactivity timer softirq then dereferences the freed object
through backlight_device_set_brightness() -> mutex_lock(&ops_lock).
B) The "if (kbd->backlight_dev)" block ran before
hid_hw_close()/hid_hw_stop(), so even after window A is closed a
late ".event" callback from the HID core (USB URB completion on
real hardware) can arrive between timer_delete_sync() and
put_device(), reach reset_inactivity_timer(), re-arm the timer
via mod_timer(), and reopen the same UAF.
Both windows produce the same KASAN slab-use-after-free on the object
allocated by devm_backlight_device_register(). Patch 1 closes them
together by moving hid_hw_close()/hid_hw_stop() before the backlight
cleanup and, inside that cleanup block, calling timer_delete_sync()
before put_device(). Shipping both as one commit avoids leaving
stable kernels in a half-fixed state where only window A is closed.
Patch 2 fixes a separate "sleeping function called from invalid
context" bug in the same subsystem. The inactivity timer is a
struct timer_list, so the callback runs in softirq context and calls
backlight_device_set_brightness() -> mutex_lock() from atomic
context; reset_inactivity_timer() has the same issue on the
brightness-restore path (it is called from appletb_kbd_hid_event()
and appletb_kbd_inp_event(), which run in softirq/IRQ context on
real USB hardware). Convert the inactivity timer to a delayed_work
and defer the brightness-restore call to a dedicated work_struct so
both sleeping calls run in process context.
Sangyun Kim (2):
HID: appletb-kbd: fix UAF in inactivity-timer cleanup path
HID: appletb-kbd: run inactivity autodim from workqueues
drivers/hid/hid-appletb-kbd.c | 56 ++++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 20 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH 1/2] HID: appletb-kbd: fix UAF in inactivity-timer cleanup path
From: Sangyun Kim @ 2026-04-20 5:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: qasdev00, gargaditya08, linux-input, linux-kernel, stable
In-Reply-To: <20260420051318.1411671-1-sangyun.kim@snu.ac.kr>
Commit 38224c472a03 ("HID: appletb-kbd: fix slab use-after-free bug in
appletb_kbd_probe") added timer_delete_sync(&kbd->inactivity_timer) to
both the probe close_hw error path and appletb_kbd_remove(), but the
way it was wired in left the inactivity timer reachable during driver
tear-down via two distinct windows.
Window A -- put_device() before timer_delete_sync():
put_device(&kbd->backlight_dev->dev);
timer_delete_sync(&kbd->inactivity_timer);
The inactivity_timer softirq reads kbd->backlight_dev and calls
backlight_device_set_brightness() -> mutex_lock(&ops_lock). If a
concurrent hid_appletb_bl unbind drops the last devm reference
between these two calls, the backlight_device is freed and the
mutex_lock() touches freed memory.
Window B -- backlight cleanup before hid_hw_stop():
if (kbd->backlight_dev) {
timer_delete_sync(...);
put_device(...);
}
hid_hw_close(hdev);
hid_hw_stop(hdev);
Even after Window A is closed, hid_hw_close()/hid_hw_stop() still run
afterwards, so a late ".event" callback from the HID core (USB URB
completion on real Apple hardware) can arrive after
timer_delete_sync() drained the softirq but before put_device() drops
the reference. That callback reaches reset_inactivity_timer(), which
calls mod_timer() and re-arms the timer. The freshly re-armed timer
can then fire on the about-to-be-freed backlight_device.
Both windows produce the same KASAN slab-use-after-free:
BUG: KASAN: slab-use-after-free in __mutex_lock+0x1aab/0x21c0
Read of size 8 at addr ffff88803ee9a108 by task swapper/0/0
Call Trace:
<IRQ>
__mutex_lock
backlight_device_set_brightness
appletb_inactivity_timer
call_timer_fn
run_timer_softirq
handle_softirqs
Allocated by task N:
devm_backlight_device_register
appletb_bl_probe
Freed by task M:
(concurrent hid_appletb_bl unbind path)
Close both windows at once by reworking the tear-down in
appletb_kbd_remove() and in the probe close_hw error path so that
1) hid_hw_close()/hid_hw_stop() run before the backlight cleanup,
guaranteeing no further .event callback can fire and re-arm the
timer, and
2) inside the "if (kbd->backlight_dev)" block, timer_delete_sync()
runs before put_device(), so the softirq is drained before the
final reference is dropped.
Fixes: 38224c472a03 ("HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe")
Cc: stable@vger.kernel.org
Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
---
drivers/hid/hid-appletb-kbd.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
index 0fdc0968b9ef..8feac9e3589b 100644
--- a/drivers/hid/hid-appletb-kbd.c
+++ b/drivers/hid/hid-appletb-kbd.c
@@ -440,13 +440,13 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
unregister_handler:
input_unregister_handler(&kbd->inp_handler);
close_hw:
- if (kbd->backlight_dev) {
- put_device(&kbd->backlight_dev->dev);
- timer_delete_sync(&kbd->inactivity_timer);
- }
hid_hw_close(hdev);
stop_hw:
hid_hw_stop(hdev);
+ if (kbd->backlight_dev) {
+ timer_delete_sync(&kbd->inactivity_timer);
+ put_device(&kbd->backlight_dev->dev);
+ }
return ret;
}
@@ -457,13 +457,13 @@ static void appletb_kbd_remove(struct hid_device *hdev)
appletb_kbd_set_mode(kbd, APPLETB_KBD_MODE_OFF);
input_unregister_handler(&kbd->inp_handler);
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+
if (kbd->backlight_dev) {
- put_device(&kbd->backlight_dev->dev);
timer_delete_sync(&kbd->inactivity_timer);
+ put_device(&kbd->backlight_dev->dev);
}
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static int appletb_kbd_suspend(struct hid_device *hdev, pm_message_t msg)
--
2.34.1
^ permalink raw reply related
* [PATCH 2/2] HID: appletb-kbd: run inactivity autodim from workqueues
From: Sangyun Kim @ 2026-04-20 5:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: qasdev00, gargaditya08, linux-input, linux-kernel, stable
In-Reply-To: <20260420051318.1411671-1-sangyun.kim@snu.ac.kr>
The autodim code in hid-appletb-kbd takes backlight_device->ops_lock
via backlight_device_set_brightness() -> mutex_lock() from two
different atomic contexts:
* appletb_inactivity_timer() is a struct timer_list callback, so it
runs in softirq context. Every expiry triggers
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591
Call Trace:
<IRQ>
__might_resched
__mutex_lock
backlight_device_set_brightness
appletb_inactivity_timer
call_timer_fn
run_timer_softirq
* reset_inactivity_timer() is called from appletb_kbd_hid_event() and
appletb_kbd_inp_event(). On real USB hardware these run in
softirq/IRQ context (URB completion and input-event dispatch).
When the Touch Bar has already been dimmed or turned off, the
reset path calls backlight_device_set_brightness() directly to
restore brightness, producing the same warning.
Both call sites hit the same mutex_lock()-from-atomic bug. Fix them
together by moving the blocking work onto the system workqueue:
* Convert the inactivity timer from struct timer_list to
struct delayed_work; the callback (appletb_inactivity_work) now
runs in process context where mutex_lock() is legal.
* Add a dedicated struct work_struct restore_brightness_work and have
reset_inactivity_timer() schedule it instead of calling
backlight_device_set_brightness() directly.
Cancel both works synchronously during driver tear-down alongside the
existing backlight reference drop.
The semantics are unchanged (same delays, same state transitions on
dim, turn-off and user activity); only the execution context of the
sleeping call changes. The timer field and callback are renamed to
match their new type; reset_inactivity_timer() keeps its name because
it is invoked from input event paths that read naturally as "reset
the inactivity timer".
Fixes: 93a0fc489481 ("HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar")
Cc: stable@vger.kernel.org
Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
---
drivers/hid/hid-appletb-kbd.c | 44 ++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
index 8feac9e3589b..462010a75899 100644
--- a/drivers/hid/hid-appletb-kbd.c
+++ b/drivers/hid/hid-appletb-kbd.c
@@ -17,7 +17,7 @@
#include <linux/module.h>
#include <linux/string.h>
#include <linux/backlight.h>
-#include <linux/timer.h>
+#include <linux/workqueue.h>
#include <linux/input/sparse-keymap.h>
#include "hid-ids.h"
@@ -62,7 +62,8 @@ struct appletb_kbd {
struct input_handle kbd_handle;
struct input_handle tpd_handle;
struct backlight_device *backlight_dev;
- struct timer_list inactivity_timer;
+ struct delayed_work inactivity_work;
+ struct work_struct restore_brightness_work;
bool has_dimmed;
bool has_turned_off;
u8 saved_mode;
@@ -164,16 +165,18 @@ static int appletb_tb_key_to_slot(unsigned int code)
}
}
-static void appletb_inactivity_timer(struct timer_list *t)
+static void appletb_inactivity_work(struct work_struct *work)
{
- struct appletb_kbd *kbd = timer_container_of(kbd, t, inactivity_timer);
+ struct appletb_kbd *kbd = container_of(to_delayed_work(work),
+ struct appletb_kbd,
+ inactivity_work);
if (kbd->backlight_dev && appletb_tb_autodim) {
if (!kbd->has_dimmed) {
backlight_device_set_brightness(kbd->backlight_dev, 1);
kbd->has_dimmed = true;
- mod_timer(&kbd->inactivity_timer,
- jiffies + secs_to_jiffies(appletb_tb_idle_timeout));
+ mod_delayed_work(system_wq, &kbd->inactivity_work,
+ secs_to_jiffies(appletb_tb_idle_timeout));
} else if (!kbd->has_turned_off) {
backlight_device_set_brightness(kbd->backlight_dev, 0);
kbd->has_turned_off = true;
@@ -181,16 +184,25 @@ static void appletb_inactivity_timer(struct timer_list *t)
}
}
+static void appletb_restore_brightness_work(struct work_struct *work)
+{
+ struct appletb_kbd *kbd = container_of(work, struct appletb_kbd,
+ restore_brightness_work);
+
+ if (kbd->backlight_dev)
+ backlight_device_set_brightness(kbd->backlight_dev, 2);
+}
+
static void reset_inactivity_timer(struct appletb_kbd *kbd)
{
if (kbd->backlight_dev && appletb_tb_autodim) {
if (kbd->has_dimmed || kbd->has_turned_off) {
- backlight_device_set_brightness(kbd->backlight_dev, 2);
kbd->has_dimmed = false;
kbd->has_turned_off = false;
+ schedule_work(&kbd->restore_brightness_work);
}
- mod_timer(&kbd->inactivity_timer,
- jiffies + secs_to_jiffies(appletb_tb_dim_timeout));
+ mod_delayed_work(system_wq, &kbd->inactivity_work,
+ secs_to_jiffies(appletb_tb_dim_timeout));
}
}
@@ -408,9 +420,11 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
dev_err_probe(dev, -ENODEV, "Failed to get backlight device\n");
} else {
backlight_device_set_brightness(kbd->backlight_dev, 2);
- timer_setup(&kbd->inactivity_timer, appletb_inactivity_timer, 0);
- mod_timer(&kbd->inactivity_timer,
- jiffies + secs_to_jiffies(appletb_tb_dim_timeout));
+ INIT_DELAYED_WORK(&kbd->inactivity_work, appletb_inactivity_work);
+ INIT_WORK(&kbd->restore_brightness_work,
+ appletb_restore_brightness_work);
+ mod_delayed_work(system_wq, &kbd->inactivity_work,
+ secs_to_jiffies(appletb_tb_dim_timeout));
}
kbd->inp_handler.event = appletb_kbd_inp_event;
@@ -444,7 +458,8 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
stop_hw:
hid_hw_stop(hdev);
if (kbd->backlight_dev) {
- timer_delete_sync(&kbd->inactivity_timer);
+ cancel_delayed_work_sync(&kbd->inactivity_work);
+ cancel_work_sync(&kbd->restore_brightness_work);
put_device(&kbd->backlight_dev->dev);
}
return ret;
@@ -461,7 +476,8 @@ static void appletb_kbd_remove(struct hid_device *hdev)
hid_hw_stop(hdev);
if (kbd->backlight_dev) {
- timer_delete_sync(&kbd->inactivity_timer);
+ cancel_delayed_work_sync(&kbd->inactivity_work);
+ cancel_work_sync(&kbd->restore_brightness_work);
put_device(&kbd->backlight_dev->dev);
}
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 1/7] iio: HID: Add helper method hid_sensor_adjust_channel_bit_mask()
From: Andy Shevchenko @ 2026-04-20 7:36 UTC (permalink / raw)
To: David Lechner
Cc: Natália Salvino André, andy, bentiss, jic23, jikos,
nuno.sa, srinivas.pandruvada, Pietro Di Consolo Gregorio,
linux-iio, linux-input
In-Reply-To: <1c49e011-ae23-4c6f-ad6f-f0b494c2dbbc@baylibre.com>
On Sat, Apr 18, 2026 at 11:32:13AM -0500, David Lechner wrote:
> On 4/17/26 5:58 PM, Natália Salvino André wrote:
> > Add helper method to deduplicate code in HID sensors.
...
> > + /* Real storage bits will change based on the report desc. */
> > + channels[channel].scan_type.realbits = size * 8;
>
> There is a BITS_PER_BYTE macro that would make sense here.
> > + /* Maximum size of a sample to capture is u32 */
> > + channels[channel].scan_type.storagebits = sizeof(u32) * 8;
>
> and here.
FWIW, we have BYTES_TO_BITS().
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 1/4] HID: pass the buffer size to hid_report_raw_event
From: Benjamin Tissoires @ 2026-04-20 8:01 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
Greg Kroah-Hartman, Lee Jones, linux-input, linux-kernel,
greybus-dev, linux-staging, linux-usb, stable
In-Reply-To: <938e8afadcbf2d7b9f0397e24926224985d9c385.camel@icenowy.me>
On Apr 20 2026, Icenowy Zheng wrote:
> 在 2026-04-16四的 16:48 +0200，Benjamin Tissoires写道：
> > commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > bogus memset()") enforced the provided data to be at least the size
> > of
> > the declared buffer in the report descriptor to prevent a buffer
> > overflow. However, we can try to be smarter by providing both the
> > buffer
> > size and the data size, meaning that hid_report_raw_event() can make
> > better decision whether we should plaining reject the buffer (buffer
> > overflow attempt) or if we can safely memset it to 0 and pass it to
> > the
> > rest of the stack.
> >
> > Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > bogus memset()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> > drivers/hid/bpf/hid_bpf_dispatch.c | 6 ++++--
> > drivers/hid/hid-core.c | 42 +++++++++++++++++++++++++---
> > ----------
> > drivers/hid/hid-gfrm.c | 4 ++--
> > drivers/hid/hid-logitech-hidpp.c | 2 +-
> > drivers/hid/hid-multitouch.c | 2 +-
> > drivers/hid/hid-primax.c | 2 +-
> > drivers/hid/hid-vivaldi-common.c | 2 +-
> > drivers/hid/wacom_sys.c | 6 +++---
> > drivers/staging/greybus/hid.c | 2 +-
> > include/linux/hid.h | 4 ++--
> > include/linux/hid_bpf.h | 14 ++++++++-----
> > 11 files changed, 53 insertions(+), 33 deletions(-)
>
> ============ 8< ===================
>
> > diff --git a/drivers/staging/greybus/hid.c
> > b/drivers/staging/greybus/hid.c
> > index 1f58c907c036..37e8605c6767 100644
> > --- a/drivers/staging/greybus/hid.c
> > +++ b/drivers/staging/greybus/hid.c
> > @@ -201,7 +201,7 @@ static void gb_hid_init_report(struct gb_hid
> > *ghid, struct hid_report *report)
> > * we just need to setup the input fields, so using
> > * hid_report_raw_event is safe.
> > */
> > - hid_report_raw_event(ghid->hid, report->type, ghid->inbuf,
> > size, 1);
> > + hid_report_raw_event(ghid->hid, report->type, ghid->inbuf,
> > ghib->bufsize, size, 1);
>
> Oops, "ghid" is misspelled here...
Damn, you're correct. Sorry.
Jiri, do you want me to send v3? Or can you fix it while applying?
>
> Found this when building some gaint kernel with this patchset.
Thanks a lot for spotting this.
Cheers,
Benjamin
>
> Thanks,
> Icenowy
>
> > }
> >
> > static void gb_hid_init_reports(struct gb_hid *ghid)
>
^ permalink raw reply
* Re: [PATCH v2] iio: magnetometer: hid-sensor-magn-3d: prefer 'unsigned int'
From: Andy Shevchenko @ 2026-04-20 8:42 UTC (permalink / raw)
To: Joshua Crofts
Cc: jikos, jic23, srinivas.pandruvada, dlechner, nuno.sa, andy,
linux-input, linux-iio, linux-kernel
In-Reply-To: <20260419180523.37396-1-joshua.crofts1@gmail.com>
On Sun, Apr 19, 2026 at 08:05:23PM +0200, Joshua Crofts wrote:
> Use 'u32' instead of bare 'unsigned' to resolve checkpatch.pl warnings
> and correct type use as defined in the hid_sensor_hub_callbacks struct.
"...in the struct hid_sensor_hub_callbacks."
> No functional change.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer
From: Aditya Garg @ 2026-04-20 12:47 UTC (permalink / raw)
To: Sangyun Kim, jikos, bentiss; +Cc: qasdev00, linux-input, linux-kernel
In-Reply-To: <20260420051318.1411671-1-sangyun.kim@snu.ac.kr>
On 4/20/26 10:43, Sangyun Kim wrote:
> This series addresses two defects in hid-appletb-kbd's inactivity
> timer subsystem. The two patches target different bugs and are
> logically independent; they are sent together because they touch the
> same tear-down code and because the same maintainer will review both.
>
> Patch 1 fixes a slab use-after-free with two related tear-down windows
> introduced by commit 38224c472a03 ("HID: appletb-kbd: fix slab
> use-after-free bug in appletb_kbd_probe"):
>
> A) Within "if (kbd->backlight_dev)" the order was
> put_device() then timer_delete_sync(). A concurrent
> hid_appletb_bl unbind between those two calls can drop the last
> devm reference and free the backlight_device; the still-armed
> inactivity timer softirq then dereferences the freed object
> through backlight_device_set_brightness() -> mutex_lock(&ops_lock).
>
> B) The "if (kbd->backlight_dev)" block ran before
> hid_hw_close()/hid_hw_stop(), so even after window A is closed a
> late ".event" callback from the HID core (USB URB completion on
> real hardware) can arrive between timer_delete_sync() and
> put_device(), reach reset_inactivity_timer(), re-arm the timer
> via mod_timer(), and reopen the same UAF.
>
> Both windows produce the same KASAN slab-use-after-free on the object
> allocated by devm_backlight_device_register(). Patch 1 closes them
> together by moving hid_hw_close()/hid_hw_stop() before the backlight
> cleanup and, inside that cleanup block, calling timer_delete_sync()
> before put_device(). Shipping both as one commit avoids leaving
> stable kernels in a half-fixed state where only window A is closed.
>
> Patch 2 fixes a separate "sleeping function called from invalid
> context" bug in the same subsystem. The inactivity timer is a
> struct timer_list, so the callback runs in softirq context and calls
> backlight_device_set_brightness() -> mutex_lock() from atomic
> context; reset_inactivity_timer() has the same issue on the
> brightness-restore path (it is called from appletb_kbd_hid_event()
> and appletb_kbd_inp_event(), which run in softirq/IRQ context on
> real USB hardware). Convert the inactivity timer to a delayed_work
> and defer the brightness-restore call to a dedicated work_struct so
> both sleeping calls run in process context.
>
> Sangyun Kim (2):
> HID: appletb-kbd: fix UAF in inactivity-timer cleanup path
> HID: appletb-kbd: run inactivity autodim from workqueues
>
> drivers/hid/hid-appletb-kbd.c | 56 ++++++++++++++++++++++-------------
> 1 file changed, 36 insertions(+), 20 deletions(-)
>
I had a very weird bug just once. And that was when I pressed fn key,
upon releasing, the touchbar mode did not restore to normal.
Although it was just once, and I was never able to reproduce it again.
Have you tested it on your Machine btw?
^ permalink raw reply
* [PATCH AUTOSEL 7.0-6.18] media: rc: fix race between unregister and urb/irq callbacks
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
To: patches, stable
Cc: Sean Young, Haotian Zhang, Patrice Chotard, Hans Verkuil,
Sasha Levin, andrzej.hajda, neil.armstrong, rfoss, airlied,
daniel, bonbons, jikos, benjamin.tissoires, hverkuil-cisco,
mchehab, maximlevitsky, wens, jernej.skrabec, samuel, david,
benpicco, crope, gregkh, dri-devel, linux-kernel, linux-input,
linux-media, linux-arm-kernel, linux-sunxi, linux-staging
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: Sean Young <sean@mess.org>
[ Upstream commit dccc0c3ddf8f16071736f98a7d6dd46a2d43e037 ]
Some rc device drivers have a race condition between rc_unregister_device()
and irq or urb callbacks. This is because rc_unregister_device() does two
things, it marks the device as unregistered so no new commands can be
issued and then it calls rc_free_device(). This means the driver has no
chance to cancel any pending urb callbacks or interrupts after the device
has been marked as unregistered. Those callbacks may access struct rc_dev
or its members (e.g. struct ir_raw_event_ctrl), which have been freed by
rc_free_device().
This change removes the implicit call to rc_free_device() from
rc_unregister_device(). This means that device drivers can call
rc_unregister_device() in their remove or disconnect function, then cancel
all the urbs and interrupts before explicitly calling rc_free_device().
Note this is an alternative fix for an issue found by Haotian Zhang, see
the Closes: tags.
Reported-by: Haotian Zhang <vulab@iscas.ac.cn>
Closes: https://lore.kernel.org/linux-media/20251114101432.2566-1-vulab@iscas.ac.cn/
Closes: https://lore.kernel.org/linux-media/20251114101418.2548-1-vulab@iscas.ac.cn/
Closes: https://lore.kernel.org/linux-media/20251114101346.2530-1-vulab@iscas.ac.cn/
Closes: https://lore.kernel.org/linux-media/20251114090605.2413-1-vulab@iscas.ac.cn/
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Good - drivers using `devm_rc_allocate_device` don't call
`rc_unregister_device` manually (they use `devm_rc_register_device`
which handles cleanup automatically). The patch removes the
`managed_alloc` flag because it changes the contract:
`rc_unregister_device` no longer frees the device regardless of how it
was allocated. Instead, `devm_rc_alloc_release` (the devres release
callback) calls `rc_free_device`, and the regular drivers must add
explicit `rc_free_device` calls.
Now let me do the full analysis write-up.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: PARSE THE SUBJECT LINE
Record: [media: rc] [fix] [race between unregister and urb/irq
callbacks]
### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
- **Reported-by:** Haotian Zhang <vulab@iscas.ac.cn> - the reporter
found multiple race conditions
- **Closes:** 4 separate lore.kernel.org links to bug reports (Nov 2025)
- **Reviewed-by:** Patrice Chotard <patrice.chotard@foss.st.com>
- **Signed-off-by:** Sean Young <sean@mess.org> (author, RC subsystem
maintainer)
- **Signed-off-by:** Hans Verkuil <hverkuil+cisco@kernel.org> (media
subsystem maintainer)
Record: Multiple bug reports (4 Closes: tags), reviewed by a maintainer,
written by the RC subsystem maintainer, merged by the media subsystem
maintainer. No Fixes: tag (expected).
### Step 1.3: ANALYZE THE COMMIT BODY TEXT
The commit explains a **use-after-free race condition**:
1. `rc_unregister_device()` marks the device as unregistered
2. Then it calls `rc_free_device()`, which frees `struct rc_dev` and
`struct ir_raw_event_ctrl`
3. Between marking unregistered and freeing, there's no window for
drivers to cancel pending URBs/IRQs
4. Those pending callbacks may still access the freed `struct rc_dev` or
its members
Record: Bug = use-after-free due to race between device unregister and
URB/IRQ callbacks. Symptom = accessing freed memory. Root cause =
`rc_unregister_device()` implicitly calls `rc_free_device()` before
drivers can cancel pending async operations.
### Step 1.4: DETECT HIDDEN BUG FIXES
This is an explicit race condition / use-after-free fix. Not hidden.
Record: This is a clear, well-documented bug fix.
## PHASE 2: DIFF ANALYSIS
### Step 2.1: INVENTORY THE CHANGES
41 files changed, ~58 insertions, ~36 deletions. Files span:
- Core: `rc-main.c`, `rc-ir-raw.c`, `rc-core.h`
- ~30+ driver files across media/rc, media/pci, media/usb, gpu/drm, hid,
staging
Record: Cross-subsystem change touching 41 files, but each driver change
is 1-3 lines. Core changes are ~10 lines net.
### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
**Core changes:**
1. `rc-main.c`: `rc_unregister_device()` no longer calls
`rc_free_device()` (removes the `if (!dev->managed_alloc)
rc_free_device(dev)` block)
2. `rc-main.c`: `rc_dev_release()` now calls `ir_raw_event_free(dev)`
before `kfree(dev)` - this ensures raw event data is cleaned up when
the device struct is finally freed
3. `rc-main.c`: `devm_rc_allocate_device()` no longer sets
`managed_alloc = true`
4. `rc-ir-raw.c`: `ir_raw_event_unregister()` no longer calls
`ir_raw_event_free()` (moved to `rc_dev_release`)
5. `rc-ir-raw.c`: `ir_raw_event_free()` removes the `if (!dev)` guard
(now only called from `rc_dev_release`)
6. `rc-core.h`: Removes `managed_alloc` field from `struct rc_dev`
**Driver changes:** Every driver using `rc_unregister_device()` now also
calls `rc_free_device()` afterward. Some USB drivers also reorder
operations to call `rc_unregister_device()` BEFORE killing URBs, then
`rc_free_device()` AFTER killing URBs.
### Step 2.3: IDENTIFY THE BUG MECHANISM
Category: **Race condition / Use-after-free**
Before the fix:
```
rc_unregister_device()
-> marks device unregistered
-> calls rc_free_device() -> frees struct rc_dev
// URB/IRQ callbacks can still fire and access freed rc_dev!
```
After the fix:
```
rc_unregister_device()
-> marks device unregistered
// Driver cancels URBs/IRQs here
rc_free_device()
-> safely frees struct rc_dev after all callbacks cancelled
```
Record: UAF race condition fix. Separating unregister from free gives
drivers a window to cancel async operations.
### Step 2.4: ASSESS THE FIX QUALITY
- Written by Sean Young (RC subsystem maintainer)
- Reviewed by Patrice Chotard
- Merged by Hans Verkuil (media maintainer)
- The approach is sound: it separates two concerns (marking unregistered
vs. freeing memory)
- Each per-driver change is trivial (add one `rc_free_device()` line)
- **Concern:** The scope is very large (41 files), though each change is
trivially mechanical
Record: High quality fix from the subsystem maintainer. Regression risk
is low per-driver but the sheer breadth is notable. The error path
changes in probe functions (removing `rdev = NULL` after
`rc_unregister_device`) are now correct because `rc_free_device(rdev)`
on the next goto target will properly free even after unregister.
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: BLAME THE CHANGED LINES
The `managed_alloc` logic was introduced in commit `ddbf7d5a698c4d`
(2016, v4.10 era) by Heiner Kallweit. The implicit `rc_free_device()`
call in `rc_unregister_device()` has been present since `d8b4b5822f51e2`
(2010, v2.6.36 era). The race condition has existed since the very
beginning of the rc_core subsystem.
Record: Bug exists in all kernel versions since 2010 (v2.6.36+). Present
in all stable trees.
### Step 3.2: FOLLOW THE FIXES: TAG
No Fixes: tag present (expected for autosel candidates).
### Step 3.3: CHECK FILE HISTORY
Multiple previous fixes for the same class of UAF bugs exist:
- `5abda7a16698d` "media: dm1105: Fix use after free bug in
dm1105_remove due to race condition"
- `29b0589a865b6` "media: rc: Fix use-after-free bugs caused by
ene_tx_irqsim()"
These were individual driver fixes for the same systemic issue. This
patch fixes it once and for all in the core.
Record: This is a systemic fix replacing multiple individual driver-
level workarounds.
### Step 3.4: CHECK THE AUTHOR
Sean Young is the RC subsystem maintainer. He has dozens of commits in
`drivers/media/rc/`. This is his subsystem.
Record: Author is the subsystem maintainer - highest trust level.
### Step 3.5: CHECK FOR DEPENDENCIES
This is a single, self-contained patch. All changes are coordinated. No
other patches are needed.
Record: Standalone fix, no dependencies.
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Found via web search:
- v1: "[PATCH] media: rc: rc_unregister_device() should not call
rc_free_device()" (Dec 20, 2025)
- v2: "[PATCH v2] media: rc: fix race between unregister and urb/irq
callbacks" (Jan 26, 2026)
The v2 has the `Reviewed-by` tag from Patrice Chotard that v1 didn't
have.
Record: Patch went through 2 revisions. Applied version is v2 (latest).
Reviewed by maintainer.
### Step 4.2: WHO REVIEWED
- Reviewed-by: Patrice Chotard (STMicroelectronics, works on st_rc
driver)
- Signed-off-by: Hans Verkuil (media subsystem co-maintainer)
- CC'd: Numerous subsystem maintainers (Maarten Lankhorst, David Airlie
for DRM, etc.)
Record: Properly reviewed through the maintainer chain.
### Step 4.3: SEARCH FOR THE BUG REPORT
4 separate bug reports from Haotian Zhang (November 2025) about UAF
races in different RC drivers.
Record: Multiple independent bug reports confirm this is a real-world
issue.
### Step 4.4: RELATED PATCHES
This is the alternative/comprehensive fix. The original individual
patches from Haotian Zhang addressed each driver separately; Sean
Young's approach fixes the root cause in the core.
Record: This is the definitive fix rather than per-driver workarounds.
### Step 4.5: STABLE MAILING LIST
No specific stable discussion found. The patch does not have Cc: stable
tag.
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: KEY FUNCTIONS
- `rc_unregister_device()` - modified to remove implicit free
- `rc_free_device()` - now must be called explicitly by drivers
- `rc_dev_release()` - now frees raw event data
- `ir_raw_event_free()` - moved from unregister path to release path
- `ir_raw_event_unregister()` - no longer frees raw events
### Step 5.2: CALLERS
Every RC driver's remove/disconnect/exit function calls
`rc_unregister_device()`. The patch adds `rc_free_device()` to all of
them. This affects USB IR receivers, PCI TV tuner cards, embedded IR
receivers, DRM bridges, etc.
Record: Broad impact - every RC device driver is affected.
### Step 5.3-5.5: CALL CHAIN AND PATTERNS
The race is triggered during device removal (USB disconnect, PCI remove,
platform remove). This is a common path triggered by:
- USB device physical disconnect
- Module unload
- System shutdown
- Device unbind via sysfs
Record: The buggy path is easily triggered by physical device
disconnect. Not theoretical.
## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
### Step 6.1: DOES THE BUGGY CODE EXIST IN STABLE?
Yes. The buggy code in `rc_unregister_device()` (calling
`rc_free_device()` implicitly) has existed since 2010. It is present in
ALL stable trees.
Record: Bug affects all stable trees (6.6.y, 6.1.y, 5.15.y, etc.).
### Step 6.2: BACKPORT COMPLICATIONS
This patch touches 41 files. Some files may have diverged between
mainline and stable trees. Backporting will likely require resolving
minor conflicts in some drivers. The core changes to `rc-main.c`, `rc-
ir-raw.c`, and `rc-core.h` are critical and should apply with minor
fuzz. The per-driver changes are mechanical and can be adapted.
Record: Expected difficulty: **medium**. Core changes likely apply
cleanly. Some driver-specific changes may need minor adjustment due to
driver-specific changes in stable trees vs mainline.
### Step 6.3: RELATED FIXES IN STABLE
Individual UAF fixes like `5abda7a16698d` (dm1105) and `29b0589a865b6`
(ene_ir) may already be in some stable trees, but they only fix specific
drivers. This systemic fix is comprehensive.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: SUBSYSTEM CRITICALITY
Media/RC subsystem - used by IR receivers on:
- TV tuner cards (USB and PCI)
- Media center remotes (iMON, MCE, Xbox)
- Embedded IR receivers (sunxi, meson, hix5hd2)
- DRM bridges with CEC/RC
Record: IMPORTANT level - affects users of IR remote control hardware
(common in HTPCs, embedded media devices).
### Step 7.2: SUBSYSTEM ACTIVITY
Active subsystem with regular maintenance from Sean Young.
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: WHO IS AFFECTED
All users with IR remote control hardware who disconnect or unbind the
device. This includes USB IR receivers (very common for HTPC setups),
PCI TV tuner cards with IR, and embedded systems with hardware IR.
Record: Moderate user population - anyone using Linux with IR remote
control hardware.
### Step 8.2: TRIGGER CONDITIONS
- Physical USB disconnect of IR receiver
- Module unload while IR device active
- Device unbind via sysfs
- System shutdown with pending IR operations
- Concurrent with incoming IR data
Record: Common trigger conditions. USB disconnect is the most frequent.
### Step 8.3: FAILURE MODE SEVERITY
Use-after-free: Accessing freed `struct rc_dev` or `struct
ir_raw_event_ctrl` from URB/IRQ callbacks. This leads to:
- Kernel crash/oops (most likely)
- Memory corruption (possible)
- Security vulnerability (potential, if attacker controls USB device)
Record: Severity: HIGH to CRITICAL. UAF can cause crashes and is
potentially exploitable.
### Step 8.4: RISK-BENEFIT RATIO
**Benefit:** HIGH - Fixes use-after-free race in all RC drivers
systemically. Prevents crashes during device disconnect. Fixes reported
real-world bug.
**Risk:** MEDIUM - 41 files touched, but each change is 1-3 lines and
mechanically identical (add `rc_free_device()` after
`rc_unregister_device()`). The core API change is well-designed. Some
USB drivers also reorder operations to call `rc_unregister_device()`
before `usb_kill_urb()`, which is the correct order.
Record: Benefit HIGH, Risk MEDIUM. The fix is well-designed but the
scope is large.
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: COMPILE THE EVIDENCE
**Evidence FOR backporting:**
- Fixes a real use-after-free race condition
- Multiple independent bug reports (4 Closes: tags)
- Written and signed by the RC subsystem maintainer (Sean Young)
- Reviewed by Patrice Chotard, merged by Hans Verkuil
- Bug has existed since 2010, affects all stable trees
- Previous individual driver fixes confirm this is a known issue pattern
- UAF can cause kernel crashes and is potentially exploitable
- The fix is logically correct - it separates unregister from free
**Evidence AGAINST backporting:**
- Very large scope: 41 files modified across multiple subsystems
- Changes a core API contract (rc_unregister_device no longer frees)
- Risk of backport conflicts due to per-driver divergence between
mainline and stable
- Some drivers may not exist in older stable trees, requiring manual
adaptation
- Removes `managed_alloc` infrastructure, affecting
`devm_rc_allocate_device()` users
### Step 9.2: STABLE RULES CHECKLIST
1. Obviously correct and tested? **YES** - reviewed by maintainers,
applied to mainline
2. Fixes a real bug? **YES** - UAF race condition with 4 bug reports
3. Important issue? **YES** - use-after-free / crash during device
disconnect
4. Small and contained? **NO** - 41 files is very large. However, each
change is mechanical (1-3 lines per file) and the core change is
small (~10 lines)
5. No new features or APIs? **YES** - only changes existing API behavior
6. Can apply to stable trees? **PARTIALLY** - core changes likely apply;
some driver changes may need adaptation
### Step 9.3: EXCEPTION CATEGORIES
Not applicable - this is a bug fix, not a device ID/quirk/DT/build fix.
### Step 9.4: DECISION
This is a genuine, important use-after-free fix from the subsystem
maintainer with multiple independent bug reports. However, the **scope
is extremely large** (41 files across multiple subsystems) and it
changes a core API contract. While each individual change is trivial and
mechanical, the sheer number of files makes this a high-risk backport.
If even one driver is missed or adapted incorrectly, it introduces a new
bug (memory leak from missing `rc_free_device()` or double-free from
calling it when not needed).
The stable kernel rules explicitly require patches to be "small and
contained." While the fix itself is correct and important, 41 files
touching DRM, HID, media PCI, media USB, and staging subsystems is not
small and contained. A backport of this magnitude would need careful
manual review for each stable tree to handle driver-specific divergence.
Verification:
- [Phase 1] Parsed tags: Reported-by: Haotian Zhang, 4 Closes: links,
Reviewed-by: Patrice Chotard, Signed-off-by: Sean Young (author/RC
maintainer), Hans Verkuil (media maintainer)
- [Phase 2] Diff analysis: 41 files changed, core change removes
implicit rc_free_device() from rc_unregister_device(), each driver
adds explicit rc_free_device() call
- [Phase 2] Verified: Current rc_unregister_device() at line 2046 has
`if (!dev->managed_alloc) rc_free_device(dev)` - confirmed the bug
exists
- [Phase 2] Verified: rc_dev_release() at line 1610 currently just does
`kfree(dev)` without freeing raw events - patch adds
ir_raw_event_free(dev) there
- [Phase 3] git blame: rc_unregister_device() implicit free from
d8b4b5822f51e2 (2010, v2.6.36), managed_alloc from ddbf7d5a698c4d
(2016, v4.10)
- [Phase 3] git show ddbf7d5a698c4d: confirmed managed_alloc was
introduced by Heiner Kallweit in 2016
- [Phase 3] Found related individual driver UAF fixes: 5abda7a16698d
(dm1105), 29b0589a865b6 (ene_ir)
- [Phase 3] git show 5abda7a16698d: confirmed dm1105 UAF race diagram
showing exactly the same pattern
- [Phase 4] Web search found v1 and v2 submissions: v1 Dec 2025, v2 Jan
26 2026
- [Phase 4] Found full v2 patch on freedesktop.org archives - matches
commit
- [Phase 4] 4 separate bug reports from Haotian Zhang (Nov 2025) confirm
real-world issue
- [Phase 5] Verified devm_rc_allocate_device callers (gpio-ir-recv,
meson-ir, etc.) don't manually call rc_unregister_device - they use
devm path, so they're unaffected
- [Phase 6] Bug exists since v2.6.36 (2010) - present in all stable
trees
- [Phase 8] Trigger: USB device disconnect during IR activity - common
operation
- UNVERIFIED: Whether all 41 files exist in all stable trees (some
drivers may have been added/removed between releases)
This is a borderline case. The fix addresses a real, reported use-after-
free bug from the subsystem maintainer with proper review. However, the
41-file scope makes it risky for stable. On balance, the UAF is serious
enough (crash, potential security) that it warrants backporting despite
the large scope, especially since each per-driver change is mechanically
identical and trivially verifiable.
**YES**
drivers/gpu/drm/bridge/sil-sii8620.c | 1 +
drivers/hid/hid-picolcd_cir.c | 1 +
drivers/media/cec/core/cec-core.c | 2 +-
drivers/media/common/siano/smsir.c | 1 +
drivers/media/i2c/ir-kbd-i2c.c | 2 ++
drivers/media/pci/bt8xx/bttv-input.c | 3 ++-
drivers/media/pci/cx23885/cx23885-input.c | 1 +
drivers/media/pci/cx88/cx88-input.c | 3 ++-
drivers/media/pci/dm1105/dm1105.c | 1 +
drivers/media/pci/mantis/mantis_input.c | 1 +
drivers/media/pci/saa7134/saa7134-input.c | 1 +
drivers/media/pci/smipcie/smipcie-ir.c | 1 +
drivers/media/pci/ttpci/budget-ci.c | 1 +
drivers/media/rc/ati_remote.c | 6 +++---
drivers/media/rc/ene_ir.c | 2 +-
drivers/media/rc/fintek-cir.c | 3 ++-
drivers/media/rc/igorplugusb.c | 1 +
drivers/media/rc/iguanair.c | 1 +
drivers/media/rc/img-ir/img-ir-hw.c | 3 ++-
drivers/media/rc/img-ir/img-ir-raw.c | 3 ++-
drivers/media/rc/imon.c | 3 ++-
drivers/media/rc/ir-hix5hd2.c | 2 +-
drivers/media/rc/ir_toy.c | 1 +
drivers/media/rc/ite-cir.c | 2 +-
drivers/media/rc/mceusb.c | 1 +
drivers/media/rc/rc-ir-raw.c | 5 -----
drivers/media/rc/rc-loopback.c | 1 +
drivers/media/rc/rc-main.c | 6 +-----
drivers/media/rc/redrat3.c | 4 +++-
drivers/media/rc/st_rc.c | 2 +-
drivers/media/rc/streamzap.c | 7 ++++---
drivers/media/rc/sunxi-cir.c | 1 +
drivers/media/rc/ttusbir.c | 2 +-
drivers/media/rc/winbond-cir.c | 2 +-
drivers/media/rc/xbox_remote.c | 5 +++--
drivers/media/usb/au0828/au0828-input.c | 1 +
drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 1 +
drivers/media/usb/dvb-usb/dvb-usb-remote.c | 6 ++++--
drivers/media/usb/em28xx/em28xx-input.c | 1 +
drivers/staging/media/av7110/av7110_ir.c | 1 +
include/media/rc-core.h | 2 --
41 files changed, 58 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index d3f238b1f2a94..982306eb4f0a7 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -2221,6 +2221,7 @@ static void sii8620_detach(struct drm_bridge *bridge)
return;
rc_unregister_device(ctx->rc_dev);
+ rc_free_device(ctx->rc_dev);
}
static int sii8620_is_packing_required(struct sii8620 *ctx,
diff --git a/drivers/hid/hid-picolcd_cir.c b/drivers/hid/hid-picolcd_cir.c
index d6faa0e00f95a..6d4c636e1c9f7 100644
--- a/drivers/hid/hid-picolcd_cir.c
+++ b/drivers/hid/hid-picolcd_cir.c
@@ -134,5 +134,6 @@ void picolcd_exit_cir(struct picolcd_data *data)
data->rc_dev = NULL;
rc_unregister_device(rdev);
+ rc_free_device(rdev);
}
diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 1953ce559ecaf..0fcd3b5e60c8d 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -338,8 +338,8 @@ int cec_register_adapter(struct cec_adapter *adap,
res = cec_devnode_register(&adap->devnode, adap->owner);
if (res) {
#ifdef CONFIG_MEDIA_CEC_RC
- /* Note: rc_unregister also calls rc_free */
rc_unregister_device(adap->rc);
+ rc_free_device(adap->rc);
adap->rc = NULL;
#endif
return res;
diff --git a/drivers/media/common/siano/smsir.c b/drivers/media/common/siano/smsir.c
index af07fed21ae12..283770d583d56 100644
--- a/drivers/media/common/siano/smsir.c
+++ b/drivers/media/common/siano/smsir.c
@@ -92,6 +92,7 @@ int sms_ir_init(struct smscore_device_t *coredev)
void sms_ir_exit(struct smscore_device_t *coredev)
{
rc_unregister_device(coredev->ir.dev);
+ rc_free_device(coredev->ir.dev);
pr_debug("\n");
}
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index 5588cdd7ec20d..6047453170043 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -355,6 +355,7 @@ static void ir_work(struct work_struct *work)
mutex_unlock(&ir->lock);
if (rc == -ENODEV) {
rc_unregister_device(ir->rc);
+ rc_free_device(ir->rc);
ir->rc = NULL;
return;
}
@@ -972,6 +973,7 @@ static void ir_remove(struct i2c_client *client)
i2c_unregister_device(ir->tx_c);
rc_unregister_device(ir->rc);
+ rc_free_device(ir->rc);
}
static const struct i2c_device_id ir_kbd_id[] = {
diff --git a/drivers/media/pci/bt8xx/bttv-input.c b/drivers/media/pci/bt8xx/bttv-input.c
index 373b6c6817d76..f704476506e07 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -572,8 +572,9 @@ void bttv_input_fini(struct bttv *btv)
if (btv->remote == NULL)
return;
- bttv_ir_stop(btv);
rc_unregister_device(btv->remote->dev);
+ bttv_ir_stop(btv);
+ rc_free_device(btv->remote->dev);
kfree(btv->remote);
btv->remote = NULL;
}
diff --git a/drivers/media/pci/cx23885/cx23885-input.c b/drivers/media/pci/cx23885/cx23885-input.c
index ffbbeca8a8e5f..554767b8ef2bf 100644
--- a/drivers/media/pci/cx23885/cx23885-input.c
+++ b/drivers/media/pci/cx23885/cx23885-input.c
@@ -402,6 +402,7 @@ void cx23885_input_fini(struct cx23885_dev *dev)
if (dev->kernel_ir == NULL)
return;
rc_unregister_device(dev->kernel_ir->rc);
+ rc_free_device(dev->kernel_ir->rc);
kfree(dev->kernel_ir->phys);
kfree(dev->kernel_ir->name);
kfree(dev->kernel_ir);
diff --git a/drivers/media/pci/cx88/cx88-input.c b/drivers/media/pci/cx88/cx88-input.c
index e958eecb29c5c..5d9ce4f9af011 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -509,8 +509,9 @@ int cx88_ir_fini(struct cx88_core *core)
if (!ir)
return 0;
- cx88_ir_stop(core);
rc_unregister_device(ir->dev);
+ cx88_ir_stop(core);
+ rc_free_device(ir->dev);
kfree(ir);
/* done */
diff --git a/drivers/media/pci/dm1105/dm1105.c b/drivers/media/pci/dm1105/dm1105.c
index de05d8b0f9dc5..bbd24769ae56b 100644
--- a/drivers/media/pci/dm1105/dm1105.c
+++ b/drivers/media/pci/dm1105/dm1105.c
@@ -763,6 +763,7 @@ static int dm1105_ir_init(struct dm1105_dev *dm1105)
static void dm1105_ir_exit(struct dm1105_dev *dm1105)
{
rc_unregister_device(dm1105->ir.dev);
+ rc_free_device(dm1105->ir.dev);
}
static int dm1105_hw_init(struct dm1105_dev *dev)
diff --git a/drivers/media/pci/mantis/mantis_input.c b/drivers/media/pci/mantis/mantis_input.c
index 34c0d979240fd..edb4cacf55d22 100644
--- a/drivers/media/pci/mantis/mantis_input.c
+++ b/drivers/media/pci/mantis/mantis_input.c
@@ -72,5 +72,6 @@ EXPORT_SYMBOL_GPL(mantis_input_init);
void mantis_input_exit(struct mantis_pci *mantis)
{
rc_unregister_device(mantis->rc);
+ rc_free_device(mantis->rc);
}
EXPORT_SYMBOL_GPL(mantis_input_exit);
diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c
index 5b71014157808..7f6680de31564 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -834,6 +834,7 @@ void saa7134_input_fini(struct saa7134_dev *dev)
return;
rc_unregister_device(dev->remote->dev);
+ rc_free_device(dev->remote->dev);
kfree(dev->remote);
dev->remote = NULL;
}
diff --git a/drivers/media/pci/smipcie/smipcie-ir.c b/drivers/media/pci/smipcie/smipcie-ir.c
index c0604d9c70119..0bbe4fa2d5a84 100644
--- a/drivers/media/pci/smipcie/smipcie-ir.c
+++ b/drivers/media/pci/smipcie/smipcie-ir.c
@@ -181,5 +181,6 @@ void smi_ir_exit(struct smi_dev *dev)
rc_unregister_device(rc_dev);
smi_ir_stop(ir);
+ rc_free_device(rc_dev);
ir->rc_dev = NULL;
}
diff --git a/drivers/media/pci/ttpci/budget-ci.c b/drivers/media/pci/ttpci/budget-ci.c
index 3709c0fb23b07..8b496b959d7ea 100644
--- a/drivers/media/pci/ttpci/budget-ci.c
+++ b/drivers/media/pci/ttpci/budget-ci.c
@@ -249,6 +249,7 @@ static void msp430_ir_deinit(struct budget_ci *budget_ci)
cancel_work_sync(&budget_ci->ir.msp430_irq_bh_work);
rc_unregister_device(budget_ci->ir.dev);
+ rc_free_device(budget_ci->ir.dev);
}
static int ciintf_read_attribute_mem(struct dvb_ca_en50221 *ca, int slot, int address)
diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 78abe810a88e7..51d85de24fae3 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -921,7 +921,6 @@ static int ati_remote_probe(struct usb_interface *interface,
input_free_device(input_dev);
exit_unregister_device:
rc_unregister_device(rc_dev);
- rc_dev = NULL;
exit_kill_urbs:
usb_kill_urb(ati_remote->irq_urb);
usb_kill_urb(ati_remote->out_urb);
@@ -941,18 +940,19 @@ static void ati_remote_disconnect(struct usb_interface *interface)
struct ati_remote *ati_remote;
ati_remote = usb_get_intfdata(interface);
- usb_set_intfdata(interface, NULL);
if (!ati_remote) {
dev_warn(&interface->dev, "%s - null device?\n", __func__);
return;
}
+ rc_unregister_device(ati_remote->rdev);
+ usb_set_intfdata(interface, NULL);
usb_kill_urb(ati_remote->irq_urb);
usb_kill_urb(ati_remote->out_urb);
if (ati_remote->idev)
input_unregister_device(ati_remote->idev);
- rc_unregister_device(ati_remote->rdev);
ati_remote_free_buffers(ati_remote);
+ rc_free_device(ati_remote->rdev);
kfree(ati_remote);
}
diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
index f8120605501ab..6f7dccc965e7f 100644
--- a/drivers/media/rc/ene_ir.c
+++ b/drivers/media/rc/ene_ir.c
@@ -1090,7 +1090,6 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
release_region(dev->hw_io, ENE_IO_SIZE);
exit_unregister_device:
rc_unregister_device(rdev);
- rdev = NULL;
exit_free_dev_rdev:
rc_free_device(rdev);
kfree(dev);
@@ -1110,6 +1109,7 @@ static void ene_remove(struct pnp_dev *pnp_dev)
ene_rx_restore_hw_buffer(dev);
spin_unlock_irqrestore(&dev->hw_lock, flags);
+ rc_free_device(dev->rdev);
free_irq(dev->irq, dev);
release_region(dev->hw_io, ENE_IO_SIZE);
kfree(dev);
diff --git a/drivers/media/rc/fintek-cir.c b/drivers/media/rc/fintek-cir.c
index f7cfa8a073ebc..5055dfc3f4651 100644
--- a/drivers/media/rc/fintek-cir.c
+++ b/drivers/media/rc/fintek-cir.c
@@ -568,6 +568,7 @@ static void fintek_remove(struct pnp_dev *pdev)
struct fintek_dev *fintek = pnp_get_drvdata(pdev);
unsigned long flags;
+ rc_unregister_device(fintek->rdev);
spin_lock_irqsave(&fintek->fintek_lock, flags);
/* disable CIR */
fintek_disable_cir(fintek);
@@ -580,7 +581,7 @@ static void fintek_remove(struct pnp_dev *pdev)
free_irq(fintek->cir_irq, fintek);
release_region(fintek->cir_addr, fintek->cir_port_len);
- rc_unregister_device(fintek->rdev);
+ rc_free_device(fintek->rdev);
kfree(fintek);
}
diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
index e034c93d57cf0..5ceb5ca44e235 100644
--- a/drivers/media/rc/igorplugusb.c
+++ b/drivers/media/rc/igorplugusb.c
@@ -242,6 +242,7 @@ static void igorplugusb_disconnect(struct usb_interface *intf)
usb_set_intfdata(intf, NULL);
usb_unpoison_urb(ir->urb);
usb_free_urb(ir->urb);
+ rc_free_device(ir->rc);
kfree(ir->buf_in);
}
diff --git a/drivers/media/rc/iguanair.c b/drivers/media/rc/iguanair.c
index c508f2536243e..0c5b8befb0af3 100644
--- a/drivers/media/rc/iguanair.c
+++ b/drivers/media/rc/iguanair.c
@@ -500,6 +500,7 @@ static void iguanair_disconnect(struct usb_interface *intf)
usb_set_intfdata(intf, NULL);
usb_kill_urb(ir->urb_in);
usb_kill_urb(ir->urb_out);
+ rc_free_device(ir->rc);
usb_free_urb(ir->urb_in);
usb_free_urb(ir->urb_out);
usb_free_coherent(ir->udev, MAX_IN_PACKET, ir->buf_in, ir->dma_in);
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index 63f6f5b36838d..f30adf4d8444d 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -1118,9 +1118,10 @@ void img_ir_remove_hw(struct img_ir_priv *priv)
struct rc_dev *rdev = hw->rdev;
if (!rdev)
return;
+ rc_unregister_device(rdev);
img_ir_set_decoder(priv, NULL, 0);
hw->rdev = NULL;
- rc_unregister_device(rdev);
+ rc_free_device(rdev);
#ifdef CONFIG_COMMON_CLK
if (!IS_ERR(priv->clk))
clk_notifier_unregister(priv->clk, &hw->clk_nb);
diff --git a/drivers/media/rc/img-ir/img-ir-raw.c b/drivers/media/rc/img-ir/img-ir-raw.c
index 92fb7b555a0f6..f1460d4acf3e8 100644
--- a/drivers/media/rc/img-ir/img-ir-raw.c
+++ b/drivers/media/rc/img-ir/img-ir-raw.c
@@ -136,6 +136,7 @@ void img_ir_remove_raw(struct img_ir_priv *priv)
if (!rdev)
return;
+ rc_unregister_device(rdev);
/* switch off and disable raw (edge) interrupts */
spin_lock_irq(&priv->lock);
raw->rdev = NULL;
@@ -145,7 +146,7 @@ void img_ir_remove_raw(struct img_ir_priv *priv)
img_ir_write(priv, IMG_IR_IRQ_CLEAR, IMG_IR_IRQ_EDGE);
spin_unlock_irq(&priv->lock);
- rc_unregister_device(rdev);
+ rc_free_device(rdev);
timer_delete_sync(&raw->timer);
}
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index 7e92161105d53..310c9fc9ae91c 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -2541,9 +2541,10 @@ static void imon_disconnect(struct usb_interface *interface)
if (ifnum == 0) {
ictx->dev_present_intf0 = false;
+ rc_unregister_device(ictx->rdev);
usb_kill_urb(ictx->rx_urb_intf0);
input_unregister_device(ictx->idev);
- rc_unregister_device(ictx->rdev);
+ rc_free_device(ictx->rdev);
if (ictx->display_supported) {
if (ictx->display_type == IMON_DISPLAY_TYPE_LCD)
usb_deregister_dev(interface, &imon_lcd_class);
diff --git a/drivers/media/rc/ir-hix5hd2.c b/drivers/media/rc/ir-hix5hd2.c
index edc46828509c8..1b061e4a3dcfa 100644
--- a/drivers/media/rc/ir-hix5hd2.c
+++ b/drivers/media/rc/ir-hix5hd2.c
@@ -331,7 +331,6 @@ static int hix5hd2_ir_probe(struct platform_device *pdev)
regerr:
rc_unregister_device(rdev);
- rdev = NULL;
clkerr:
clk_disable_unprepare(priv->clock);
err:
@@ -346,6 +345,7 @@ static void hix5hd2_ir_remove(struct platform_device *pdev)
clk_disable_unprepare(priv->clock);
rc_unregister_device(priv->rdev);
+ rc_free_device(priv->rdev);
}
#ifdef CONFIG_PM_SLEEP
diff --git a/drivers/media/rc/ir_toy.c b/drivers/media/rc/ir_toy.c
index d6472de5da87d..089833e411786 100644
--- a/drivers/media/rc/ir_toy.c
+++ b/drivers/media/rc/ir_toy.c
@@ -536,6 +536,7 @@ static void irtoy_disconnect(struct usb_interface *intf)
usb_free_urb(ir->urb_out);
usb_kill_urb(ir->urb_in);
usb_free_urb(ir->urb_in);
+ rc_free_device(ir->rc);
kfree(ir->in);
kfree(ir->out);
kfree(ir);
diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index bf544517c67a9..bde2a70512310 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -1414,7 +1414,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
release_region(itdev->cir_addr, itdev->params->io_region_size);
exit_unregister_device:
rc_unregister_device(rdev);
- rdev = NULL;
exit_free_dev_rdev:
rc_free_device(rdev);
kfree(itdev);
@@ -1439,6 +1438,7 @@ static void ite_remove(struct pnp_dev *pdev)
release_region(dev->cir_addr, dev->params->io_region_size);
rc_unregister_device(dev->rdev);
+ rc_free_device(dev->rdev);
kfree(dev);
}
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index ed55e9ec3c570..06222eee17540 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -1850,6 +1850,7 @@ static void mceusb_dev_disconnect(struct usb_interface *intf)
usb_free_urb(ir->urb_in);
usb_free_coherent(dev, ir->len_in, ir->buf_in, ir->dma_in);
usb_put_dev(dev);
+ rc_free_device(ir->rc);
kfree(ir);
}
diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index 2e269ef5e26be..ba24c2f22d39f 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -648,9 +648,6 @@ int ir_raw_event_register(struct rc_dev *dev)
void ir_raw_event_free(struct rc_dev *dev)
{
- if (!dev)
- return;
-
kfree(dev->raw);
dev->raw = NULL;
}
@@ -674,8 +671,6 @@ void ir_raw_event_unregister(struct rc_dev *dev)
lirc_bpf_free(dev);
- ir_raw_event_free(dev);
-
/*
* A user can be calling bpf(BPF_PROG_{QUERY|ATTACH|DETACH}), so
* ensure that the raw member is null on unlock; this is how
diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
index 78ac09b3cbd34..53d0540717b36 100644
--- a/drivers/media/rc/rc-loopback.c
+++ b/drivers/media/rc/rc-loopback.c
@@ -263,6 +263,7 @@ static int __init loop_init(void)
static void __exit loop_exit(void)
{
rc_unregister_device(loopdev.dev);
+ rc_free_device(loopdev.dev);
}
module_init(loop_init);
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 821607504008a..dda3479ea3add 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1611,6 +1611,7 @@ static void rc_dev_release(struct device *device)
{
struct rc_dev *dev = to_rc_dev(device);
+ ir_raw_event_free(dev);
kfree(dev);
}
@@ -1773,7 +1774,6 @@ struct rc_dev *devm_rc_allocate_device(struct device *dev,
}
rc->dev.parent = dev;
- rc->managed_alloc = true;
*dr = rc;
devres_add(dev, dr);
@@ -2042,11 +2042,7 @@ void rc_unregister_device(struct rc_dev *dev)
device_del(&dev->dev);
ida_free(&rc_ida, dev->minor);
-
- if (!dev->managed_alloc)
- rc_free_device(dev);
}
-
EXPORT_SYMBOL_GPL(rc_unregister_device);
/*
diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index 3b917a2a89188..3f828a564e192 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -1131,11 +1131,13 @@ static void redrat3_dev_disconnect(struct usb_interface *intf)
{
struct usb_device *udev = interface_to_usbdev(intf);
struct redrat3_dev *rr3 = usb_get_intfdata(intf);
+ struct rc_dev *rc = rr3->rc;
usb_set_intfdata(intf, NULL);
- rc_unregister_device(rr3->rc);
+ rc_unregister_device(rc);
led_classdev_unregister(&rr3->led);
redrat3_delete(rr3, udev);
+ rc_free_device(rc);
}
static int redrat3_dev_suspend(struct usb_interface *intf, pm_message_t message)
diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
index 6b70bac5f45d6..0ba06bfc9e14b 100644
--- a/drivers/media/rc/st_rc.c
+++ b/drivers/media/rc/st_rc.c
@@ -203,6 +203,7 @@ static void st_rc_remove(struct platform_device *pdev)
device_init_wakeup(&pdev->dev, false);
clk_disable_unprepare(rc_dev->sys_clock);
rc_unregister_device(rc_dev->rdev);
+ rc_free_device(rc_dev->rdev);
}
static int st_rc_open(struct rc_dev *rdev)
@@ -334,7 +335,6 @@ static int st_rc_probe(struct platform_device *pdev)
return ret;
rcerr:
rc_unregister_device(rdev);
- rdev = NULL;
clkerr:
clk_disable_unprepare(rc_dev->sys_clock);
err:
diff --git a/drivers/media/rc/streamzap.c b/drivers/media/rc/streamzap.c
index 5a18603f9a95c..7103da57c19f1 100644
--- a/drivers/media/rc/streamzap.c
+++ b/drivers/media/rc/streamzap.c
@@ -388,15 +388,16 @@ static void streamzap_disconnect(struct usb_interface *interface)
struct streamzap_ir *sz = usb_get_intfdata(interface);
struct usb_device *usbdev = interface_to_usbdev(interface);
- usb_set_intfdata(interface, NULL);
-
if (!sz)
return;
- usb_kill_urb(sz->urb_in);
rc_unregister_device(sz->rdev);
+ usb_set_intfdata(interface, NULL);
+
+ usb_kill_urb(sz->urb_in);
usb_free_urb(sz->urb_in);
usb_free_coherent(usbdev, sz->buf_in_len, sz->buf_in, sz->dma_in);
+ rc_free_device(sz->rdev);
kfree(sz);
}
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 92ef4e7c6f69f..cb4c56bf0752a 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -371,6 +371,7 @@ static void sunxi_ir_remove(struct platform_device *pdev)
struct sunxi_ir *ir = platform_get_drvdata(pdev);
rc_unregister_device(ir->rc);
+ rc_free_device(ir->rc);
sunxi_ir_hw_exit(&pdev->dev);
}
diff --git a/drivers/media/rc/ttusbir.c b/drivers/media/rc/ttusbir.c
index 110a469001146..a2a64a860264b 100644
--- a/drivers/media/rc/ttusbir.c
+++ b/drivers/media/rc/ttusbir.c
@@ -333,7 +333,6 @@ static int ttusbir_probe(struct usb_interface *intf,
return 0;
out3:
rc_unregister_device(rc);
- rc = NULL;
out2:
led_classdev_unregister(&tt->led);
out:
@@ -373,6 +372,7 @@ static void ttusbir_disconnect(struct usb_interface *intf)
}
usb_kill_urb(tt->bulk_urb);
usb_free_urb(tt->bulk_urb);
+ rc_free_device(tt->rc);
usb_set_intfdata(intf, NULL);
kfree(tt);
}
diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
index 515469dd82d4c..8e804661a6215 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -1132,7 +1132,6 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
release_region(data->wbase, WAKEUP_IOMEM_LEN);
exit_unregister_device:
rc_unregister_device(data->dev);
- data->dev = NULL;
exit_free_rc:
rc_free_device(data->dev);
exit_unregister_led:
@@ -1163,6 +1162,7 @@ wbcir_remove(struct pnp_dev *device)
wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
rc_unregister_device(data->dev);
+ rc_free_device(data->dev);
led_classdev_unregister(&data->led);
diff --git a/drivers/media/rc/xbox_remote.c b/drivers/media/rc/xbox_remote.c
index 3e3da70cf8da0..c64123e9d16a5 100644
--- a/drivers/media/rc/xbox_remote.c
+++ b/drivers/media/rc/xbox_remote.c
@@ -277,14 +277,15 @@ static void xbox_remote_disconnect(struct usb_interface *interface)
struct xbox_remote *xbox_remote;
xbox_remote = usb_get_intfdata(interface);
- usb_set_intfdata(interface, NULL);
if (!xbox_remote) {
dev_warn(&interface->dev, "%s - null device?\n", __func__);
return;
}
- usb_kill_urb(xbox_remote->irq_urb);
rc_unregister_device(xbox_remote->rdev);
+ usb_set_intfdata(interface, NULL);
+ usb_kill_urb(xbox_remote->irq_urb);
+ rc_free_device(xbox_remote->rdev);
usb_free_urb(xbox_remote->irq_urb);
kfree(xbox_remote);
}
diff --git a/drivers/media/usb/au0828/au0828-input.c b/drivers/media/usb/au0828/au0828-input.c
index 7dec1a360da6a..81d0165a3064d 100644
--- a/drivers/media/usb/au0828/au0828-input.c
+++ b/drivers/media/usb/au0828/au0828-input.c
@@ -357,6 +357,7 @@ void au0828_rc_unregister(struct au0828_dev *dev)
return;
rc_unregister_device(ir->rc);
+ rc_free_device(ir->rc);
/* done */
kfree(ir);
diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index 600cff8a4abdc..bd86d250433df 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -187,6 +187,7 @@ static int dvb_usbv2_remote_exit(struct dvb_usb_device *d)
if (d->rc_dev) {
cancel_delayed_work_sync(&d->rc_query_work);
rc_unregister_device(d->rc_dev);
+ rc_free_device(d->rc_dev);
d->rc_dev = NULL;
}
diff --git a/drivers/media/usb/dvb-usb/dvb-usb-remote.c b/drivers/media/usb/dvb-usb/dvb-usb-remote.c
index 65e2c9e2cdc99..6dc11718dfb98 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-remote.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-remote.c
@@ -347,10 +347,12 @@ int dvb_usb_remote_exit(struct dvb_usb_device *d)
{
if (d->state & DVB_USB_STATE_REMOTE) {
cancel_delayed_work_sync(&d->rc_query_work);
- if (d->props.rc.mode == DVB_RC_LEGACY)
+ if (d->props.rc.mode == DVB_RC_LEGACY) {
input_unregister_device(d->input_dev);
- else
+ } else {
rc_unregister_device(d->rc_dev);
+ rc_free_device(d->rc_dev);
+ }
}
d->state &= ~DVB_USB_STATE_REMOTE;
return 0;
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 20fdd59b55185..ab61d9a29b10e 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -853,6 +853,7 @@ static int em28xx_ir_fini(struct em28xx *dev)
goto ref_put;
rc_unregister_device(ir->rc);
+ rc_free_device(ir->rc);
kfree(ir->i2c_client);
diff --git a/drivers/staging/media/av7110/av7110_ir.c b/drivers/staging/media/av7110/av7110_ir.c
index 68b3979ba5f20..fdae467fd7ab8 100644
--- a/drivers/staging/media/av7110/av7110_ir.c
+++ b/drivers/staging/media/av7110/av7110_ir.c
@@ -151,6 +151,7 @@ int av7110_ir_init(struct av7110 *av7110)
void av7110_ir_exit(struct av7110 *av7110)
{
rc_unregister_device(av7110->ir.rcdev);
+ rc_free_device(av7110->ir.rcdev);
}
//MODULE_AUTHOR("Holger Waechtler <holger@convergence.de>, Oliver Endriss <o.endriss@gmx.de>");
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 35c7a0546f02e..7c964b5ad7926 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -81,7 +81,6 @@ struct lirc_fh {
/**
* struct rc_dev - represents a remote control device
* @dev: driver model's view of this device
- * @managed_alloc: devm_rc_allocate_device was used to create rc_dev
* @registered: set to true by rc_register_device(), false by
* rc_unregister_device
* @idle: used to keep track of RX state
@@ -156,7 +155,6 @@ struct lirc_fh {
*/
struct rc_dev {
struct device dev;
- bool managed_alloc;
bool registered;
bool idle;
bool encode_wakeup;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.6] HID: logitech-hidpp: fix race condition when accessing stale stack pointer
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
To: patches, stable
Cc: Benoît Sevens, Jiri Kosina, Sasha Levin, jikos, bentiss,
linux-input, linux-kernel
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: Benoît Sevens <bsevens@google.com>
[ Upstream commit e2aaf2d3ad92ac4a8afa6b69ad4c38e7747d3d6e ]
The driver uses hidpp->send_receive_buf to point to a stack-allocated
buffer in the synchronous command path (__do_hidpp_send_message_sync).
However, this pointer is not cleared when the function returns.
If an event is processed (e.g. by a different thread) while the
send_mutex is held by a new command, but before that command has
updated send_receive_buf, the handler (hidpp_raw_hidpp_event) will
observe that the mutex is locked and dereference the stale pointer.
This results in an out-of-bounds access on a different thread's kernel
stack (or a NULL pointer dereference on the very first command).
Fix this by:
1. Clearing hidpp->send_receive_buf to NULL before releasing the mutex
in the synchronous command path.
2. Moving the assignment of the local 'question' and 'answer' pointers
inside the mutex_is_locked() block in the handler, and adding
a NULL check before dereferencing.
Signed-off-by: Benoît Sevens <bsevens@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have enough information to make a thorough analysis. Let me
compile my findings.
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Parse Subject Line**
- Record: Subsystem `HID: logitech-hidpp`, action verb `fix`, summary:
race condition when accessing stale stack pointer (clear bug fix).
**Step 1.2: Parse Tags**
- Record: `Signed-off-by: Benoît Sevens <bsevens@google.com>` (author)
and `Signed-off-by: Jiri Kosina <jkosina@suse.com>` (HID subsystem
maintainer). No Reported-by/Link/Fixes tags. No stable tag. Author is
a known Google security researcher (has other HID UAF/OOB fixes in
roccat and wacom).
**Step 1.3: Analyze Body**
- Record: Bug description: `hidpp->send_receive_buf` is assigned to
point at a stack-allocated response buffer inside
`__do_hidpp_send_message_sync()` but never cleared when the function
returns. Meanwhile `hidpp_raw_hidpp_event()` speculatively reads this
pointer whenever `send_mutex` is locked. The race:
1. Thread A finishes a command; leaves `send_receive_buf` pointing at
A's stack.
2. Thread B grabs `send_mutex`; there is a window before B writes
`send_receive_buf = response`.
3. Event handler fires (different thread), sees the mutex locked,
dereferences the stale pointer from A's stack. The handler even
performs `*answer = *report` — a WRITE to the stale stack pointer
using device-supplied data (potential stack corruption / info leak
/ exploit primitive).
4. On very first use, `send_receive_buf` is NULL → NULL deref.
Symptom: OOB stack access on a different thread or NULL deref on
first command.
**Step 1.4: Hidden Bug Detection**
- Record: Not hidden — commit title already says "fix race condition".
Classic race + UAF/stale-pointer bug.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- Record: One file changed (`drivers/hid/hid-logitech-hidpp.c`), +17/-7
lines. Two functions touched: `__do_hidpp_send_message_sync()` and
`hidpp_raw_hidpp_event()`. Classification: single-file surgical fix.
**Step 2.2: Code Flow Change**
- Record: Producer (`__do_hidpp_send_message_sync`): early returns
converted to `goto out`, new `out:` label clears
`hidpp->send_receive_buf = NULL` before returning. Consumer
(`hidpp_raw_hidpp_event`): `question/answer` assignments moved inside
the `mutex_is_locked()` block, plus `if (!question) return 0;` NULL
guard before use.
**Step 2.3: Bug Mechanism**
- Record: Category (b)+(d) — race / memory safety fix. Adds implicit
synchronization by ensuring the shared pointer is NULL'd while still
holding `send_mutex`, and adds a NULL check on the read side to close
the small window between mutex acquisition and pointer assignment.
Addresses two failure modes: stale stack pointer dereference (UAF of
stack memory) and NULL dereference on first use.
**Step 2.4: Fix Quality**
- Record: Obviously correct. Minimal. No new locks, no API/ABI changes.
Possible (extremely minor) regression risk: if a report raced in
between `mutex_lock` and the assignment, the early-out `return 0` will
now skip matching it against the question — but this was already
broken (it used a stale pointer) and the send path has a 5-second
timeout with retry, so the benign behavior is strictly safer. No
regression risk beyond that.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: Blame**
- Record: The stale-pointer pattern (`hidpp->send_receive_buf =
response;` and the speculative read `struct hidpp_report *question =
hidpp->send_receive_buf;`) dates to commit `2f31c52529103d` "HID:
Introduce hidpp" by Benjamin Tissoires, Sep 2014 (≈ v3.18). Bug is
therefore present in every stable tree from v3.18 through
v6.19/mainline.
**Step 3.2: Fixes: tag**
- Record: No `Fixes:` tag in the commit. Manually identified introducing
commit as `2f31c52529103d` (original driver introduction, 2014).
**Step 3.3: Related Changes to the File**
- Record: Recent file history shows actively maintained file (device ID
adds, quirks, other UAF fix `f7a4c78b` "Prevent use-after-free on
force feedback initialisation failure"). Function was split into
`__do_hidpp_send_message_sync`/`hidpp_send_message_sync` in
`60165ab774cb0c` (v6.7, Jul 2023). Before that split (v6.6 is the
earliest with `__do_hidpp_send_message_sync`), the logic lived inline
in `hidpp_send_message_sync`.
**Step 3.4: Author's Other Commits**
- Record: Benoît Sevens (Google) has prior HID security fixes:
`d802d848` (roccat UAF), `2f1763f6` (wacom OOB), plus similar fixes in
uvcvideo and ALSA. Consistent pattern of Google-originated kernel
security research. High trust.
**Step 3.5: Dependencies**
- Record: Self-contained, no prerequisites. Fix only touches one file
and two functions. For v6.1.y and older the function was not yet
split, so the fix requires trivial rewording to apply (move the NULL-
out before `mutex_unlock` in `hidpp_send_message_sync`), but the
change is mechanical.
## PHASE 4: MAILING LIST RESEARCH
**Step 4.1: Original Discussion**
- Record: `b4 dig -c e2aaf2d3ad92a` found the thread at https://lore.ker
nel.org/all/20260401144811.1242722-1-bsevens@google.com/. Only one
version (v1) was posted; no review-imposed revisions.
**Step 4.2: Reviewers**
- Record: `b4 dig -w`: patch CC'd `Filipe Laíns`, `Bastien Nocera`,
`Jiri Kosina`, `Benjamin Tissoires`, linux-input, linux-kernel — all
the correct maintainers. Merged by Jiri Kosina (subsystem maintainer)
with note: "Now applied. Benjamin had some ideas on further cleanup
(allocating with __free__ instead of using stack pointer), but that'd
be a little bigger cleanup, so let's keep that separate." Confirms
maintainer reviewed and accepted; any follow-up is an orthogonal
cleanup, not a fix correction.
**Step 4.3: Bug Reports**
- Record: No Reported-by or Link tags. Author is from Google — likely
discovered via internal audit/fuzzing. No public reproducer cited.
**Step 4.4: Related Series**
- Record: Single standalone patch, no series.
**Step 4.5: Stable ML**
- Record: Not searched; the patch was only posted April 1, 2026 and
applied soon after — too fresh for independent stable ML activity.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: Key Functions**
- Record: `__do_hidpp_send_message_sync` (producer) and
`hidpp_raw_hidpp_event` (consumer).
**Step 5.2: Callers**
- Record: 45 call sites for `hidpp_send_*_sync` inside the driver —
every HID++ query (battery, connect, feature discovery, probe,
get_report_length, etc.). `hidpp_raw_hidpp_event` is invoked from
`hidpp_raw_event` (registered as `.raw_event` in `hid_ll_driver`),
reached from the HID core for every HID report coming from any
Logitech HID++ device (receivers, mice, keyboards, touchpads). Both
paths fire during normal operation — not rare.
**Step 5.3: Callees**
- Record: Sync path calls `__hidpp_send_report()` (USB/Bluetooth
transmit) and `wait_event_timeout()`. Event path does a struct-copy
`*answer = *report` — this is the dangerous write when `answer` is
stale.
**Step 5.4: Reachability**
- Record: The sync path runs in process context (probe, sysfs,
workqueue). The event path runs from HID input processing (URB
completion / BT callback, softirq or kthread depending on transport).
Different contexts on different CPUs → true concurrent race possible.
Triggers do not require privilege — any HID++ device that sends
unsolicited reports while a command is in flight can race. This is the
normal mode of operation for hidpp devices (connect events, battery
notifications, keypresses).
**Step 5.5: Similar Patterns**
- Record: Only one occurrence of `send_receive_buf` in the file; pattern
is unique to this driver.
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1: Buggy Code in Stable**
- Record: Verified via `git show <tag>:drivers/hid/hid-logitech-hidpp.c`
that the exact same vulnerable pattern (`send_receive_buf = response`
without clearing, `question = hidpp->send_receive_buf` read without
NULL check) exists in v4.19, v5.4, v5.10, v5.15, v6.1, v6.6. All
active stable trees are affected.
**Step 6.2: Backport Complications**
- Record: v6.6+ (and mainline): patch applies cleanly — identical
function structure. v6.1 and earlier: `__do_hidpp_send_message_sync`
does not yet exist; the logic is inline in `hidpp_send_message_sync`
which also holds/releases `send_mutex`. Backport requires mechanically
placing `hidpp->send_receive_buf = NULL;` before
`mutex_unlock(&hidpp->send_mutex)` in `hidpp_send_message_sync`, and
applying the event-handler hunk unchanged. Straightforward for the
stable maintainers.
**Step 6.3: Related Fixes in Stable**
- Record: No prior independent fix for this specific race is in stable.
Unrelated recent fixes (force-feedback UAF `f7a4c78b`) target other
paths.
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1: Criticality**
- Record: Subsystem: `drivers/hid/` HID++ Logitech driver. Logitech
Unifying receivers/MX mice/keyboards are ubiquitous on laptops and
desktops; the driver ships on most distributions. Classification:
IMPORTANT (wide hardware user base), not CORE.
**Step 7.2: Activity**
- Record: Actively developed — multiple merges per release cycle (device
IDs, quirks, bug fixes). Mature core codepaths in the driver have been
stable for years.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Who Is Affected**
- Record: Anyone with a Logitech HID++ device (mice, keyboards,
receivers, presenters) using the in-tree driver. Essentially most
laptop users with Logitech peripherals. Driver-specific but very
widespread hardware.
**Step 8.2: Trigger Conditions**
- Record: Requires concurrent activity: one thread invoking a sync
command while the device sends an asynchronous report. Races happen
during connect/disconnect, battery reporting, feature queries. The
very-first-use path yields NULL deref (no concurrency needed: any
async event before any sync command completes once) — but that path is
rare because probe typically drives the first sync command before any
report arrives. Attacker plane: a malicious or faulty HID device can
flood reports to widen the window — reachable from device-trust
boundary, relevant for BadUSB-style threat models.
**Step 8.3: Failure Mode Severity**
- Record: (a) NULL pointer dereference → kernel oops (CRITICAL: crash).
(b) Stale stack pointer read → OOB read (HIGH: info leak). (c) Stale
stack pointer WRITE via `*answer = *report` using device-controlled
data → stack corruption on an unrelated thread (CRITICAL: memory
corruption, potential privilege escalation/exploit primitive). Overall
severity: CRITICAL.
**Step 8.4: Risk/Benefit**
- Record: Benefit HIGH — closes a long-standing race with
crash/corruption potential in a widely-deployed driver. Risk LOW —
24-line surgical change, no new locking, no API change, reviewed and
applied by subsystem maintainer, only risk is a missed-answer timeout
that already has retry logic and a 5-second wait. Ratio clearly favors
backporting.
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: Evidence**
- FOR: Fixes real race/UAF/NULL-deref, very widespread hardware, small
and surgical, author has security-fix track record, applied by
subsystem maintainer, bug is 10+ years old (present in every active
stable), failure modes include potential stack corruption from device-
controlled data (security-relevant).
- AGAINST: No Reported-by/Fixes/Cc: stable tag; no public reproducer;
for older stables (≤v6.1) a trivial mechanical adaptation is needed
because the function was later split.
- UNRESOLVED: Exact kernel trigger rate in the wild is not documented;
CVE status unknown.
**Step 9.2: Stable Rules**
1. Obviously correct & tested? Yes — tiny, mechanical; applied by
maintainer.
2. Fixes real bug? Yes — race + stale/NULL pointer deref with concrete
failure modes.
3. Important? Yes — kernel oops and potential stack corruption.
4. Small & contained? Yes — 24 lines, one file, two functions.
5. No new features/APIs? Correct — pure fix.
6. Applies to stable? v6.6+: clean. ≤v6.1: needs trivial adaptation.
**Step 9.3: Exceptions**
- Not an exception category; qualifies on normal bug-fix merit.
**Step 9.4: Decision**
- The commit is a textbook stable candidate: small, targeted race fix in
a broadly used driver, repairing a NULL-deref + stale-pointer write
that a faulty/malicious device can provoke.
### Verification
- [Phase 1] Read full commit message — no Fixes/Reported-by/Cc: stable
tags; identified bug description and two-step fix.
- [Phase 2] Read diff end-to-end; confirmed +17/-7 in one file, two
functions, classic NULL-out + NULL-check pattern with `goto out`.
- [Phase 3] `git blame` on both functions: pattern dates to
`2f31c52529103d` (2014, v3.18). Split into
`__do_hidpp_send_message_sync` at `60165ab774cb0c` (v6.7).
- [Phase 3] `git log --oneline -- drivers/hid/hid-logitech-hidpp.c`:
file is actively maintained.
- [Phase 3] `git log --oneline --author="Benoit Sevens\|Benoît Sevens"`:
confirmed author's pattern of HID/UVC/ALSA security fixes.
- [Phase 4] `b4 dig -c e2aaf2d3ad92a`: found lore thread at `https://lor
e.kernel.org/all/20260401144811.1242722-1-bsevens@google.com/`.
- [Phase 4] `b4 dig -w`: confirmed CC list includes Jiri Kosina,
Benjamin Tissoires, Filipe Laíns, Bastien Nocera, linux-input, linux-
kernel.
- [Phase 4] `b4 dig -a`: only v1 was posted; no further revisions.
- [Phase 4] Read `/tmp/hidpp_race.mbox`: confirmed maintainer Jiri
Kosina's "Now applied" reply and note about deferred cleanup idea from
Benjamin Tissoires.
- [Phase 5] `rg send_receive_buf`: confirmed only 4 references (struct
member + 1 write + 2 reads), no other use sites.
- [Phase 5] Read `hidpp_raw_event` caller: confirmed registered as
`.raw_event` and called for every HID++ short/long/very-long report
from the device.
- [Phase 6] `git show vX:drivers/hid/hid-logitech-hidpp.c` for v4.19,
v5.4, v5.10, v5.15, v6.1, v6.6: verified vulnerable pattern exists in
every stable tree.
- [Phase 6] Verified `__do_hidpp_send_message_sync` only exists from
v6.6 onward; older trees need trivial adaptation of the producer hunk.
- [Phase 8] Reviewed event handler body: `*answer = *report` is a write,
confirming the stack-corruption / write-primitive aspect (not merely
an info leak).
- UNVERIFIED: Whether a CVE was assigned; whether a public reproducer
exists; exact in-wild trigger rate.
The fix is small, surgical, reviewed by the HID maintainer, repairs a
long-standing race with crash and stack-corruption consequences
affecting extremely common Logitech hardware, and backports cleanly to
recent stables with only a mechanical adjustment needed for older ones.
**YES**
drivers/hid/hid-logitech-hidpp.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5f63f1d2303a0..b1330d23bd2d0 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -306,21 +306,22 @@ static int __do_hidpp_send_message_sync(struct hidpp_device *hidpp,
if (ret) {
dbg_hid("__hidpp_send_report returned err: %d\n", ret);
memset(response, 0, sizeof(struct hidpp_report));
- return ret;
+ goto out;
}
if (!wait_event_timeout(hidpp->wait, hidpp->answer_available,
5*HZ)) {
dbg_hid("%s:timeout waiting for response\n", __func__);
memset(response, 0, sizeof(struct hidpp_report));
- return -ETIMEDOUT;
+ ret = -ETIMEDOUT;
+ goto out;
}
if (response->report_id == REPORT_ID_HIDPP_SHORT &&
response->rap.sub_id == HIDPP_ERROR) {
ret = response->rap.params[1];
dbg_hid("%s:got hidpp error %02X\n", __func__, ret);
- return ret;
+ goto out;
}
if ((response->report_id == REPORT_ID_HIDPP_LONG ||
@@ -328,10 +329,14 @@ static int __do_hidpp_send_message_sync(struct hidpp_device *hidpp,
response->fap.feature_index == HIDPP20_ERROR) {
ret = response->fap.params[1];
dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret);
- return ret;
+ goto out;
}
- return 0;
+ ret = 0;
+
+out:
+ hidpp->send_receive_buf = NULL;
+ return ret;
}
/*
@@ -3843,8 +3848,7 @@ static int hidpp_input_configured(struct hid_device *hdev,
static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
int size)
{
- struct hidpp_report *question = hidpp->send_receive_buf;
- struct hidpp_report *answer = hidpp->send_receive_buf;
+ struct hidpp_report *question, *answer;
struct hidpp_report *report = (struct hidpp_report *)data;
int ret;
int last_online;
@@ -3854,6 +3858,12 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
* previously sent command.
*/
if (unlikely(mutex_is_locked(&hidpp->send_mutex))) {
+ question = hidpp->send_receive_buf;
+ answer = hidpp->send_receive_buf;
+
+ if (!question)
+ return 0;
+
/*
* Check for a correct hidpp20 answer or the corresponding
* error
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox