On 19/09/2026 at 21:36:36 GMT, Hui Peng <[email protected]> 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 <[email protected]>
>
> ---
> 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
