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 for Android: free password hash cracker in your pocket
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Mon, 24 Jun 2024 20:44:11 -0700
From: Jeff Johnson <quic_jjohnson@...cinc.com>
To: 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>
CC: <netdev@...r.kernel.org>, Johannes Berg <johannes@...solutions.net>,
        Kees
 Cook <keescook@...omium.org>
Subject: Re: ieee80211.h virtual_map splat

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"

So the purpose of this message is to notify the netdev maintainers that this 
issue has been observed on a LTS kernel with the popular OpenWrt package so 
that the maintainers can make a backporting decision.

/jeff


> [29009.581820] ------------[ cut here ]------------
> [29009.581898] WARNING: CPU: 0 PID: 0 at
> ../ath10k-ct-smallbuffers/ath10k-ct-2024.03.02~eb3f488a/ath10k-6.7/wmi.c:4043
> ath10k_wmi_event_host_swba+0x7c4/0x824 [ath10k_core]
> [29009.585574] 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)
> [29009.712626]  unwind_backtrace from show_stack+0x10/0x14
> [29009.717217]  show_stack from dump_stack_lvl+0x40/0x4c
> [29009.722337]  dump_stack_lvl from __warn+0x94/0xbc
> [29009.727546]  __warn from warn_slowpath_fmt+0xf8/0x15c
> [29009.732233]  warn_slowpath_fmt from
> ath10k_wmi_event_host_swba+0x7c4/0x824 [ath10k_core]
> [29009.737309]  ath10k_wmi_event_host_swba [ath10k_core] from
> ath10k_wmi_10_4_op_rx+0x444/0x6a4 [ath10k_core]
> [29009.745437]  ath10k_wmi_10_4_op_rx [ath10k_core] from
> ath10k_htc_rx_completion_handler+0xa8/0x210 [ath10k_core]
> [29009.754899]  ath10k_htc_rx_completion_handler [ath10k_core] from
> ath10k_pci_fw_dump_work+0xf28/0xf94 [ath10k_pci]
> [29009.764894]  ath10k_pci_fw_dump_work [ath10k_pci] from
> ath10k_ce_per_engine_service+0x64/0x84 [ath10k_core]
> [29009.775299]  ath10k_ce_per_engine_service [ath10k_core] from
> ath10k_ce_per_engine_service_any+0x74/0x194 [ath10k_core]
> [29009.784848]  ath10k_ce_per_engine_service_any [ath10k_core] from
> ath10k_pci_napi_poll+0x44/0x138 [ath10k_pci]
> [29009.795611]  ath10k_pci_napi_poll [ath10k_pci] from
> __napi_poll.constprop.0+0x2c/0x180
> [29009.805589]  __napi_poll.constprop.0 from net_rx_action+0x140/0x2e8
> [29009.813400]  net_rx_action from __do_softirq+0x100/0x270
> [29009.819561]  __do_softirq from irq_exit+0x88/0xb4
> [29009.825117]  irq_exit from call_with_stack+0x18/0x20
> [29009.829715]  call_with_stack from __irq_svc+0x80/0x98
> [29009.834751] Exception stack(0xc0d01f28 to 0xc0d01f70)
> [29009.839706] 1f20:                   00000003 00000001 1d2e2e44
> 40000000 00000000 c0d04f68
> [29009.844745] 1f40: c0d084c0 c0d04fa0 00000000 00000000 c0d04f08
> 00000000 0000001f c0d01f78
> [29009.852898] 1f60: c09deaf8 c09df260 60000013 ffffffff
> [29009.861055]  __irq_svc from default_idle_call+0x2c/0x30
> [29009.866089]  default_idle_call from do_idle+0x1d8/0x228
> [29009.871124]  do_idle from cpu_startup_entry+0x28/0x2c
> [29009.876328]  cpu_startup_entry from kernel_init+0x0/0x12c
> [29009.881537]  kernel_init from arch_post_acpi_subsys_init+0x0/0x8
> [29009.886973] ---[ end trace 0000000000000000 ]---