On 19/08/2024 20:19, Haoyu Li wrote:
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@xxxxxxxxxxxxxxxxxxx/. Please kindly correct us if we missed any key information. Looking forward to your response!
You are quite right that the case when (wdev->conn->params.channel != NULL) should initialize n_channels to 1 first. The other question is if it's legal to take address beyond the end of array. I'm talking about request->ssids = (void *)&request->channels[n_channels]; But you can easily check it yourself with pretty simple program.
Best, Haoyu Li
