Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Adi Prasan]

Hi Dan,

Went and checked for the things you suggested.

Fixes tag: git blame shows this check hasn't been touched since the
original import, 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi
driver"). Added that in v2.

On MAX_BSSINFO_LEN: I couldn't find any rationale for 1000 anywhere
in the history - it's exactly as it was in the 2017 import, no
comment, no commit explaining it. Header (24) + MAX_IE_SZ (768) =
792, so there's already ~200 bytes of slack in the allocation beyond
what ies[] can actually hold. Looks like an arbitrary/conservative
number carried over from wherever this was ported from, not derived
from any struct size in this tree. My patch doesn't touch the
allocation, just tightens the check to match what ies[] can hold.

On the timestamp write - I don't think it's corrupting IE data,
though I get why it looks that way. network.ies[] isn't a pure IE
list despite the name - its declaration comment says "timestamp,
beacon interval, and capability information", and collect_bss_info()
confirms it: it memcpy's straight from the raw frame body right
after the header, so ies[0:8] is the captured TSF, ies[8:10] is
beacon_interval, ies[10:12] is capab_info, and actual variable IEs
start at offset 12 (matches _FIXED_IE_LENGTH_ used elsewhere in this
file). So the memcpy() followed by the timestamp write isn't
scribbling an IE entry - it's replacing the captured TSF (bytes 0-7)
with notify_timestamp = ktime_to_us(ktime_get_boottime()), while
beacon_interval/capab_info/IEs from the original capture stay
untouched. Order doesn't affect the result since it's the same 8
bytes either way.

That said, I'm not certain cfg80211 is fine getting a local boottime
value here instead of the AP's real TSF - if that's actually wrong
I'd like to understand why, I don't have full context on what
cfg80211_inform_bss_frame does with that field internally.

Thanks,
Adi