Linux Input/HID development
 help / color / mirror / Atom feed
* [linux-6.6.y 1/3] HID: core: Add printk_ratelimited variants to hid_warn() etc
@ 2026-06-03 13:31 Lee Jones
  2026-06-03 13:31 ` [linux-6.6.y 2/3] HID: pass the buffer size to hid_report_raw_event Lee Jones
  2026-06-03 13:31 ` [linux-6.6.y 3/3] HID: core: Fix size_t specifier in hid_report_raw_event() Lee Jones
  0 siblings, 2 replies; 4+ messages in thread
From: Lee Jones @ 2026-06-03 13:31 UTC (permalink / raw)
  To: lee, Jiri Kosina, Benjamin Tissoires, Filipe Laíns,
	Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
	Johan Hovold, Alex Elder, Greg Kroah-Hartman, Sasha Levin,
	linux-input, linux-kernel, greybus-dev, linux-staging, bpf
  Cc: stable, Vicki Pfau, Jiri Kosina

From: Vicki Pfau <vi@endrift.com>

hid_warn_ratelimited() is needed. Add the others as part of the block.

Signed-off-by: Vicki Pfau <vi@endrift.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
(cherry picked from commit 1d64624243af8329b4b219d8c39e28ea448f9929)
Signed-off-by: Lee Jones <lee@kernel.org>
---
 include/linux/hid.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5d37e2349fae..f4bdaf1b8f41 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1252,4 +1252,15 @@ int hid_pidff_init_with_quirks(struct hid_device *hid, __u32 initial_quirks);
 #define hid_dbg_once(hid, fmt, ...)			\
 	dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
 
+#define hid_err_ratelimited(hid, fmt, ...)			\
+	dev_err_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice_ratelimited(hid, fmt, ...)			\
+	dev_notice_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn_ratelimited(hid, fmt, ...)			\
+	dev_warn_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info_ratelimited(hid, fmt, ...)			\
+	dev_info_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg_ratelimited(hid, fmt, ...)			\
+	dev_dbg_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+
 #endif
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [linux-6.6.y 2/3] HID: pass the buffer size to hid_report_raw_event
  2026-06-03 13:31 [linux-6.6.y 1/3] HID: core: Add printk_ratelimited variants to hid_warn() etc Lee Jones
@ 2026-06-03 13:31 ` Lee Jones
  2026-06-03 13:44   ` sashiko-bot
  2026-06-03 13:31 ` [linux-6.6.y 3/3] HID: core: Fix size_t specifier in hid_report_raw_event() Lee Jones
  1 sibling, 1 reply; 4+ messages in thread
From: Lee Jones @ 2026-06-03 13:31 UTC (permalink / raw)
  To: lee, Jiri Kosina, Benjamin Tissoires, Filipe Laíns,
	Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
	Johan Hovold, Alex Elder, Greg Kroah-Hartman, Sasha Levin,
	linux-input, linux-kernel, greybus-dev, linux-staging, bpf
  Cc: stable, Benjamin Tissoires, Jiri Kosina

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>
---
 drivers/hid/bpf/hid_bpf_dispatch.c |  3 ++-
 drivers/hid/hid-core.c             | 35 +++++++++++++++++++++---------
 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            |  4 ++--
 11 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 7903c8638e81..e0c03942ff7c 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -47,7 +47,7 @@ __weak noinline int hid_bpf_device_event(struct hid_bpf_ctx *ctx)
 
 u8 *
 dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type, u8 *data,
-			      u32 *size, int interrupt)
+			      size_t *buf_size, u32 *size, int interrupt)
 {
 	struct hid_bpf_ctx_kern ctx_kern = {
 		.ctx = {
@@ -81,6 +81,7 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
 		*size = ret;
 	}
 
+	*buf_size = ctx_kern.ctx.allocated_size;
 	return ctx_kern.data;
 }
 EXPORT_SYMBOL_GPL(dispatch_hid_bpf_device_event);
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e3d728d67b53..c6d98634b97e 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1986,24 +1986,32 @@ 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, enum hid_report_type type, u8 *data, u32 size,
-			 int interrupt)
+int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
+			 size_t bufsize, u32 size, int interrupt)
 {
 	struct hid_report_enum *report_enum = hid->report_enum + type;
 	struct hid_report *report;
 	struct hid_driver *hdrv;
 	int max_buffer_size = HID_MAX_BUFFER_SIZE;
 	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);
@@ -2016,9 +2024,15 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 	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);
 	}
 
@@ -2027,7 +2041,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 	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) {
@@ -2039,7 +2053,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 
 	if (hid->claimed & HID_CLAIMED_INPUT)
 		hidinput_report_event(hid, report);
-out:
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(hid_report_raw_event);
@@ -2055,12 +2069,13 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
  *
  * This is data entry for lower layers.
  */
-int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
-		     int interrupt)
+int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data,
+		     u32 size, int interrupt)
 {
 	struct hid_report_enum *report_enum;
 	struct hid_driver *hdrv;
 	struct hid_report *report;
+	size_t bufsize = size;
 	int ret = 0;
 
 	if (!hid)
@@ -2076,7 +2091,7 @@ int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data
 	report_enum = hid->report_enum + type;
 	hdrv = hid->driver;
 
-	data = dispatch_hid_bpf_device_event(hid, type, data, &size, interrupt);
+	data = dispatch_hid_bpf_device_event(hid, type, data, &bufsize, &size, interrupt);
 	if (IS_ERR(data)) {
 		ret = PTR_ERR(data);
 		goto unlock;
@@ -2105,7 +2120,7 @@ int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data
 			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 339a227c457e..113307157a27 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3692,7 +3692,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 003950894362..6c04eed0a464 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -479,7 +479,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-common.c b/drivers/hid/hid-vivaldi-common.c
index b0af2be94895..7fb986615768 100644
--- a/drivers/hid/hid-vivaldi-common.c
+++ b/drivers/hid/hid-vivaldi-common.c
@@ -85,7 +85,7 @@ 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 52011503ff3b..52b7f474d866 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -74,7 +74,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);
@@ -319,7 +319,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 {
@@ -380,7 +380,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 15335c38cb26..e761411ccf64 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 f4bdaf1b8f41..a9857fdfc327 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1205,8 +1205,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, enum hid_report_type type, u8 *data, u32 size,
-			 int interrupt);
+int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
+			 size_t bufsize, u32 size, int interrupt);
 
 /* HID quirks API */
 unsigned long hid_lookup_quirk(const struct hid_device *hdev);
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index e9afb61e6ee0..eafaac1109ac 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -147,7 +147,7 @@ struct hid_bpf_link {
 
 #ifdef CONFIG_HID_BPF
 u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
-				  u32 *size, int interrupt);
+				  size_t *buf_size, u32 *size, int interrupt);
 int hid_bpf_connect_device(struct hid_device *hdev);
 void hid_bpf_disconnect_device(struct hid_device *hdev);
 void hid_bpf_destroy_device(struct hid_device *hid);
@@ -155,7 +155,7 @@ void hid_bpf_device_init(struct hid_device *hid);
 u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size);
 #else /* CONFIG_HID_BPF */
 static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type,
-						u8 *data, u32 *size, int interrupt) { return data; }
+						u8 *data, size_t *buf_size, u32 *size, int interrupt) { return data; }
 static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; }
 static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {}
 static inline void hid_bpf_destroy_device(struct hid_device *hid) {}
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [linux-6.6.y 3/3] HID: core: Fix size_t specifier in hid_report_raw_event()
  2026-06-03 13:31 [linux-6.6.y 1/3] HID: core: Add printk_ratelimited variants to hid_warn() etc Lee Jones
  2026-06-03 13:31 ` [linux-6.6.y 2/3] HID: pass the buffer size to hid_report_raw_event Lee Jones
@ 2026-06-03 13:31 ` Lee Jones
  1 sibling, 0 replies; 4+ messages in thread
From: Lee Jones @ 2026-06-03 13:31 UTC (permalink / raw)
  To: lee, Jiri Kosina, Benjamin Tissoires, Filipe Laíns,
	Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
	Johan Hovold, Alex Elder, Greg Kroah-Hartman, Sasha Levin,
	linux-input, linux-kernel, greybus-dev, linux-staging, bpf
  Cc: stable, Nathan Chancellor, Miguel Ojeda, Linus Torvalds

From: Nathan Chancellor <nathan@kernel.org>

[ Upstream commit 4d3a2a466b8d68d852a1f3bbf11204b718428dc4 ]

When building for 32-bit platforms, for which 'size_t' is
'unsigned int', there are warnings around using the incorrect format
specifier to print bsize in hid_report_raw_event():

  drivers/hid/hid-core.c:2054:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
   2053 |                 hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
        |                                                                                         ~~~
        |                                                                                         %zu
   2054 |                                      report->id, csize, bsize);
        |                                                         ^~~~~
  drivers/hid/hid-core.c:2076:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
   2075 |                 hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
        |                                                                                          ~~~
        |                                                                                          %zu
   2076 |                                      report->id, rsize, bsize);
        |                                                         ^~~~~

Use the proper 'size_t' format specifier, '%zu', to clear up the
warnings.

Cc: stable@vger.kernel.org
Fixes: 2c85c61d1332 ("HID: pass the buffer size to hid_report_raw_event")
Reported-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://lore.kernel.org/20260516020430.110135-1-ojeda@kernel.org/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
(cherry picked from commit 3ab135238832446399614e7a4bb796d620717806)
Signed-off-by: Lee Jones <lee@kernel.org>
(cherry picked from commit 76606eed6cb62ec97afcc270948ef8335d02e944)
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/hid/hid-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index c6d98634b97e..2de4d831c526 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2003,7 +2003,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 		return 0;
 
 	if (unlikely(bsize < csize)) {
-		hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
+		hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %zu)\n",
 				     report->id, csize, bsize);
 		return -EINVAL;
 	}
@@ -2025,7 +2025,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 		rsize = max_buffer_size;
 
 	if (bsize < rsize) {
-		hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
+		hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
 				     report->id, rsize, bsize);
 		return -EINVAL;
 	}
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [linux-6.6.y 2/3] HID: pass the buffer size to hid_report_raw_event
  2026-06-03 13:31 ` [linux-6.6.y 2/3] HID: pass the buffer size to hid_report_raw_event Lee Jones
