Skip to content

Commit 7316fef

Browse files
author
Benjamin Tissoires
committed
HID: core: remove one more kmemdup on .probe()
That last kmemdup while opening the report descriptor was required to have a common kfree() on it. Move that kmemdup in the only special case it's required (if there is a .report_fixup()), and add a more elaborated check before freeing hdev->rdesc, to avoid a double free. Reviewed-by: Peter Hutterer <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Benjamin Tissoires <[email protected]>
1 parent 52cd190 commit 7316fef

File tree

1 file changed

+20
-8
lines changed

1 file changed

+20
-8
lines changed

drivers/hid/hid-core.c

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,14 @@ static void hid_close_report(struct hid_device *device)
685685
INIT_LIST_HEAD(&report_enum->report_list);
686686
}
687687

688-
kfree(device->rdesc);
688+
/*
689+
* If the HID driver had a rdesc_fixup() callback, dev->rdesc
690+
* will be allocated by hid-core and needs to be freed.
691+
* Otherwise, it is either equal to dev_rdesc or bpf_rdesc, in
692+
* which cases it'll be freed later on device removal or destroy.
693+
*/
694+
if (device->rdesc != device->dev_rdesc && device->rdesc != device->bpf_rdesc)
695+
kfree(device->rdesc);
689696
device->rdesc = NULL;
690697
device->rsize = 0;
691698

@@ -1214,7 +1221,6 @@ int hid_open_report(struct hid_device *device)
12141221
struct hid_item item;
12151222
unsigned int size;
12161223
const __u8 *start;
1217-
__u8 *buf = NULL;
12181224
const __u8 *end;
12191225
const __u8 *next;
12201226
int ret;
@@ -1241,17 +1247,23 @@ int hid_open_report(struct hid_device *device)
12411247
* on a copy of our report descriptor so it can
12421248
* change it.
12431249
*/
1244-
buf = kmemdup(start, size, GFP_KERNEL);
1250+
__u8 *buf = kmemdup(start, size, GFP_KERNEL);
1251+
12451252
if (buf == NULL)
12461253
return -ENOMEM;
12471254

12481255
start = device->driver->report_fixup(device, buf, &size);
1249-
}
12501256

1251-
start = kmemdup(start, size, GFP_KERNEL);
1252-
kfree(buf);
1253-
if (start == NULL)
1254-
return -ENOMEM;
1257+
/*
1258+
* The second kmemdup is required in case report_fixup() returns
1259+
* a static read-only memory, but we have no idea if that memory
1260+
* needs to be cleaned up or not at the end.
1261+
*/
1262+
start = kmemdup(start, size, GFP_KERNEL);
1263+
kfree(buf);
1264+
if (start == NULL)
1265+
return -ENOMEM;
1266+
}
12551267

12561268
device->rdesc = start;
12571269
device->rsize = size;

0 commit comments

Comments
 (0)