[PATCH v2] Bluetooth: HIDP: fix missing length checks in hidp_input_report()

Muhammad Bilal posted 1 patch 3 days, 23 hours ago
There is a newer version of this series
net/bluetooth/hidp/core.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
[PATCH v2] Bluetooth: HIDP: fix missing length checks in hidp_input_report()
Posted by Muhammad Bilal 3 days, 23 hours ago
hidp_input_report() reads keyboard and mouse payload data from an skb
without first verifying that skb->len contains enough data.

hidp_recv_intr_frame() pulls the 1-byte HIDP header before dispatching
to hidp_input_report(). If a paired device sends a truncated packet,
the handler reads beyond the valid skb data, resulting in an
out-of-bounds read of skb data. The OOB bytes may be interpreted as
phantom key presses or spurious mouse movement.

Add a check that skb->len is non-zero before the type switch, and
per-report-type minimum length checks before accessing the payload.

Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 net/bluetooth/hidp/core.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 976f91eeb..03838a6ff 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -179,12 +179,22 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
 {
 	struct input_dev *dev = session->input;
 	unsigned char *keys = session->keys;
-	unsigned char *udata = skb->data + 1;
-	signed char *sdata = skb->data + 1;
-	int i, size = skb->len - 1;
+	unsigned char *udata;
+	signed char *sdata;
+	int i, size;
+
+	if (!skb->len)
+		return;
+
+	udata = skb->data + 1;
+	sdata = skb->data + 1;
+	size = skb->len - 1;
 
 	switch (skb->data[0]) {
 	case 0x01:	/* Keyboard report */
+		if (size < 8)
+			break;
+
 		for (i = 0; i < 8; i++)
 			input_report_key(dev, hidp_keycode[i + 224], (udata[0] >> i) & 1);
 
@@ -213,6 +223,9 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
 		break;
 
 	case 0x02:	/* Mouse report */
+		if (size < 3)
+			break;
+
 		input_report_key(dev, BTN_LEFT,   sdata[0] & 0x01);
 		input_report_key(dev, BTN_RIGHT,  sdata[0] & 0x02);
 		input_report_key(dev, BTN_MIDDLE, sdata[0] & 0x04);
-- 
2.54.0
Re: [PATCH v2] Bluetooth: HIDP: fix missing length checks in hidp_input_report()
Posted by Luiz Augusto von Dentz 3 days, 23 hours ago
Hi Muhammad,

On Wed, May 20, 2026 at 5:41 PM Muhammad Bilal <meatuni001@gmail.com> wrote:
>
> hidp_input_report() reads keyboard and mouse payload data from an skb
> without first verifying that skb->len contains enough data.
>
> hidp_recv_intr_frame() pulls the 1-byte HIDP header before dispatching
> to hidp_input_report(). If a paired device sends a truncated packet,
> the handler reads beyond the valid skb data, resulting in an
> out-of-bounds read of skb data. The OOB bytes may be interpreted as
> phantom key presses or spurious mouse movement.
>
> Add a check that skb->len is non-zero before the type switch, and
> per-report-type minimum length checks before accessing the payload.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  net/bluetooth/hidp/core.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 976f91eeb..03838a6ff 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -179,12 +179,22 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
>  {
>         struct input_dev *dev = session->input;
>         unsigned char *keys = session->keys;
> -       unsigned char *udata = skb->data + 1;
> -       signed char *sdata = skb->data + 1;
> -       int i, size = skb->len - 1;
> +       unsigned char *udata;
> +       signed char *sdata;
> +       int i, size;
> +
> +       if (!skb->len)
> +               return;
> +
> +       udata = skb->data + 1;
> +       sdata = skb->data + 1;
> +       size = skb->len - 1;

If you use skb_pull_data, you won't need to use pointer arithmetic, or
store the actual size.

>
>         switch (skb->data[0]) {
>         case 0x01:      /* Keyboard report */
> +               if (size < 8)
> +                       break;
> +
>                 for (i = 0; i < 8; i++)
>                         input_report_key(dev, hidp_keycode[i + 224], (udata[0] >> i) & 1);
>
> @@ -213,6 +223,9 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
>                 break;
>
>         case 0x02:      /* Mouse report */
> +               if (size < 3)
> +                       break;
> +
>                 input_report_key(dev, BTN_LEFT,   sdata[0] & 0x01);
>                 input_report_key(dev, BTN_RIGHT,  sdata[0] & 0x02);
>                 input_report_key(dev, BTN_MIDDLE, sdata[0] & 0x04);
> --
> 2.54.0
>


-- 
Luiz Augusto von Dentz
[PATCH v3] Bluetooth: HIDP: fix missing length checks in hidp_input_report()
Posted by Muhammad Bilal 3 days, 22 hours ago
hidp_input_report() reads keyboard and mouse payload data from an skb
without first verifying that skb->len contains enough data.

hidp_recv_intr_frame() pulls the 1-byte HIDP header before dispatching
to hidp_input_report(). If a paired device sends a truncated packet,
the handler reads beyond the valid skb data, resulting in an
out-of-bounds read of skb data. The OOB bytes may be interpreted as
phantom key presses or spurious mouse movement.

Replace the open-coded length tracking and pointer arithmetic with
skb_pull_data() calls. skb_pull_data() returns NULL if the requested
bytes are not present, eliminating the need for a manual size variable
and the separate skb->len guard.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
v3:
 - Replace manual length checks and pointer arithmetic with
   skb_pull_data() per Luiz's review
v2:
 - Add Cc: stable@vger.kernel.org per Greg KH's note
---
 net/bluetooth/hidp/core.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 976f91eeb..70344bd32 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -179,12 +179,21 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
 {
 	struct input_dev *dev = session->input;
 	unsigned char *keys = session->keys;
-	unsigned char *udata = skb->data + 1;
-	signed char *sdata = skb->data + 1;
-	int i, size = skb->len - 1;
+	unsigned char *udata;
+	signed char *sdata;
+	u8 *hdr;
+	int i;
+
+	hdr = skb_pull_data(skb, 1);
+	if (!hdr)
+		return;
 
-	switch (skb->data[0]) {
+	switch (*hdr) {
 	case 0x01:	/* Keyboard report */
+		udata = skb_pull_data(skb, 8);
+		if (!udata)
+			break;
+
 		for (i = 0; i < 8; i++)
 			input_report_key(dev, hidp_keycode[i + 224], (udata[0] >> i) & 1);
 
@@ -213,6 +222,10 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
 		break;
 
 	case 0x02:	/* Mouse report */
+		sdata = skb_pull_data(skb, 3);
+		if (!sdata)
+			break;
+
 		input_report_key(dev, BTN_LEFT,   sdata[0] & 0x01);
 		input_report_key(dev, BTN_RIGHT,  sdata[0] & 0x02);
 		input_report_key(dev, BTN_MIDDLE, sdata[0] & 0x04);
@@ -222,7 +235,7 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
 		input_report_rel(dev, REL_X, sdata[1]);
 		input_report_rel(dev, REL_Y, sdata[2]);
 
-		if (size > 3)
+		if (skb->len > 0)
 			input_report_rel(dev, REL_WHEEL, sdata[3]);
 		break;
 	}
-- 
2.54.0