Search Linux Wireless

Re: [net/wireless] Question about `cfg80211_conn_scan` func: misuse of __counted_by

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux