mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mac802154: rx: fix beacon/mac_cmd skb leak and short skb_trim underflow
@ 2026-09-19 21:36 Hui Peng
  2026-09-20 13:10 ` Miquel Raynal
  0 siblings, 1 reply; 2+ messages in thread
From: Hui Peng @ 2026-09-19 21:36 UTC (permalink / raw)
  To: alex.aring, stefan, miquel.raynal, davem, edumazet, kuba, pabeni
  Cc: linux-wpan, netdev, linux-kernel

Fix two sk_buff handling bugs in `net/mac802154/rx.c`:

1. In `ieee802154_subif_frame()`, the `skb` passed by
   `__ieee802154_rx_handle_packet()` is already a dedicated
   `skb_clone()` owned by `ieee802154_subif_frame()` (with `skb->users
   == 1`), and all normal return paths consume it without extra
   reference increments. However, the `IEEE802154_FC_TYPE_BEACON` and
   `IEEE802154_FC_TYPE_MAC_CMD` branches call `mac_pkt->skb =
   skb_get(skb)` (incrementing `skb->users` to `2`) and return
   `NET_RX_SUCCESS`. When the worker later calls
   `kfree_skb(mac_pkt->skb)`, `skb->users` only drops from `2` to `1`,
   permanently leaking every received Beacon and MAC Command `sk_buff`.
   Assign `mac_pkt->skb = skb` directly.
2. In `ieee802154_rx()`, when `IEEE802154_HW_RX_OMIT_CKSUM` is set,
   `skb_put(skb, 2)` is called without ensuring 2 bytes of tailroom, and
   when `IEEE802154_HW_RX_OMIT_CKSUM` is not set,
   `__ieee802154_rx_handle_packet()` calls `skb_trim(skb, skb->len - 2)`
   without verifying `skb->len >= 2`. Ensure tailroom with
   `pskb_expand_head(skb, 0, 2, GFP_ATOMIC)` and drop frames shorter
   than 2 bytes.

Fixes: 57588c71177f ("mac802154: Handle passive scanning")
Fixes: d021d218f6d9 ("mac802154: Handle received BEACON_REQ")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>

---
 net/mac802154/rx.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index 19b5382e85a8..dbdaf5962ca7 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -287,7 +287,7 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 		if (!mac_pkt)
 			goto fail;
 
-		mac_pkt->skb = skb_get(skb);
+		mac_pkt->skb = skb;
 		mac_pkt->sdata = sdata;
 		mac_pkt->page = sdata->local->scan_page;
 		mac_pkt->channel = sdata->local->scan_channel;
@@ -304,7 +304,7 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 		if (!mac_pkt)
 			goto fail;
 
-		mac_pkt->skb = skb_get(skb);
+		mac_pkt->skb = skb;
 		mac_pkt->sdata = sdata;
 		netdev_hold(sdata->dev, &mac_pkt->dev_tracker, GFP_ATOMIC);
 		spin_lock(&sdata->local->rx_lock);
@@ -487,10 +487,15 @@ void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb)
 	 * solution because the monitor needs a crc here.
 	 */
 	if (local->hw.flags & IEEE802154_HW_RX_OMIT_CKSUM) {
+		if (pskb_expand_head(skb, 0, 2, GFP_ATOMIC))
+			goto free_skb;
 		crc = crc_ccitt(0, skb->data, skb->len);
 		put_unaligned_le16(crc, skb_put(skb, 2));
 	}
 
+	if (skb->len < 2)
+		goto free_skb;
+
 	rcu_read_lock();
 
 	ieee802154_monitors_rx(local, skb);
-- 
2.55.0.1082.g2b9226bbc0-goog


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

* Re: [PATCH] mac802154: rx: fix beacon/mac_cmd skb leak and short skb_trim underflow
  2026-09-19 21:36 [PATCH] mac802154: rx: fix beacon/mac_cmd skb leak and short skb_trim underflow Hui Peng
@ 2026-09-20 13:10 ` Miquel Raynal
  0 siblings, 0 replies; 2+ messages in thread