@ 2026-06-03 13:44   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-03 13:44 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-input, dmitry.torokhov, bpf

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] The patch fails to resolve the short report regression because `hid_input_report()` incorrectly defaults `bufsize` to the received payload `size`.
- [High] `wacom_wac_queue_flush()` incorrectly passes `size` instead of `sizeof(buf)` as the `bufsize` parameter.
- [Low] Incorrect format specifiers in `hid_warn_ratelimited` cause compiler warnings on 32-bit platforms.

Pre-existing issues:
- [Critical] Unbounded `memcpy` in `dispatch_hid_bpf_device_event` leads to a heap buffer overflow.
--

commit e280aff3473f90b2372da480e1f061c96bb64fd0
Author: Benjamin Tissoires <bentiss@kernel.org>

HID: pass the buffer size to hid_report_raw_event

This patch adds a buffer size parameter to hid_report_raw_event() to better
differentiate between the allocated buffer capacity and the actual payload
size. This aims to reject buffer overflow attempts while safely zero-padding
valid short reports. The changes propagate the new parameter through various
HID transport drivers.

[ Upstream commit 2c85c61d1332e1e16f020d76951baf167dcb6f7a ]

> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index 7903c8638e817..e0c03942ff7c4 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -47,7 +47,7 @@ __weak noinline int hid_bpf_device_event(struct hid_bpf_ctx *ctx)
>  
>  u8 *
>  dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type, u8 *data,
> -			      u32 *size, int interrupt)
> +			      size_t *buf_size, u32 *size, int interrupt)
>  {

[Severity: Critical]
This isn't a bug introduced by this patch, but can the memcpy inside this
function cause a heap buffer overflow?

When dispatch_hid_bpf_device_event() handles incoming events, it executes:

    memset(ctx_kern.data, 0, hdev->bpf.allocated_data);
    memcpy(ctx_kern.data, data, *size);

If a malicious device or a compromised userspace process using /dev/uhid
injects a report with a payload up to 4096 bytes (UHID_DATA_MAX), and this
exceeds hdev->bpf.allocated_data, doesn't this unbounded memcpy overwrite
the heap?

> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index e3d728d67b538..c6d98634b97e6 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
[ ... ]
> @@ -1986,24 +1986,32 @@ 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, enum hid_report_type type, u8 *data, u32 size,
> -			 int interrupt)
> +int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
> +			 size_t bufsize, u32 size, int interrupt)
>  {
>  	struct hid_report_enum *report_enum = hid->report_enum + type;
>  	struct hid_report *report;
>  	struct hid_driver *hdrv;
>  	int max_buffer_size = HID_MAX_BUFFER_SIZE;
>  	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);

[Severity: Low]
Does using %ld for size_t cause compiler warnings on 32-bit platforms?

Since size_t is an unsigned int on 32-bit architectures, shouldn't we use
%zu for bsize, and %u instead of %d for csize?

> +		return -EINVAL;
> +	}
>  
[ ... ]
> @@ -2055,12 +2069,13 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
>   *
>   * This is data entry for lower layers.
>   */
> -int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
> -		     int interrupt)
> +int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data,
> +		     u32 size, int interrupt)
>  {
>  	struct hid_report_enum *report_enum;
>  	struct hid_driver *hdrv;
>  	struct hid_report *report;
> +	size_t bufsize = size;

[Severity: High]
Does setting bufsize to size defeat the purpose of the new bounds checks?

By initializing bufsize to the received payload size instead of the
actual buffer capacity, the new checks in hid_report_raw_event():

    if (bsize < rsize) {
        hid_warn_ratelimited(...);
        return -EINVAL;
    }

will always evaluate to true for short reports, as bsize will equal the short
payload size (csize). Will this unconditionally reject short reports and
leave the original regression unfixed for standard HID devices?

>  	int ret = 0;
>  
[ ... ]
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 52011503ff3ba..52b7f474d8669 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -74,7 +74,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);

[Severity: High]
Should this pass sizeof(buf) as the buffer size instead of size?

Because buf is allocated as u8 buf[WACOM_PKGLEN_MAX] in this function,
passing size (the number of bytes read) as both bufsize and size means
the buffer size limit equals the payload size.

If this is a valid short report, won't it be incorrectly rejected by the
new size checks in hid_report_raw_event()?

>  		if (err) {
>  			hid_warn(hdev, "%s: unable to flush event due to error %d\n",
>  				 __func__, err);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603133140.3069226-2-lee@kernel.org?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-03 13:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-03 13:31 [linux-6.6.y 1/3] HID: core: Add printk_ratelimited variants to hid_warn() etc Lee Jones
2026-06-03 13:31 ` [linux-6.6.y 2/3] HID: pass the buffer size to hid_report_raw_event Lee Jones
2026-06-03 13:44   ` sashiko-bot
2026-06-03 13:31 ` [linux-6.6.y 3/3] HID: core: Fix size_t specifier in hid_report_raw_event() Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox