[PATCH v1 1/1] Bluetooth: L2CAP: fix heap over-read in l2cap_get_conf_opt

[PATCH v1 1/1] Bluetooth: L2CAP: fix heap over-read in l2cap_get_conf_opt

[Muhammad Bilal]

l2cap_get_conf_opt() reads opt->val via a switch on opt->len (1, 2,
or 4 bytes).  opt->len is a remote-controlled u8.  All three callers
loop on (len >= L2CAP_CONF_OPT_SIZE), so the loop body executes with
as few as 2 bytes remaining.  A packet ending with opt->len=4 and
only 2 bytes left causes get_unaligned_le32(opt->val) to read 4 bytes
past the buffer before the caller can act on the return value.

Commit 7c9cbd0b5e38 ("Bluetooth: Verify that l2cap_get_conf_opt
provides large enough buffer") added a post-call len < 0 guard in
each caller, but the over-read fires inside l2cap_get_conf_opt()
before that guard is reached.

Add a buflen parameter and validate L2CAP_CONF_OPT_SIZE + opt->len
<= buflen before any access to opt->val.  Return -EINVAL on
violation.  Update all three callers to capture the return value and
break on negative.  With the bounds check ensuring the option fits
within the remaining buffer, the post-call len < 0 check is no
longer needed and is removed.

Fixes: 7c9cbd0b5e38 ("Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 net/bluetooth/l2cap_core.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fdccd62ccca8..6052ffb280ac 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3051,12 +3051,23 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn, u8 code,
 }
 
 static inline int l2cap_get_conf_opt(void **ptr, int *type, int *olen,
-				     unsigned long *val)
+				     unsigned long *val, size_t buflen)
 {
 	struct l2cap_conf_opt *opt = *ptr;
 	int len;
 
+	/* Guard opt->len dereference: reject if the 2-byte option header
+	 * itself does not fit in the remaining buffer.
+	 */
+	if (buflen < L2CAP_CONF_OPT_SIZE)
+		return -EINVAL;
+
 	len = L2CAP_CONF_OPT_SIZE + opt->len;
+
+	/* Reject options whose payload extends past the remaining buffer. */
+	if ((size_t)len > buflen)
+		return -EINVAL;
+
 	*ptr += len;
 
 	*type = opt->type;
@@ -3437,9 +3448,11 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
 	BT_DBG("chan %p", chan);
 
 	while (len >= L2CAP_CONF_OPT_SIZE) {
-		len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
-		if (len < 0)
+		int optlen = l2cap_get_conf_opt(&req, &type, &olen, &val, len);
+
+		if (optlen < 0)
 			break;
+		len -= optlen;
 
 		hint  = type & L2CAP_CONF_HINT;
 		type &= L2CAP_CONF_MASK;
@@ -3675,9 +3688,11 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len,
 	BT_DBG("chan %p, rsp %p, len %d, req %p", chan, rsp, len, data);
 
 	while (len >= L2CAP_CONF_OPT_SIZE) {
-		len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
-		if (len < 0)
+		int optlen = l2cap_get_conf_opt(&rsp, &type, &olen, &val, len);
+
+		if (optlen < 0)
 			break;
+		len -= optlen;
 
 		switch (type) {
 		case L2CAP_CONF_MTU:
@@ -3946,9 +3961,11 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
 		return;
 
 	while (len >= L2CAP_CONF_OPT_SIZE) {
-		len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
-		if (len < 0)
+		int optlen = l2cap_get_conf_opt(&rsp, &type, &olen, &val, len);
+
+		if (optlen < 0)
 			break;
+		len -= optlen;
 
 		switch (type) {
 		case L2CAP_CONF_RFC:
-- 
2.53.0

Re: [PATCH v1 1/1] Bluetooth: L2CAP: fix heap over-read in l2cap_get_conf_opt

[Paul Menzel]

Dear Muhammad,


Thank you for your patch.

Am 26.05.26 um 04:17 schrieb Muhammad Bilal:
> l2cap_get_conf_opt() reads opt->val via a switch on opt->len (1, 2,
> or 4 bytes).  opt->len is a remote-controlled u8.  All three callers
> loop on (len >= L2CAP_CONF_OPT_SIZE), so the loop body executes with
> as few as 2 bytes remaining.  A packet ending with opt->len=4 and
> only 2 bytes left causes get_unaligned_le32(opt->val) to read 4 bytes
> past the buffer before the caller can act on the return value.
> 
> Commit 7c9cbd0b5e38 ("Bluetooth: Verify that l2cap_get_conf_opt
> provides large enough buffer") added a post-call len < 0 guard in
> each caller, but the over-read fires inside l2cap_get_conf_opt()
> before that guard is reached.
> 
> Add a buflen parameter and validate L2CAP_CONF_OPT_SIZE + opt->len
> <= buflen before any access to opt->val.  Return -EINVAL on
> violation.  Update all three callers to capture the return value and
> break on negative.  With the bounds check ensuring the option fits
> within the remaining buffer, the post-call len < 0 check is no
> longer needed and is removed.

By any chance, do you have a reproducer?

> Fixes: 7c9cbd0b5e38 ("Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>   net/bluetooth/l2cap_core.c | 31 ++++++++++++++++++++++++-------
>   1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index fdccd62ccca8..6052ffb280ac 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3051,12 +3051,23 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn, u8 code,
>   }
>   
>   static inline int l2cap_get_conf_opt(void **ptr, int *type, int *olen,
> -				     unsigned long *val)
> +				     unsigned long *val, size_t buflen)
>   {
>   	struct l2cap_conf_opt *opt = *ptr;
>   	int len;
>   
> +	/* Guard opt->len dereference: reject if the 2-byte option header
> +	 * itself does not fit in the remaining buffer.
> +	 */
> +	if (buflen < L2CAP_CONF_OPT_SIZE)
> +		return -EINVAL;

I always wonder, if Linux should log a debug message or even warning.

> +
>   	len = L2CAP_CONF_OPT_SIZE + opt->len;
> +
> +	/* Reject options whose payload extends past the remaining buffer. */
> +	if ((size_t)len > buflen)
> +		return -EINVAL;

Ditto.

> +
>   	*ptr += len;
>   
>   	*type = opt->type;
> @@ -3437,9 +3448,11 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
>   	BT_DBG("chan %p", chan);
>   
>   	while (len >= L2CAP_CONF_OPT_SIZE) {
> -		len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
> -		if (len < 0)
> +		int optlen = l2cap_get_conf_opt(&req, &type, &olen, &val, len);
> +
> +		if (optlen < 0)
>   			break;
> +		len -= optlen;
>   
>   		hint  = type & L2CAP_CONF_HINT;
>   		type &= L2CAP_CONF_MASK;
> @@ -3675,9 +3688,11 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len,
>   	BT_DBG("chan %p, rsp %p, len %d, req %p", chan, rsp, len, data);
>   
>   	while (len >= L2CAP_CONF_OPT_SIZE) {
> -		len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
> -		if (len < 0)
> +		int optlen = l2cap_get_conf_opt(&rsp, &type, &olen, &val, len);
> +
> +		if (optlen < 0)
>   			break;
> +		len -= optlen;
>   
>   		switch (type) {
>   		case L2CAP_CONF_MTU:
> @@ -3946,9 +3961,11 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
>   		return;
>   
>   	while (len >= L2CAP_CONF_OPT_SIZE) {
> -		len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
> -		if (len < 0)
> +		int optlen = l2cap_get_conf_opt(&rsp, &type, &olen, &val, len);
> +
> +		if (optlen < 0)
>   			break;
> +		len -= optlen;
>   
>   		switch (type) {
>   		case L2CAP_CONF_RFC:

The diff looks good.

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

Re: [PATCH v1 1/1] Bluetooth: L2CAP: fix heap over-read in l2cap_get_conf_opt

[Muhammad Bilal]

Thanks for the review.

> By any chance, do you have a reproducer?

No standalone reproducer is available. The issue can be triggered by
a malformed L2CAP configuration request where opt->len exceeds the
remaining buffer, i.e. a crafted packet from a remote peer.

> I always wonder, if Linux should log a debug message or even warning.

Existing callers generally handle malformed configuration options by
silently aborting parsing, so I followed the same pattern. Adding a
BT_ERR() on -EINVAL could be reasonable; I can include that in a v2
if preferred.

Regards,
Muhammad Bilal

Re: [PATCH v1 1/1] Bluetooth: L2CAP: fix heap over-read in l2cap_get_conf_opt

[Paul Menzel]

Dear Muhammad,


Am 27.05.26 um 07:18 schrieb Muhammad Bilal:

>> By any chance, do you have a reproducer?
> 
> No standalone reproducer is available. The issue can be triggered by
> a malformed L2CAP configuration request where opt->len exceeds the
> remaining buffer, i.e. a crafted packet from a remote peer.

Understood.

>> I always wonder, if Linux should log a debug message or even warning.
> 
> Existing callers generally handle malformed configuration options by
> silently aborting parsing, so I followed the same pattern. Adding a
> BT_ERR() on -EINVAL could be reasonable; I can include that in a v2
> if preferred.

Thank you for sharing the reasoning. It makes sense, and no need to add 
it then.


Kind regards,

Paul