[PATCH] HID: core: fix OOB read of field->usage in hid_set_field()

[PATCH] HID: core: fix OOB read of field->usage in hid_set_field()

[Baul Lee]

hid_set_field() hands field->usage + offset to hid_dump_input() before
the guard that bounds offset:

	hid_dump_input(field->report->device, field->usage + offset, value);

	if (offset >= field->report_count) {
		hid_err(...);
		return -1;
	}

Under CONFIG_DEBUG_FS hid_dump_input() dereferences that pointer, with
buf = hid_resolv_usage(usage->hid, NULL).  The usage[] array is
allocated inline with the hid_field in hid_register_field() and holds
field->maxusage entries, so an offset past it reads off the end of the
kvzalloc()ed allocation and into a neighbouring object.  Had the guard
run first, offset < report_count <= maxusage would already have confined
the pointer to the array.

A caller supplies such an offset today.  picolcd_fb_send_tile()
validates only report->maxfield before issuing
hid_set_field(report->field[0], 11 + i, ...) for i = 0..31, so its
offsets are fixed at 11..42 and are never checked against the bound
field.  When the device registers that field with fewer usages, the
framebuffer deferred-io work drives the read on every tile.  KASAN
reports a 4-byte slab-out-of-bounds read in hid_dump_input() below
hid_set_field(), and the same boot logs "offset (1) exceeds
report_count (1)" from the guard that runs only afterwards.

Move the hid_dump_input() call below the guard.  Because
field->maxusage >= field->report_count, the guard then establishes that
field->usage + offset lies inside the array before it is dereferenced,
for every caller and without changing behaviour on the valid path.

Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
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>
---
 drivers/hid/hid-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index f107f5103b35..c3035414109d 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1933,13 +1933,14 @@ int hid_set_field(struct hid_field *field, unsigned offset, __s32 value)
 
 	size = field->report_size;
 
-	hid_dump_input(field->report->device, field->usage + offset, value);
-
 	if (offset >= field->report_count) {
 		hid_err(field->report->device, "offset (%d) exceeds report_count (%d)\n",
 				offset, field->report_count);
 		return -1;
 	}
+
+	hid_dump_input(field->report->device, field->usage + offset, value);
+
 	if (field->logical_minimum < 0) {
 		if (value != snto32(s32ton(value, size), size)) {
 			hid_err(field->report->device, "value %d is out of range\n", value);
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] HID: core: fix OOB read of field->usage in hid_set_field()

[Jiri Kosina]

On Sun, 26 Jul 2026, Baul Lee wrote:

> hid_set_field() hands field->usage + offset to hid_dump_input() before
> the guard that bounds offset:
> 
> 	hid_dump_input(field->report->device, field->usage + offset, value);
> 
> 	if (offset >= field->report_count) {
> 		hid_err(...);
> 		return -1;
> 	}
> 
> Under CONFIG_DEBUG_FS hid_dump_input() dereferences that pointer, with
> buf = hid_resolv_usage(usage->hid, NULL).  The usage[] array is
> allocated inline with the hid_field in hid_register_field() and holds
> field->maxusage entries, so an offset past it reads off the end of the
> kvzalloc()ed allocation and into a neighbouring object.  Had the guard
> run first, offset < report_count <= maxusage would already have confined
> the pointer to the array.
> 
> A caller supplies such an offset today.  picolcd_fb_send_tile()
> validates only report->maxfield before issuing
> hid_set_field(report->field[0], 11 + i, ...) for i = 0..31, so its
> offsets are fixed at 11..42 and are never checked against the bound
> field.  When the device registers that field with fewer usages, the
> framebuffer deferred-io work drives the read on every tile.  KASAN
> reports a 4-byte slab-out-of-bounds read in hid_dump_input() below
> hid_set_field(), and the same boot logs "offset (1) exceeds
> report_count (1)" from the guard that runs only afterwards.
> 
> Move the hid_dump_input() call below the guard.  Because
> field->maxusage >= field->report_count, the guard then establishes that
> field->usage + offset lies inside the array before it is dereferenced,
> for every caller and without changing behaviour on the valid path.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 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>

Good catch. Applied, thanks.

-- 
Jiri Kosina
SUSE Labs