[PATCH] Bluetooth: RFCOMM: require a credit byte before consuming it

Pengpeng Hou posted 1 patch 1 month, 4 weeks ago
net/bluetooth/rfcomm/core.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] Bluetooth: RFCOMM: require a credit byte before consuming it
Posted by Pengpeng Hou 1 month, 4 weeks ago
rfcomm_recv_data() treats the first payload byte as a credit field when
the UIH frame carries PF and credit-based flow control is enabled.

After the header has been stripped, the code does not re-check that the
frame still has at least one payload byte before dereferencing skb->data.
A malformed short frame can therefore trigger an out-of-bounds read.

Drop the frame if the optional credit byte is not present.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 net/bluetooth/rfcomm/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 611a9a94151e..964a78d473cc 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1715,6 +1715,9 @@ static int rfcomm_recv_data(struct rfcomm_session *s, u8 dlci, int pf, struct sk
 	}
 
 	if (pf && d->cfc) {
+		if (!skb->len)
+			goto drop;
+
 		u8 credits = *(u8 *) skb->data; skb_pull(skb, 1);
 
 		d->tx_credits += credits;
-- 
2.50.1 (Apple Git-155)
Re: [PATCH] Bluetooth: RFCOMM: require a credit byte before consuming it
Posted by Luiz Augusto von Dentz 1 month, 3 weeks ago
Hi Pengpeng,

On Fri, Apr 17, 2026 at 3:35 AM Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
>
> rfcomm_recv_data() treats the first payload byte as a credit field when
> the UIH frame carries PF and credit-based flow control is enabled.
>
> After the header has been stripped, the code does not re-check that the
> frame still has at least one payload byte before dereferencing skb->data.
> A malformed short frame can therefore trigger an out-of-bounds read.
>
> Drop the frame if the optional credit byte is not present.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  net/bluetooth/rfcomm/core.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 611a9a94151e..964a78d473cc 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -1715,6 +1715,9 @@ static int rfcomm_recv_data(struct rfcomm_session *s, u8 dlci, int pf, struct sk
>         }
>
>         if (pf && d->cfc) {
> +               if (!skb->len)
> +                       goto drop;

We can probably use skb_pull_data below, which checks skb->len.

>                 u8 credits = *(u8 *) skb->data; skb_pull(skb, 1);
>
>                 d->tx_credits += credits;
> --
> 2.50.1 (Apple Git-155)
>


-- 
Luiz Augusto von Dentz
[PATCH v2] Bluetooth: RFCOMM: pull credit byte with skb_pull_data()
Posted by Pengpeng Hou 1 month, 3 weeks ago
rfcomm_recv_data() treats the first payload byte as a credit field when
the UIH frame carries PF and credit-based flow control is enabled.

After the header has been stripped, the PF/CFC path consumes that byte
with a direct skb->data dereference followed by skb_pull(). A malformed
short frame can reach this path without a byte available.

Use skb_pull_data() so the length check and pull happen together before
the returned credit byte is consumed.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- use skb_pull_data() as suggested by Luiz Augusto von Dentz

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 611a9a94151e..d11bd5337d57 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1715,9 +1715,12 @@ static int rfcomm_recv_data(struct rfcomm_session *s, u8 dlci, int pf, struct sk
 	}
 
 	if (pf && d->cfc) {
-		u8 credits = *(u8 *) skb->data; skb_pull(skb, 1);
+		u8 *credits = skb_pull_data(skb, 1);
 
-		d->tx_credits += credits;
+		if (!credits)
+			goto drop;
+
+		d->tx_credits += *credits;
 		if (d->tx_credits)
 			clear_bit(RFCOMM_TX_THROTTLED, &d->flags);
 	}
-- 
2.50.1 (Apple Git-155)