* [PATCH 6.12.y 2/4] HID: pidff: Use ARRAY_SIZE macro instead of sizeof
2026-08-26 13:47 ` [PATCH 6.12.y 1/4] HID: pidff: Rework pidff_set_time() to fix warnings Sasha Levin
@ 2026-08-26 13:47 ` Sasha Levin
2026-08-26 13:47 ` [PATCH 6.12.y 3/4] HID: pidff: clang-format pass Sasha Levin
2026-08-26 13:47 ` [PATCH 6.12.y 4/4] HID: pidff: fix OOB write when hid->inputs is empty Sasha Levin
2 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-08-26 13:47 UTC (permalink / raw)
To: stable; +Cc: Tomasz Pakuła, Oleg Makarenko, Jiri Kosina, Sasha Levin
From: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
[ Upstream commit 8de2cef6d0de8cdddee8d653a18791525af26fad ]
Could lead to issues when arrays won't be 8 bit fields
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Stable-dep-of: 67bb1074e3d2 ("HID: pidff: fix OOB write when hid->inputs is empty")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/usbhid/hid-pidff.c | 46 +++++++++++++++++-----------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 471dbc8fabc98..fe5462a19df38 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -158,20 +158,20 @@ struct pidff_usage {
struct pidff_device {
struct hid_device *hid;
- struct hid_report *reports[sizeof(pidff_reports)];
+ struct hid_report *reports[ARRAY_SIZE(pidff_reports)];
- struct pidff_usage set_effect[sizeof(pidff_set_effect)];
- struct pidff_usage set_envelope[sizeof(pidff_set_envelope)];
- struct pidff_usage set_condition[sizeof(pidff_set_condition)];
- struct pidff_usage set_periodic[sizeof(pidff_set_periodic)];
- struct pidff_usage set_constant[sizeof(pidff_set_constant)];
- struct pidff_usage set_ramp[sizeof(pidff_set_ramp)];
+ struct pidff_usage set_effect[ARRAY_SIZE(pidff_set_effect)];
+ struct pidff_usage set_envelope[ARRAY_SIZE(pidff_set_envelope)];
+ struct pidff_usage set_condition[ARRAY_SIZE(pidff_set_condition)];
+ struct pidff_usage set_periodic[ARRAY_SIZE(pidff_set_periodic)];
+ struct pidff_usage set_constant[ARRAY_SIZE(pidff_set_constant)];
+ struct pidff_usage set_ramp[ARRAY_SIZE(pidff_set_ramp)];
- struct pidff_usage device_gain[sizeof(pidff_device_gain)];
- struct pidff_usage block_load[sizeof(pidff_block_load)];
- struct pidff_usage pool[sizeof(pidff_pool)];
- struct pidff_usage effect_operation[sizeof(pidff_effect_operation)];
- struct pidff_usage block_free[sizeof(pidff_block_free)];
+ struct pidff_usage device_gain[ARRAY_SIZE(pidff_device_gain)];
+ struct pidff_usage block_load[ARRAY_SIZE(pidff_block_load)];
+ struct pidff_usage pool[ARRAY_SIZE(pidff_pool)];
+ struct pidff_usage effect_operation[ARRAY_SIZE(pidff_effect_operation)];
+ struct pidff_usage block_free[ARRAY_SIZE(pidff_block_free)];
/*
* Special field is a field that is not composed of
@@ -194,10 +194,10 @@ struct pidff_device {
/* Special field in effect_operation */
struct hid_field *effect_operation_status;
- int control_id[sizeof(pidff_device_control)];
- int type_id[sizeof(pidff_effect_types)];
- int status_id[sizeof(pidff_block_load_status)];
- int operation_id[sizeof(pidff_effect_operation_status)];
+ int control_id[ARRAY_SIZE(pidff_device_control)];
+ int type_id[ARRAY_SIZE(pidff_effect_types)];
+ int status_id[ARRAY_SIZE(pidff_block_load_status)];
+ int operation_id[ARRAY_SIZE(pidff_effect_operation_status)];
int pid_id[PID_EFFECTS_MAX];
@@ -642,7 +642,7 @@ static void pidff_set_device_control(struct pidff_device *pidff, int field)
hid_dbg(pidff->hid, "DEVICE_CONTROL is a bitmask\n");
/* Clear current bitmask */
- for (i = 0; i < sizeof(pidff_device_control); i++) {
+ for (i = 0; i < ARRAY_SIZE(pidff_device_control); i++) {
index = pidff->control_id[i];
if (index < 1)
continue;
@@ -1002,7 +1002,7 @@ static int pidff_check_usage(int usage)
{
int i;
- for (i = 0; i < sizeof(pidff_reports); i++)
+ for (i = 0; i < ARRAY_SIZE(pidff_reports); i++)
if (usage == (HID_UP_PID | pidff_reports[i]))
return i;
@@ -1121,7 +1121,7 @@ static int pidff_find_special_keys(int *keys, struct hid_field *fld,
#define PIDFF_FIND_SPECIAL_KEYS(keys, field, name) \
pidff_find_special_keys(pidff->keys, pidff->field, pidff_ ## name, \
- sizeof(pidff_ ## name))
+ ARRAY_SIZE(pidff_ ## name))
/*
* Find and check the special fields
@@ -1196,7 +1196,7 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
if (PIDFF_FIND_SPECIAL_KEYS(status_id, block_load_status,
block_load_status) !=
- sizeof(pidff_block_load_status)) {
+ ARRAY_SIZE(pidff_block_load_status)) {
hid_err(pidff->hid,
"block load status identifiers not found\n");
return -1;
@@ -1204,7 +1204,7 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
if (PIDFF_FIND_SPECIAL_KEYS(operation_id, effect_operation_status,
effect_operation_status) !=
- sizeof(pidff_effect_operation_status)) {
+ ARRAY_SIZE(pidff_effect_operation_status)) {
hid_err(pidff->hid, "effect operation identifiers not found\n");
return -1;
}
@@ -1220,7 +1220,7 @@ static int pidff_find_effects(struct pidff_device *pidff,
{
int i;
- for (i = 0; i < sizeof(pidff_effect_types); i++) {
+ for (i = 0; i < ARRAY_SIZE(pidff_effect_types); i++) {
int pidff_type = pidff->type_id[i];
if (pidff->set_effect_type->usage[pidff_type].hid !=
@@ -1270,7 +1270,7 @@ static int pidff_find_effects(struct pidff_device *pidff,
#define PIDFF_FIND_FIELDS(name, report, strict) \
pidff_find_fields(pidff->name, pidff_ ## name, \
pidff->reports[report], \
- sizeof(pidff_ ## name), strict)
+ ARRAY_SIZE(pidff_ ## name), strict)
/*
* Fill and check the pidff_usages
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 6.12.y 3/4] HID: pidff: clang-format pass
2026-08-26 13:47 ` [PATCH 6.12.y 1/4] HID: pidff: Rework pidff_set_time() to fix warnings Sasha Levin
2026-08-26 13:47 ` [PATCH 6.12.y 2/4] HID: pidff: Use ARRAY_SIZE macro instead of sizeof Sasha Levin
@ 2026-08-26 13:47 ` Sasha Levin
2026-08-26 13:47 ` [PATCH 6.12.y 4/4] HID: pidff: fix OOB write when hid->inputs is empty Sasha Levin
2 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-08-26 13:47 UTC (permalink / raw)
To: stable; +Cc: Tomasz Pakuła, Jiri Kosina, Sasha Levin
From: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
[ Upstream commit ae42428fb4e3d2eed344f0d6fcfa778bc8b8f80a ]
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Stable-dep-of: 67bb1074e3d2 ("HID: pidff: fix OOB write when hid->inputs is empty")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/usbhid/hid-pidff.c | 55 ++++++++++++++++------------------
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index fe5462a19df38..72ed11e054665 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -9,12 +9,11 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include "hid-pidff.h"
+#include <linux/hid.h>
#include <linux/input.h>
+#include <linux/minmax.h>
#include <linux/slab.h>
#include <linux/usb.h>
-#include <linux/hid.h>
-#include <linux/minmax.h>
-
#define PID_EFFECTS_MAX 64
#define PID_INFINITE U16_MAX
@@ -285,7 +284,7 @@ static s32 pidff_clamp(s32 i, struct hid_field *field)
static int pidff_rescale(int i, int max, struct hid_field *field)
{
return i * (field->logical_maximum - field->logical_minimum) / max +
- field->logical_minimum;
+ field->logical_minimum;
}
/*
@@ -329,18 +328,18 @@ static void pidff_set_signed(struct pidff_usage *usage, s16 value)
else {
if (value < 0)
usage->value[0] =
- pidff_rescale(-value, -S16_MIN, usage->field);
+ pidff_rescale(-value, -S16_MIN, usage->field);
else
usage->value[0] =
- pidff_rescale(value, S16_MAX, usage->field);
+ pidff_rescale(value, S16_MAX, usage->field);
}
pr_debug("calculated from %d to %d\n", value, usage->value[0]);
}
static void pidff_set_time(struct pidff_usage *usage, u16 time)
{
- usage->value[0] = pidff_clamp(
- pidff_rescale_time(time, usage->field), usage->field);
+ usage->value[0] = pidff_clamp(pidff_rescale_time(time, usage->field),
+ usage->field);
}
static void pidff_set_duration(struct pidff_usage *usage, u16 duration)
@@ -463,11 +462,11 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
pidff_get_effect_type_id(pidff, effect);
pidff_set_duration(&pidff->set_effect[PID_DURATION],
- effect->replay.length);
+ effect->replay.length);
pidff->set_effect[PID_TRIGGER_BUTTON].value[0] = effect->trigger.button;
pidff_set_time(&pidff->set_effect[PID_TRIGGER_REPEAT_INT],
- effect->trigger.interval);
+ effect->trigger.interval);
pidff->set_effect[PID_GAIN].value[0] =
pidff->set_effect[PID_GAIN].field->logical_maximum;
pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = 1;
@@ -477,10 +476,10 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
/* Omit setting delay field if it's missing */
if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_DELAY))
pidff_set_time(&pidff->set_effect[PID_START_DELAY],
- effect->replay.delay);
+ effect->replay.delay);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_EFFECT],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -510,10 +509,10 @@ static void pidff_set_periodic_report(struct pidff_device *pidff,
effect->u.periodic.offset);
pidff_set(&pidff->set_periodic[PID_PHASE], effect->u.periodic.phase);
pidff_set_time(&pidff->set_periodic[PID_PERIOD],
- effect->u.periodic.period);
+ effect->u.periodic.period);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_PERIODIC],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -560,7 +559,7 @@ static void pidff_set_condition_report(struct pidff_device *pidff,
pidff_set(&pidff->set_condition[PID_DEAD_BAND],
effect->u.condition[i].deadband);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_CONDITION],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
}
@@ -623,7 +622,7 @@ static void pidff_set_gain_report(struct pidff_device *pidff, u16 gain)
pidff_set(&pidff->device_gain[PID_DEVICE_GAIN_FIELD], gain);
hid_hw_request(pidff->hid, pidff->reports[PID_DEVICE_GAIN],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -717,21 +716,19 @@ static void pidff_fetch_pool(struct pidff_device *pidff)
*/
static int pidff_request_effect_upload(struct pidff_device *pidff, int efnum)
{
- int j;
-
pidff->create_new_effect_type->value[0] = efnum;
hid_hw_request(pidff->hid, pidff->reports[PID_CREATE_NEW_EFFECT],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
hid_dbg(pidff->hid, "create_new_effect sent, type: %d\n", efnum);
pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0;
pidff->block_load_status->value[0] = 0;
hid_hw_wait(pidff->hid);
- for (j = 0; j < 60; j++) {
+ for (int i = 0; i < 60; i++) {
hid_dbg(pidff->hid, "pid_block_load requested\n");
hid_hw_request(pidff->hid, pidff->reports[PID_BLOCK_LOAD],
- HID_REQ_GET_REPORT);
+ HID_REQ_GET_REPORT);
hid_hw_wait(pidff->hid);
if (pidff->block_load_status->value[0] ==
pidff->status_id[PID_BLOCK_LOAD_SUCCESS]) {
@@ -808,8 +805,8 @@ static int pidff_erase_effect(struct input_dev *dev, int effect_id)
struct pidff_device *pidff = dev->ff->private;
int pid_id = pidff->pid_id[effect_id];
- hid_dbg(pidff->hid, "starting to erase %d/%d\n",
- effect_id, pidff->pid_id[effect_id]);
+ hid_dbg(pidff->hid, "starting to erase %d/%d\n", effect_id,
+ pidff->pid_id[effect_id]);
/*
* Wait for the queue to clear. We do not want
@@ -929,7 +926,7 @@ static void pidff_autocenter(struct pidff_device *pidff, u16 magnitude)
pidff->set_effect[PID_START_DELAY].value[0] = 0;
hid_hw_request(pidff->hid, pidff->reports[PID_SET_EFFECT],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -1196,7 +1193,7 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
if (PIDFF_FIND_SPECIAL_KEYS(status_id, block_load_status,
block_load_status) !=
- ARRAY_SIZE(pidff_block_load_status)) {
+ ARRAY_SIZE(pidff_block_load_status)) {
hid_err(pidff->hid,
"block load status identifiers not found\n");
return -1;
@@ -1204,7 +1201,7 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
if (PIDFF_FIND_SPECIAL_KEYS(operation_id, effect_operation_status,
effect_operation_status) !=
- ARRAY_SIZE(pidff_effect_operation_status)) {
+ ARRAY_SIZE(pidff_effect_operation_status)) {
hid_err(pidff->hid, "effect operation identifiers not found\n");
return -1;
}
@@ -1408,8 +1405,8 @@ static int pidff_check_autocenter(struct pidff_device *pidff,
int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks)
{
struct pidff_device *pidff;
- struct hid_input *hidinput = list_entry(hid->inputs.next,
- struct hid_input, list);
+ struct hid_input *hidinput =
+ list_entry(hid->inputs.next, struct hid_input, list);
struct input_dev *dev = hidinput->input;
struct ff_device *ff;
int max_effects;
@@ -1496,7 +1493,7 @@ int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks)
return 0;
- fail:
+fail:
hid_device_io_stop(hid);
kfree(pidff);
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 6.12.y 4/4] HID: pidff: fix OOB write when hid->inputs is empty
2026-08-26 13:47 ` [PATCH 6.12.y 1/4] HID: pidff: Rework pidff_set_time() to fix warnings Sasha Levin
2026-08-26 13:47 ` [PATCH 6.12.y 2/4] HID: pidff: Use ARRAY_SIZE macro instead of sizeof Sasha Levin
2026-08-26 13:47 ` [PATCH 6.12.y 3/4] HID: pidff: clang-format pass Sasha Levin
@ 2026-08-26 13:47 ` Sasha Levin
2 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-08-26 13:47 UTC (permalink / raw)
To: stable; +Cc: Baul Lee, Federico Kirschbaum, Jiri Kosina, Sasha Levin
From: Baul Lee <baul.lee@xbow.com>
[ Upstream commit 67bb1074e3d2d12fa059a9cc707e89398a4e4704 ]
hid_pidff_init_with_quirks() derives its input_dev from
list_entry(hid->inputs.next, struct hid_input, list)
without first checking that hid->inputs is non-empty. The list member
of struct hid_input is at offset 0, so on an empty list list_entry()
yields &hid->inputs itself and the following hidinput->input load reads
an unrelated member of struct hid_device. dev is then a type-confused
pointer, and force-feedback init writes through it: each
set_bit(FF_*, dev->ffbit) stores 8 bytes at dev + 192, past the end of
the object dev actually aliases, and input_ff_create() adds further
writes of a heap pointer and two function pointers.
Until hid-universal-pidff the only caller was hid_pidff_init() from
usbhid, which runs under HID_CLAIMED_INPUT and therefore always has at
least one hid_input. universal_pidff_probe() starts the device with
HID_CONNECT_DEFAULT & ~HID_CONNECT_FF and then calls
hid_pidff_init_with_quirks() directly whenever the descriptor carries a
PID usage page, bypassing that gate. A report descriptor whose only
application collection is on HID_UP_PID leaves hid->inputs empty while
hid_connect() still succeeds through the hidraw claim, so probe reaches
the unguarded list_entry().
The write happens in the USB probe path, on the hotplug workqueue, so
plugging in a malicious device is enough to trigger it; no attacker
software and no logged-in user are required. KASAN reports an 8-byte
out-of-bounds write in hid_pidff_init_with_quirks() reached from
universal_pidff_probe().
Check for an empty list before deriving dev and return -ENODEV, as the
other HID force-feedback drivers already do. universal_pidff_probe()
propagates the error and unwinds.
Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
Fixes: f06bf8d94fff ("HID: Add hid-universal-pidff driver and supported device ids")
Reported-by: Federico Kirschbaum <federico.kirschbaum@xbow.com>
Reported-by: Baul Lee <baul.lee@xbow.com>
Cc: stable@vger.kernel.org
Signed-off-by: Baul Lee <baul.lee@xbow.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/usbhid/hid-pidff.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 72ed11e054665..751570de7f52a 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -1405,13 +1405,20 @@ static int pidff_check_autocenter(struct pidff_device *pidff,
int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks)
{
struct pidff_device *pidff;
- struct hid_input *hidinput =
- list_entry(hid->inputs.next, struct hid_input, list);
- struct input_dev *dev = hidinput->input;
+ struct hid_input *hidinput;
+ struct input_dev *dev;
struct ff_device *ff;
int max_effects;
int error;
+ if (list_empty(&hid->inputs)) {
+ hid_err(hid, "no inputs found\n");
+ return -ENODEV;
+ }
+
+ hidinput = list_first_entry(&hid->inputs, struct hid_input, list);
+ dev = hidinput->input;
+
hid_dbg(hid, "starting pid init\n");
if (list_empty(&hid->report_enum[HID_OUTPUT_REPORT].report_list)) {
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread