Hi Dan,
- wireless@ (not relevant for net/ieee802154 and net/mac802154 changes)
+ Alex and Stefan, the 802154 maintainers
error27@xxxxxxxxx wrote on Thu, 15 Dec 2022 11:05:47 +0300:
> Hello Miquel Raynal,
>
> The patch 4d1c7d87030b: "mac802154: Move an skb free within the rx
> path" from Oct 26, 2022, leads to the following Smatch static checker
> warning:
>
> net/mac802154/rx.c:307 ieee802154_rx()
> warn: 'skb' was already freed.
It took me a good minute to figure this one out, actually the main
purpose of commit 4d1c7d87030b ("mac802154: Move an skb free within the
rx path") is to do the freeing in one part, that's why we no longer need to
do it in __ieee802154_rx_handle_packet(). But, well, I forgot the very
first check at the top which was still freeing the skb upon parsing error.
I will immediately send a fix, thanks for the report.
Miquèl
> net/mac802154/rx.c
> 271 void ieee802154_rx(struct ieee802154_local *local, struct
> sk_buff *skb) 272 {
> 273 u16 crc;
> 274
> 275 WARN_ON_ONCE(softirq_count() == 0);
> 276
> 277 if (local->suspended)
> 278 goto free_skb;
> 279
> 280 /* TODO: When a transceiver omits the checksum here,
> we 281 * add an own calculated one. This is currently an ugly
> 282 * solution because the monitor needs a crc here.
> 283 */
> 284 if (local->hw.flags & IEEE802154_HW_RX_OMIT_CKSUM) {
> 285 crc = crc_ccitt(0, skb->data, skb->len);
> 286 put_unaligned_le16(crc, skb_put(skb, 2));
> 287 }
> 288
> 289 rcu_read_lock();
> 290
> 291 ieee802154_monitors_rx(local, skb);
> 292
> 293 /* Level 1 filtering: Check the FCS by software when
> relevant */ 294 if (local->hw.phy->filtering ==
> IEEE802154_FILTERING_NONE) { 295 crc = crc_ccitt(0,
> skb->data, skb->len); 296 if (crc)
> 297 goto drop;
> 298 }
> 299 /* remove crc */
> 300 skb_trim(skb, skb->len - 2);
> 301
> 302 __ieee802154_rx_handle_packet(local, skb);
>
> This frees skb.
>
> 303
> 304 drop:
> 305 rcu_read_unlock();
> 306 free_skb:
> --> 307 kfree_skb(skb);
>
> Double free.
>
> 308 }
>
> regards,
> dan carpenter
