[PATCH] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[PATCH] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[Muhammad Bilal]

smp_cmd_keypress_notify() accesses the received payload as
struct smp_cmd_keypress_notify without verifying that skb->len
contains enough data.

smp_sig_channel() removes the opcode byte before dispatching to
command handlers, so a SMP_CMD_KEYPRESS_NOTIFY packet without a
payload leaves skb->len equal to zero on entry to the handler,
causing a 1-byte out-of-bounds read from the heap.

Add a length check before accessing the payload and return
SMP_INVALID_PARAMS when the packet is too short, matching the
pattern used by other SMP command handlers.

Fixes: 1408bb6efb04 ("Bluetooth: Add dummy handler for LE SC keypress notification")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 net/bluetooth/smp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 98f1da4f5..4c98e2a3a 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -2932,6 +2932,9 @@ static int smp_cmd_keypress_notify(struct l2cap_conn *conn,
 {
 	struct smp_cmd_keypress_notify *kp = (void *) skb->data;
 
+	if (skb->len < sizeof(*kp))
+		return SMP_INVALID_PARAMS;
+
 	bt_dev_dbg(conn->hcon->hdev, "value 0x%02x", kp->value);
 
 	return 0;
-- 
2.54.0

RE: Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1096089

---Test result---

Test Summary:
CheckPatch                    PASS      0.74 seconds
GitLint                       PASS      0.35 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      28.81 seconds
CheckAllWarning               PASS      29.40 seconds
CheckSparse                   PASS      28.55 seconds
BuildKernel32                 PASS      26.07 seconds
TestRunnerSetup               PASS      576.08 seconds
TestRunner_smp-tester         PASS      18.39 seconds
IncrementalBuild              PASS      25.74 seconds



https://github.com/bluez/bluetooth-next/pull/205

---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[Paul Menzel]

Dear Muhammad,


Thank you for patch.

Am 17.05.26 um 16:54 schrieb Muhammad Bilal:
> smp_cmd_keypress_notify() accesses the received payload as
> struct smp_cmd_keypress_notify without verifying that skb->len
> contains enough data.
> 
> smp_sig_channel() removes the opcode byte before dispatching to
> command handlers, so a SMP_CMD_KEYPRESS_NOTIFY packet without a
> payload leaves skb->len equal to zero on entry to the handler,
> causing a 1-byte out-of-bounds read from the heap.
> 
> Add a length check before accessing the payload and return
> SMP_INVALID_PARAMS when the packet is too short, matching the
> pattern used by other SMP command handlers.
> 
> Fixes: 1408bb6efb04 ("Bluetooth: Add dummy handler for LE SC keypress notification")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>   net/bluetooth/smp.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 98f1da4f5..4c98e2a3a 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -2932,6 +2932,9 @@ static int smp_cmd_keypress_notify(struct l2cap_conn *conn,
>   {
>   	struct smp_cmd_keypress_notify *kp = (void *) skb->data;
>   
> +	if (skb->len < sizeof(*kp))
> +		return SMP_INVALID_PARAMS;
> +
>   	bt_dev_dbg(conn->hcon->hdev, "value 0x%02x", kp->value);

Add the check after the debug log, so it can be inspected?

>   
>   	return 0;


Kind regards,

Paul

Re: [PATCH] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[Muhammad Bilal]

Hi Paul,

Thanks for the review.

Moving the check after bt_dev_dbg() would not be safe since the debug
statement reads kp->value, which is exactly what the length check is guarding.

On a truncated SMP_CMD_KEYPRESS_NOTIFY packet, skb->len may be smaller
than sizeof(*kp) when entering the handler, so evaluating kp->value in
the debug log would already access out-of-bounds memory before the
guard is reached.

Therefore the length check needs to remain before any access to
kp->value.

Regards,
Muhammad Bilal

Re: [PATCH] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[Paul Menzel]

Dear Muhammad,


Am 17.05.26 um 20:08 schrieb Muhammad Bilal:

> Thanks for the review.

Thank you for your instant reply.

> Moving the check after bt_dev_dbg() would not be safe since the debug
> statement reads kp->value, which is exactly what the length check is guarding.
> 
> On a truncated SMP_CMD_KEYPRESS_NOTIFY packet, skb->len may be smaller
> than sizeof(*kp) when entering the handler, so evaluating kp->value in
> the debug log would already access out-of-bounds memory before the
> guard is reached.
> 
> Therefore the length check needs to remain before any access to
> kp->value.

Thank you for the explanation. Is there another to log the faulty value?


Kind regards,

Paul

Re: [PATCH] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[Muhammad Bilal]

Hi Paul,

There is no safe way to access kp->value in the truncated case, since
the payload is not guaranteed to be present when skb->len < sizeof(*kp).

If diagnostic information is still useful, only metadata can be logged:

	if (skb->len < sizeof(*kp)) {
		bt_dev_dbg(conn->hcon->hdev,
			   "truncated keypress notify, len=%u",
			   skb->len);
		return SMP_INVALID_PARAMS;
	}

This keeps visibility into malformed packets without touching unvalidated
memory. Happy to send a v2 if that looks good to you.

Regards,
Muhammad Bilal

Re: [PATCH] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[Paul Menzel]

Dear Muhammad,


Am 17.05.26 um 21:03 schrieb Muhammad Bilal:

> There is no safe way to access kp->value in the truncated case, since
> the payload is not guaranteed to be present when skb->len < sizeof(*kp).

Indeed, you are right.

> If diagnostic information is still useful, only metadata can be logged:
> 
> 	if (skb->len < sizeof(*kp)) {
> 		bt_dev_dbg(conn->hcon->hdev,
> 			   "truncated keypress notify, len=%u",
> 			   skb->len);
> 		return SMP_INVALID_PARAMS;
> 	}
> 
> This keeps visibility into malformed packets without touching unvalidated
> memory. Happy to send a v2 if that looks good to you.

It looks good to me. Hopefully the maintainers agree.


Kind regards,

Paul

Re: [PATCH] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[Luiz Augusto von Dentz]

Hi Muhammad,

On Sun, May 17, 2026 at 10:55 AM Muhammad Bilal <meatuni001@gmail.com> wrote:
>
> smp_cmd_keypress_notify() accesses the received payload as
> struct smp_cmd_keypress_notify without verifying that skb->len
> contains enough data.
>
> smp_sig_channel() removes the opcode byte before dispatching to
> command handlers, so a SMP_CMD_KEYPRESS_NOTIFY packet without a
> payload leaves skb->len equal to zero on entry to the handler,
> causing a 1-byte out-of-bounds read from the heap.
>
> Add a length check before accessing the payload and return
> SMP_INVALID_PARAMS when the packet is too short, matching the
> pattern used by other SMP command handlers.
>
> Fixes: 1408bb6efb04 ("Bluetooth: Add dummy handler for LE SC keypress notification")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  net/bluetooth/smp.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 98f1da4f5..4c98e2a3a 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -2932,6 +2932,9 @@ static int smp_cmd_keypress_notify(struct l2cap_conn *conn,
>  {
>         struct smp_cmd_keypress_notify *kp = (void *) skb->data;

Perhaps we should stop assigning it directly and instead just use
`skb_pull_data`, which performs bounds checks on its own.

> +       if (skb->len < sizeof(*kp))
> +               return SMP_INVALID_PARAMS;

I suggested we add a bt_dev_warn_ratelimit with something like "Too
small packet: skb->len %u < %u" to make debugging easier.

> +
>         bt_dev_dbg(conn->hcon->hdev, "value 0x%02x", kp->value);
>
>         return 0;
> --
> 2.54.0
>


-- 
Luiz Augusto von Dentz

[PATCH v2] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[Muhammad Bilal]

smp_cmd_keypress_notify() accesses the received payload as
struct smp_cmd_keypress_notify without verifying that skb->len
contains enough data.

smp_sig_channel() removes the opcode byte before dispatching to
command handlers, so a SMP_CMD_KEYPRESS_NOTIFY packet without a
payload leaves skb->len equal to zero on entry to the handler,
causing a 1-byte out-of-bounds read from the heap.

Use skb_pull_data() to safely consume the payload; it performs
a bounds check internally and returns NULL when the packet is too
short.  Add a ratelimited warning in that path to aid debugging
of malformed packets, matching the pattern used by hci_event.c.

Fixes: 1408bb6efb04 ("Bluetooth: Add dummy handler for LE SC keypress notification")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 net/bluetooth/smp.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 98f1da4f5..1b237e623 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -2930,7 +2930,15 @@ static int smp_cmd_dhkey_check(struct l2cap_conn *conn, struct sk_buff *skb)
 static int smp_cmd_keypress_notify(struct l2cap_conn *conn,
 				   struct sk_buff *skb)
 {
-	struct smp_cmd_keypress_notify *kp = (void *) skb->data;
+	struct smp_cmd_keypress_notify *kp;
+
+	kp = skb_pull_data(skb, sizeof(struct smp_cmd_keypress_notify));
+	if (!kp) {
+		bt_dev_warn_ratelimited(conn->hcon->hdev,
+					"Too small packet: skb->len %u < %zu",
+					skb->len, sizeof(struct smp_cmd_keypress_notify));
+		return SMP_INVALID_PARAMS;
+	}
 
 	bt_dev_dbg(conn->hcon->hdev, "value 0x%02x", kp->value);
 
-- 
2.54.0

RE: [v2] Bluetooth: SMP: add missing skb len check in smp_cmd_keypress_notify

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1096915

---Test result---

Test Summary:
CheckPatch                    PASS      0.54 seconds
GitLint                       PASS      0.20 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      26.48 seconds
CheckAllWarning               PASS      29.04 seconds
CheckSparse                   PASS      27.45 seconds
BuildKernel32                 PASS      25.80 seconds
TestRunnerSetup               PASS      568.32 seconds
TestRunner_smp-tester         PASS      17.82 seconds
IncrementalBuild              PASS      26.03 seconds



https://github.com/bluez/bluetooth-next/pull/212

---
Regards,
Linux Bluetooth