From: Miquel Raynal @ 2026-09-20 13:10 UTC (permalink / raw)
  To: Hui Peng
  Cc: alex.aring, stefan, davem, edumazet, kuba, pabeni, linux-wpan,
	netdev, linux-kernel

On 19/09/2026 at 21:36:36 GMT, Hui Peng <benquike@gmail.com> wrote:

> Fix two sk_buff handling bugs in `net/mac802154/rx.c`:
>
> 1. In `ieee802154_subif_frame()`, the `skb` passed by
>    `__ieee802154_rx_handle_packet()` is already a dedicated
>    `skb_clone()` owned by `ieee802154_subif_frame()` (with `skb->users
>    == 1`), and all normal return paths consume it without extra
>    reference increments. However, the `IEEE802154_FC_TYPE_BEACON` and
>    `IEEE802154_FC_TYPE_MAC_CMD` branches call `mac_pkt->skb =
>    skb_get(skb)` (incrementing `skb->users` to `2`) and return
>    `NET_RX_SUCCESS`. When the worker later calls
>    `kfree_skb(mac_pkt->skb)`, `skb->users` only drops from `2` to `1`,
>    permanently leaking every received Beacon and MAC Command `sk_buff`.
>    Assign `mac_pkt->skb = skb` directly.

This is okay and should probably an independent fix.

> 2. In `ieee802154_rx()`, when `IEEE802154_HW_RX_OMIT_CKSUM` is set,
>    `skb_put(skb, 2)` is called without ensuring 2 bytes of tailroom, and
>    when `IEEE802154_HW_RX_OMIT_CKSUM` is not set,
>    `__ieee802154_rx_handle_packet()` calls `skb_trim(skb, skb->len - 2)`
>    without verifying `skb->len >= 2`. Ensure tailroom with
>    `pskb_expand_head(skb, 0, 2, GFP_ATOMIC)` and drop frames shorter
>    than 2 bytes.
>
> Fixes: 57588c71177f ("mac802154: Handle passive scanning")
> Fixes: d021d218f6d9 ("mac802154: Handle received BEACON_REQ")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
>
> ---
>  net/mac802154/rx.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
> index 19b5382e85a8..dbdaf5962ca7 100644
> --- a/net/mac802154/rx.c
> +++ b/net/mac802154/rx.c
> @@ -287,7 +287,7 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
>  		if (!mac_pkt)
>  			goto fail;
>  
> -		mac_pkt->skb = skb_get(skb);
> +		mac_pkt->skb = skb;
>  		mac_pkt->sdata = sdata;
>  		mac_pkt->page = sdata->local->scan_page;
>  		mac_pkt->channel = sdata->local->scan_channel;
> @@ -304,7 +304,7 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
>  		if (!mac_pkt)
>  			goto fail;
>  
> -		mac_pkt->skb = skb_get(skb);
> +		mac_pkt->skb = skb;
>  		mac_pkt->sdata = sdata;
>  		netdev_hold(sdata->dev, &mac_pkt->dev_tracker, GFP_ATOMIC);
>  		spin_lock(&sdata->local->rx_lock);
> @@ -487,10 +487,15 @@ void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb)
>  	 * solution because the monitor needs a crc here.
>  	 */
>  	if (local->hw.flags & IEEE802154_HW_RX_OMIT_CKSUM) {
> +		if (pskb_expand_head(skb, 0, 2, GFP_ATOMIC))
> +			goto free_skb;
>  		crc = crc_ccitt(0, skb->data, skb->len);
>  		put_unaligned_le16(crc, skb_put(skb, 2));

This, why not.

>  	}
>  
> +	if (skb->len < 2)
> +		goto free_skb;

But this? This is a situation that cannot happen, right? Either the PHY
adds the checksum, or it does not and we add it above.

> +
>  	rcu_read_lock();
>  
>  	ieee802154_monitors_rx(local, skb);

Miquèl

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

end of thread, other threads:[~2026-09-20 13:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-19 21:36 [PATCH] mac802154: rx: fix beacon/mac_cmd skb leak and short skb_trim underflow Hui Peng
2026-09-20 13:10 ` Miquel Raynal

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

all inboxes | Powered by JetHome®