netdev - Re: [PATCH wpan] mac802154: Fix LQI recording

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <370df52d-8e9f-3628-36db-a59c1722e01f@datenfreihafen.org>
Date:   Mon, 24 Oct 2022 11:19:34 +0200
From:   Stefan Schmidt <stefan@...enfreihafen.org>
To:     Miquel Raynal <miquel.raynal@...tlin.com>,
        Alexander Aring <alex.aring@...il.com>,
        linux-wpan@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
        David Girault <david.girault@...vo.com>,
        Romuald Despres <romuald.despres@...vo.com>,
        Frederic Blain <frederic.blain@...vo.com>,
        Nicolas Schodet <nico@...fr.eu.org>,
        Guilhem Imberton <guilhem.imberton@...vo.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH wpan] mac802154: Fix LQI recording

Hello.

On 20.10.22 16:25, Miquel Raynal wrote:
> Back in 2014, the LQI was saved in the skb control buffer (skb->cb, or
> mac_cb(skb)) without any actual reset of this area prior to its use.
> 
> As part of a useful rework of the use of this region, 32edc40ae65c
> ("ieee802154: change _cb handling slightly") introduced mac_cb_init() to
> basically memset the cb field to 0. In particular, this new function got
> called at the beginning of mac802154_parse_frame_start(), right before
> the location where the buffer got actually filled.
> 
> What went through unnoticed however, is the fact that the very first
> helper called by device drivers in the receive path already used this
> area to save the LQI value for later extraction. Resetting the cb field
> "so late" led to systematically zeroing the LQI.
> 
> If we consider the reset of the cb field needed, we can make it as soon
> as we get an skb from a device driver, right before storing the LQI,
> as is the very first time we need to write something there.
> 
> Cc: stable@...r.kernel.org
> Fixes: 32edc40ae65c ("ieee802154: change _cb handling slightly")
> Signed-off-by: Miquel Raynal <miquel.raynal@...tlin.com>
> ---
> 
> Hello,
> 
> I am surprised the LQI was gone for all those years and nobody
> noticed it, so perhaps I did misinterpret slightly the situation, but I
> am pretty sure the cb area reset was erasing the LQI.
> 
> About the backports, they will likely fail on the older kernels because
> of some function/file moves, but I don't think we really care.
> 
> Cheers,
> Miquèl
> 
>   net/mac802154/rx.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
> index d1f7b8df41fe..a4733a62911f 100644
> --- a/net/mac802154/rx.c
> +++ b/net/mac802154/rx.c
> @@ -134,7 +134,7 @@ static int
>   ieee802154_parse_frame_start(struct sk_buff *skb, struct ieee802154_hdr *hdr)
>   {
>   	int hlen;
> -	struct ieee802154_mac_cb *cb = mac_cb_init(skb);
> +	struct ieee802154_mac_cb *cb = mac_cb(skb);
>   
>   	skb_reset_mac_header(skb);
>   
> @@ -305,8 +305,9 @@ void
>   ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff *skb, u8 lqi)
>   {
>   	struct ieee802154_local *local = hw_to_local(hw);
> +	struct ieee802154_mac_cb *cb = mac_cb_init(skb);
>   
> -	mac_cb(skb)->lqi = lqi;
> +	cb->lqi = lqi;
>   	skb->pkt_type = IEEE802154_RX_MSG;
>   	skb_queue_tail(&local->skb_queue, skb);
>   	tasklet_schedule(&local->tasklet);


This patch has been applied to the wpan tree and will be
part of the next pull request to net. Thanks!

regards
Stefan Schmidt