From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Dan Carpenter <error27@gmail.com>
Cc: linux-wpan@vger.kernel.org, linux-wireless@vger.kernel.org,
Alexander Aring <aahringo@redhat.com>,
Stefan Schmidt <stefan@datenfreihafen.org>
Subject: Re: [bug report] mac802154: Move an skb free within the rx path
Date: Fri, 16 Dec 2022 14:59:46 +0100 [thread overview]
Message-ID: <20221216145946.3b38c86f@xps-13> (raw)
In-Reply-To: <Y5rVW/Mb8nw0MCF3@kili>
Hi Dan,
- wireless@ (not relevant for net/ieee802154 and net/mac802154 changes)
+ Alex and Stefan, the 802154 maintainers
error27@gmail.com 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
prev parent reply other threads:[~2022-12-16 13:59 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-15 8:05 [bug report] mac802154: Move an skb free within the rx path Dan Carpenter
2022-12-16 13:59 ` Miquel Raynal [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221216145946.3b38c86f@xps-13 \
--to=miquel.raynal@bootlin.com \
--cc=aahringo@redhat.com \
--cc=error27@gmail.com \
--cc=linux-wireless@vger.kernel.org \
--cc=linux-wpan@vger.kernel.org \
--cc=stefan@datenfreihafen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.