netdev - [net/mac80211] Question about `ieee80211_copy_mbssid_beacon` func: misuse of __counted

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

Message-ID: <CAPbMC75nfHS=rV3XJH8Pkc_q_A3UmsCrdTs4DL_0s=Mb6W0N4Q@mail.gmail.com>
Date: Mon, 19 Aug 2024 13:59:41 -0700
From: Haoyu Li <lihaoyu499@...il.com>
To: Johannes Berg <johannes@...solutions.net>, linux-wireless@...r.kernel.org, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [net/mac80211] Question about `ieee80211_copy_mbssid_beacon` func:
 misuse of __counted_by

Dear Linux Developers for NETWORKING and MAC80211,

We are curious about the use of `struct cfg80211_mbssid_elems *dst` in
function `ieee80211_copy_mbssid_beacon`.
The definition of `struct cfg80211_mbssid_elems` is at
https://elixir.bootlin.com/linux/v6.10.6/source/include/net/cfg80211.h#L1304.
```
struct cfg80211_mbssid_elems {
u8 cnt;
struct {
const u8 *data;
size_t len;
} elem[] __counted_by(cnt);
};
```

Our question is: The `elem` member of `struct cfg80211_mbssid_elems`
is annotated with "__counted_by", which means the array size is
indicated by `cnt`. Only if we set `cnt` before accessing `elem[i]`,
the flexible member `elem` can be properly bounds-checked at run-time
when enabling CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE. Or there
will be a warning from each array access that is prior to the
initialization because the number of elements is zero.

In function `ieee80211_copy_mbssid_beacon` at
https://elixir.bootlin.com/linux/v6.10.6/source/net/mac80211/cfg.c#L1073,
we think it's needed to relocate `dst->cnt = src->cnt` before
accessing `dst->elem[i]`.
And https://elixir.bootlin.com/linux/v6.10.6/source/net/mac80211/cfg.c#L1090
is the same.

Here is a fix example of a similar situation :
https://lore.kernel.org/stable/20240613113225.898955993@linuxfoundation.org/.

Please kindly correct us if we missed any key information. Looking
forward to your response!

Best,
Haoyu Li