netdev - Re: [PATCH] wifi: mac80211: Fix ieee80211_convert_to

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <877ccgd7re.fsf@toke.dk>
Date: Fri, 16 Aug 2024 13:31:01 +0200
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Jeff Johnson <quic_jjohnson@...cinc.com>, Johannes Berg
 <johannes@...solutions.net>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo
 Abeni <pabeni@...hat.com>, Michael Braun <michael-dev@...i-braun.de>
Cc: Harsh Kumar Bijlani <hbijlani@....qualcomm.com>, Kalyan Tallapragada
 <ktallapr@....qualcomm.com>, Jyothi Chukkapalli
 <jchukkap@....qualcomm.com>, Anirban Sirkhell <anirban@....qualcomm.com>,
 Johannes Berg <johannes.berg@...el.com>, linux-wireless@...r.kernel.org,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 ath12k@...ts.infradead.org, Jeff Johnson <quic_jjohnson@...cinc.com>
Subject: Re: [PATCH] wifi: mac80211: Fix ieee80211_convert_to_unicast() logic

Jeff Johnson <quic_jjohnson@...cinc.com> writes:

> The current logic in ieee80211_convert_to_unicast() uses skb_clone()
> to obtain an skb for each individual destination of a multicast
> frame, and then updates the destination address in the cloned skb's
> data buffer before placing that skb on the provided queue.
>
> This logic is flawed since skb_clone() shares the same data buffer
> with the original and the cloned skb, and hence each time the
> destination address is updated, it overwrites the previous destination
> address in this shared buffer. As a result, due to the special handing
> of the first valid destination, all of the skbs will eventually be
> sent to that first destination.

Did you actually observe this happen in practice? ieee80211_change_da()
does an skb_ensure_writable() check on the Ethernet header before
writing it, so AFAICT it does not, in fact, overwrite the data of the
original frame.

> Fix this issue by using skb_copy() instead of skb_clone(). This will
> result in a duplicate data buffer being allocated for each
> destination, and hence each skb will be transmitted to the proper
> destination.

Cf the above, it seems this change will just lead to more needless
copying.

-Toke