From: Benjamin Tissoires bentiss@kernel.org
[ Upstream commit 2c85c61d1332e1e16f020d76951baf167dcb6f7a ]
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 Acked-by: Johan Hovold johan@kernel.org Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Jiri Kosina jkosina@suse.com Stable-dep-of: 206342541fc8 ("HID: core: introduce hid_safe_input_report()") Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit 509c2605065004fc4cd86ee50a9350d402785307) [Lee: Backported to linux-6.12.y and beyond] Signed-off-by: Lee Jones lee@kernel.org (cherry picked from commit f9393998660f146970047bda31526aeb96190f28) Signed-off-by: Lee Jones lee@kernel.org --- drivers/hid/hid-core.c | 29 ++++++++++++++++++++++------- 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.c | 2 +- drivers/hid/wacom_sys.c | 6 +++--- drivers/staging/greybus/hid.c | 2 +- include/linux/hid.h | 4 ++-- 9 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index aa9ae6ccb28a..c73f4ac16fdf 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1775,8 +1775,8 @@ int __hid_request(struct hid_device *hid, struct hid_report *report, } EXPORT_SYMBOL_GPL(__hid_request);
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, - int interrupt) +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, + size_t bufsize, u32 size, int interrupt) { struct hid_report_enum *report_enum = hid->report_enum + type; struct hid_report *report; @@ -1784,16 +1784,24 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, int max_buffer_size = HID_MAX_BUFFER_SIZE; unsigned int a; u32 rsize, csize = size; + size_t bsize = bufsize; u8 *cdata = data; int ret = 0;
report = hid_get_report(report_enum, data); if (!report) - goto out; + return 0; + + if (unlikely(bsize < csize)) { + hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n", + report->id, csize, bsize); + return -EINVAL; + }
if (report_enum->numbered) { cdata++; csize--; + bsize--; }
rsize = hid_compute_report_size(report); @@ -1806,9 +1814,15 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, else if (rsize > max_buffer_size) rsize = max_buffer_size;
+ if (bsize < rsize) { + hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n", + report->id, rsize, bsize); + return -EINVAL; + } + if (csize < rsize) { dbg_hid("report %d is too short, (%d < %d)\n", report->id, - csize, rsize); + csize, rsize); memset(cdata + csize, 0, rsize - csize); }
@@ -1817,7 +1831,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, if (hid->claimed & HID_CLAIMED_HIDRAW) { ret = hidraw_report_event(hid, data, size); if (ret) - goto out; + return ret; }
if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) { @@ -1830,7 +1844,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
if (hid->claimed & HID_CLAIMED_INPUT) hidinput_report_event(hid, report); -out: + return ret; } EXPORT_SYMBOL_GPL(hid_report_raw_event); @@ -1851,6 +1865,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int i struct hid_report_enum *report_enum; struct hid_driver *hdrv; struct hid_report *report; + size_t bufsize = size; int ret = 0;
if (!hid) @@ -1889,7 +1904,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int i goto unlock; }
- ret = hid_report_raw_event(hid, type, data, size, interrupt); + ret = hid_report_raw_event(hid, type, data, bufsize, size, interrupt);
unlock: up(&hid->driver_input_lock); diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c index 699186ff2349..d2a56bf92b41 100644 --- a/drivers/hid/hid-gfrm.c +++ b/drivers/hid/hid-gfrm.c @@ -66,7 +66,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report, switch (data[1]) { case GFRM100_SEARCH_KEY_DOWN: ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_dn, - sizeof(search_key_dn), 1); + sizeof(search_key_dn), sizeof(search_key_dn), 1); break;
case GFRM100_SEARCH_KEY_AUDIO_DATA: @@ -74,7 +74,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report,
case GFRM100_SEARCH_KEY_UP: ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_up, - sizeof(search_key_up), 1); + sizeof(search_key_up), sizeof(search_key_up), 1); break;
default: diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 98562a0ed0c3..d31f2737b13d 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -3176,7 +3176,7 @@ static int hidpp10_consumer_keys_raw_event(struct hidpp_device *hidpp, memcpy(&consumer_report[1], &data[3], 4); /* We are called from atomic context */ hid_report_raw_event(hidpp->hid_dev, HID_INPUT_REPORT, - consumer_report, 5, 1); + consumer_report, sizeof(consumer_report), 5, 1);
return 1; } diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 948bd59ab5d2..c3bcc23d7c7c 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -449,7 +449,7 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report) }
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf, - size, 0); + size, size, 0); if (ret) dev_warn(&hdev->dev, "failed to report feature\n"); } diff --git a/drivers/hid/hid-primax.c b/drivers/hid/hid-primax.c index 1e6413d07cae..16e2a811eda9 100644 --- a/drivers/hid/hid-primax.c +++ b/drivers/hid/hid-primax.c @@ -44,7 +44,7 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report, data[0] |= (1 << (data[idx] - 0xE0)); data[idx] = 0; } - hid_report_raw_event(hid, HID_INPUT_REPORT, data, size, 0); + hid_report_raw_event(hid, HID_INPUT_REPORT, data, size, size, 0); return 1;
default: /* unknown report */ diff --git a/drivers/hid/hid-vivaldi.c b/drivers/hid/hid-vivaldi.c index d57ec1767037..fdfea1355ee7 100644 --- a/drivers/hid/hid-vivaldi.c +++ b/drivers/hid/hid-vivaldi.c @@ -126,7 +126,7 @@ static void vivaldi_feature_mapping(struct hid_device *hdev, }
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data, - report_len, 0); + report_len, report_len, 0); if (ret) { dev_warn(&hdev->dev, "failed to report feature %d\n", field->report->id); diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index abbfb53bb7dc..09b513812fff 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -79,7 +79,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev, int err;
size = kfifo_out(fifo, buf, sizeof(buf)); - err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false); + err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false); if (err) { hid_warn(hdev, "%s: unable to flush event due to error %d\n", __func__, err); @@ -324,7 +324,7 @@ static void wacom_feature_mapping(struct hid_device *hdev, data, n, WAC_CMD_RETRIES); if (ret == n && features->type == HID_GENERIC) { ret = hid_report_raw_event(hdev, - HID_FEATURE_REPORT, data, n, 0); + HID_FEATURE_REPORT, data, n, n, 0); } else if (ret == 2 && features->type != HID_GENERIC) { features->touch_max = data[1]; } else { @@ -385,7 +385,7 @@ static void wacom_feature_mapping(struct hid_device *hdev, data, n, WAC_CMD_RETRIES); if (ret == n) { ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, - data, n, 0); + data, n, n, 0); } else { hid_warn(hdev, "%s: could not retrieve sensor offsets\n", __func__); diff --git a/drivers/staging/greybus/hid.c b/drivers/staging/greybus/hid.c index ed706f39e87a..d68f60da0dd1 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, ghid->bufsize, size, 1); }
static void gb_hid_init_reports(struct gb_hid *ghid) diff --git a/include/linux/hid.h b/include/linux/hid.h index ab56fffb74a2..aaae2fecd4ae 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -1175,8 +1175,8 @@ static inline u32 hid_report_len(struct hid_report *report) return DIV_ROUND_UP(report->size, 8) + (report->id > 0); }
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, - int interrupt); +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, + size_t bufsize, u32 size, int interrupt);
/* HID quirks API */ unsigned long hid_lookup_quirk(const struct hid_device *hdev);