netdev - [net/wireless] Question about `cfg80211_conn_scan` 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>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPbMC760=5UeaU2wwNZkBMi2ZMVhr2GQgG+VkM8Z7zNbt-FtTA@mail.gmail.com>
Date: Mon, 19 Aug 2024 12:19:07 -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/wireless] Question about `cfg80211_conn_scan` func: misuse of __counted_by

Dear Linux Developers for NETWORKING and CFG80211/NL80211,

We are curious about the use of `struct cfg80211_scan_request *request`
in function `cfg80211_conn_scan`.
The definition of `struct cfg80211_scan_request` is at
https://elixir.bootlin.com/linux/v6.10.6/source/include/net/cfg80211.h#L2675.
```
struct cfg80211_scan_request {
struct cfg80211_ssid *ssids;
int n_ssids;
u32 n_channels;
const u8 *ie;
size_t ie_len;
u16 duration;
bool duration_mandatory;
u32 flags;

u32 rates[NUM_NL80211_BANDS];

struct wireless_dev *wdev;

u8 mac_addr[ETH_ALEN] __aligned(2);
u8 mac_addr_mask[ETH_ALEN] __aligned(2);
u8 bssid[ETH_ALEN] __aligned(2);

/* internal */
struct wiphy *wiphy;
unsigned long scan_start;
struct cfg80211_scan_info info;
bool notified;
bool no_cck;
bool scan_6ghz;
u32 n_6ghz_params;
struct cfg80211_scan_6ghz_params *scan_6ghz_params;
s8 tsf_report_link_id;

/* keep last */
struct ieee80211_channel *channels[] __counted_by(n_channels);
};
```

Our question is: The `channels` member of `struct
cfg80211_scan_request` is annotated
with "__counted_by", which means the array size is indicated by
`n_channels`. Only if we set `n_channels` before accessing
`channels[i]`, the flexible
member `hws` 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 `cfg80211_conn_scan` at
https://elixir.bootlin.com/linux/v6.10.6/source/net/wireless/sme.c#L117,
we think it's needed to relocate `request->n_channels = n_channels` before
accessing `request->channels[...]`.

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