netdev - Re: ieee80211.h virtual

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

Date: Tue, 25 Jun 2024 09:56:35 +0300
From: Kalle Valo <kvalo@...nel.org>
To: Jeff Johnson <quic_jjohnson@...cinc.com>
Cc: Koen Vandeputte <koen.vandeputte@...ymesh.com>,
  <ath10k@...ts.infradead.org>,  linux-wireless
 <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>,
  <netdev@...r.kernel.org>,  Johannes Berg <johannes@...solutions.net>,
  Kees Cook <keescook@...omium.org>
Subject: Re: ieee80211.h virtual_map splat

Jeff Johnson <quic_jjohnson@...cinc.com> writes:

> On 6/21/2024 1:04 AM, Koen Vandeputte wrote:
>
>> Hi all,
>> 
>> Within OpenWRT, we switched to kernel 6.6 some time ago.
>> 
>> During testing on a WiFi WDS setup (ath10k), I noticed an old standing
>> bug which now prints a lot more data due to the kernel upgrade:
>> 
>> - All WDS stations are connected
>> - The splat occurs
>> - All WDS station seem to go in timeout and disconnect
>> - The behavior is fixed after a reboot
>> 
>> Yes, we use ath10k-ct over here, but this part of the code is
>> identical to upstream ath10k.
>> 
>> The main issue:
>> 
>> memcpy: detected field-spanning write (size 64) of single field
>> "tim->virtual_map" at
>> ../ath10k-ct-smallbuffers/ath10k-ct-2024.03.02~eb3f488a/ath10k-6.7/wmi.c:4043
>> (size 1)
>> 
>> 
>> looks like virtual_map is defined as  "u8 virtual_map[1]", triggering
>> that error within "include/linux/ieee80211.h"
>> 
>> /**
>>  * struct ieee80211_tim_ie - Traffic Indication Map information element
>>  * @dtim_count: DTIM Count
>>  * @dtim_period: DTIM Period
>>  * @bitmap_ctrl: Bitmap Control
>>  * @virtual_map: Partial Virtual Bitmap
>>  *
>>  * This structure represents the payload of the "TIM element" as
>>  * described in IEEE Std 802.11-2020 section 9.4.2.5.
>>  */
>> struct ieee80211_tim_ie {
>>         u8 dtim_count;
>>         u8 dtim_period;
>>         u8 bitmap_ctrl;
>>         /* variable size: 1 - 251 bytes */
>>         u8 virtual_map[1];
>> } __packed;
>> 
>> 
>> According to this page, defining it this way is actually deprecated:
>> https://www.kernel.org/doc/html/latest/process/deprecated.html
>> 
>> What is the correct way to fix this?
>> Converting it to "u8 virtual_map[];"  ?
>
> Adding netdev to the initial message in the thread.
> https://lore.kernel.org/all/CAPh3n83zb1PwFBFijJKChBqY95zzpYh=2iPf8tmh=YTS6e3xPw@mail.gmail.com/
>
> There was some discussion in the thread, with the observation that the splat 
> is fixed by:
> 2ae5c9248e06 ("wifi: mac80211: Use flexible array in struct ieee80211_tim_ie")
>
> Followed by discussion if this should be backported.
>
> Kees said that "netdev [...] maintainers have asked that contributors not 
> include "Cc: stable" tags, as they want to evaluate for themselves whether 
> patches should go to stable or not"

BTW this rule doesn't apply to wireless subsystem. For wireless patches
it's ok to "Cc: stable" in patches or anyone can send a request to
stable maintainers to pick a patch.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches