| 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
| ||
|
Message-ID:
<TYZPR01MB555633BF44D21481AE70FB95C9182@TYZPR01MB5556.apcprd01.prod.exchangelabs.com>
Date: Fri, 3 May 2024 00:24:13 +0800
From: Ziyang Huang <hzyitc@...look.com>
To: Jeff Johnson <quic_jjohnson@...cinc.com>
Cc: kvalo@...nel.org, jjohnson@...nel.org, Larry.Finger@...inger.net,
linux-wireless@...r.kernel.org, ath11k@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] wifi: ath11k: fix remapped ce accessing issue on 64bit OS
> On 5/1/2024 9:14 AM, Ziyang Huang wrote:
>> On 64bit OS, when ab->mem_ce is lower than or 4G far away from ab->mem,
>> u32 is not enough to store the offsets, which makes ath11k_ahb_read32()
>> and ath11k_ahb_write32() access incorrect address and causes Data Abort
>> Exception.
>
> Are you actually observing this issue?
> Or is this a hypothetical situation?
Yes, here is the log:
[ 14.896160] ath11k c000000.wifi: ipq5018 hw1.0
[ 14.896210] ath11k c000000.wifi: FW memory mode: 2
[ 14.899576] ath11k c000000.wifi: ath11k_hal_srng_create_config:
ab->mem=0xffffffc088000000
[ 14.904290] ath11k c000000.wifi: ath11k_hal_srng_create_config:
ab->mem_ce=0xffffffc082800000
[ 14.912593] ath11k c000000.wifi: ath11k_hal_srng_create_config:
HAL_SEQ_WCSS_UMAC_CE0_SRC_REG(ab)=0x00000000
[ 14.921247] ath11k c000000.wifi: ath11k_hal_srng_create_config:
HAL_CE_DST_RING_BASE_LSB=0x00000000
[ 14.931115] ath11k c000000.wifi: ath11k_hal_srng_create_config:
ATH11K_CE_OFFSET(ab)=0xfffffffffa800000
[ 14.939863] ath11k c000000.wifi: ath11k_hal_srng_create_config:
s->reg_start[0]=0xfa800000
...
[ 15.155895] ath11k c000000.wifi: chip_id 0x0 chip_family 0x4
board_id 0x10 soc_id 0xffffffff
[ 15.155954] ath11k c000000.wifi: fw_version 0x270206d0
fw_build_timestamp 2022-08-04 13:28 fw_build_id
WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
[ 15.292858] ath11k c000000.wifi: ath11k_hal_srng_src_hw_init:
reg_base=0xfa800000
[ 15.292938] ath11k c000000.wifi: ath11k_ahb_write32:
ab->mem=0xffffffc088000000
[ 15.299549] ath11k c000000.wifi: ath11k_ahb_write32:
offset=0xfa800000
[ 15.306525] ath11k c000000.wifi: ath11k_ahb_write32:
addr=0xffffffc182800000
[ 15.313088] Unable to handle kernel paging request at virtual
address ffffffc182800000
[ 15.320309] Mem abort info:
[ 15.328023] ESR = 0x0000000096000045
[ 15.330691] EC = 0x25: DABT (current EL), IL = 32 bits
[ 15.334512] SET = 0, FnV = 0
[ 15.340030] EA = 0, S1PTW = 0
[ 15.342843] FSC = 0x05: level 1 translation fault
[ 15.345900] Data abort info:
[ 15.350741] ISV = 0, ISS = 0x00000045, ISS2 = 0x00000000
[ 15.353868] CM = 0, WnR = 1, TnD = 0, TagAccess = 0
[ 15.359187] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 15.364286] swapper pgtable: 4k pages, 39-bit VAs,
pgdp=0000000041a22000
[ 15.369685] [ffffffc182800000] pgd=0000000000000000,
p4d=0000000000000000, pud=0000000000000000
[ 15.376369] Internal error: Oops: 0000000096000045 [#1] SMP
[ 15.384771] Modules linked in: pppoe ppp_async nft_fib_inet
nf_flow_table_inet ath11k_pci(O) ath11k_ahb(O) ath11k(O) pppox
ppp_generic nft_reject_ipv6 nft_reject_ipv4 nft_reject_inet nft_reject
nft_redir nft_quota nft_numgen nft_nat nft_masq nft_log nft_limit
nft_hash nft_flow_offload nft_fib_ipv6 nft_fib_ipv4 nft_fib nft_ct
nft_chain_nat nf_tables nf_nat nf_flow_table nf_conntrack mac80211(O)
cfg80211(O) slhc qrtr_smd qrtr_mhi qrtr qmi_helpers(O) nfnetlink
nf_reject_ipv6 nf_reject_ipv4 nf_log_syslog nf_defrag_ipv6
nf_defrag_ipv4 mhi mdio_netlink(O) libcrc32c hwmon crc_ccitt compat(O)
sha512_generic sha512_arm64 seqiv sha3_generic jitterentropy_rng drbg
michael_mic hmac geniv cmac leds_gpio xhci_plat_hcd xhci_pci xhci_hcd
dwc3 dwc3_qcom qca_nss_dp(O) qca_ssdk(O) gpio_button_hotplug(O) ext4
mbcache jbd2 crc32c_generic
[ 15.440670] CPU: 1 PID: 127 Comm: kworker/u4:4 Tainted:
G O 6.6.28 #0
[ 15.462900] Hardware name: Redmi AX3000 (DT)
[ 15.470623] Workqueue: ath11k_qmi_driver_event
ath11k_qmi_deinit_service [ath11k]
[ 15.474965] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT
-SSBS BTYPE=--)
[ 15.482342] pc : 0xffffffc0796774f8
[ 15.489108] lr : 0xffffffc0796774ec
[ 15.492581] sp : ffffffc08256bb60
[ 15.496052] x29: ffffffc08256bb60 x28: ffffff8003ec3200 x27:
ffffff8003ec2400
[ 15.499530] x26: ffffff8003ed0000 x25: ffffff8003ec1200 x24:
0000000000000020
[ 15.506647] x23: ffffff8003ec3760 x22: 0000000042c4f000 x21:
ffffffc0796b0048
[ 15.513766] x20: ffffff8003ec0000 x19: 00000000fa800000 x18:
00000000000000c7
[ 15.520883] x17: 3030303838306366 x16: 666666666678303d x15:
ffffffc081356e20
[ 15.528001] x14: 0000000000000255 x13: 00000000000000c7 x12:
00000000ffffffea
[ 15.535119] x11: 00000000ffffefff x10: ffffffc0813aee20 x9 :
ffffffc081356dc8
[ 15.542237] x8 : 0000000000017fe8 x7 : c0000000ffffefff x6 :
0000000000057fa8
[ 15.549356] x5 : 0000000000000fff x4 : 0000000000000000 x3 :
0000000000000000
[ 15.556474] x2 : 0000000000000000 x1 : ffffff80036b6c00 x0 :
ffffffc182800000
[ 15.563593] Call trace:
[ 15.570707] 0xffffffc0796774f8
[ 15.572963] ath11k_hal_srng_setup+0x5a0/0x89c [ath11k]
[ 15.576090] ath11k_ce_get_attr_flags+0xb4/0x270 [ath11k]
[ 15.581299] ath11k_ce_init_pipes+0x4c/0x19c [ath11k]
[ 15.586854] ath11k_core_qmi_firmware_ready+0x3c/0x580 [ath11k]
[ 15.591889] ath11k_qmi_deinit_service+0x126c/0x1d80 [ath11k]
[ 15.597618] process_one_work+0x158/0x2a8
[ 15.603519] worker_thread+0x2ac/0x48c
[ 15.607512] kthread+0xdc/0xe0
[ 15.611156] ret_from_fork+0x10/0x20
[ 15.614203] Code: 940a315e f94e2680 8b130000 d50332bf (b9000016)
[ 15.617933] ---[ end trace 0000000000000000 ]---
[ 15.623922] Kernel panic - not syncing: Oops: Fatal exception
[ 15.628610] SMP: stopping secondary CPUs
[ 15.834290] Kernel Offset: disabled
[ 15.834310] CPU features: 0x0,00000000,10000000,0000400b
[ 15.836573] Memory Limit: none
[ 15.842128] Rebooting in 1 seconds..
>> Let's use the high bits of offsets to decide where to access, which is
>> similar as ath11k_pci_get_window_start() done. In the future, we can
merge
>> these functions for unified regs accessing.
>
> Performing unnecessary tests and masking for every ioread/write
operation will
> potentially impact performance.
But this is what __ath11k_pcic_write32(), ath11k_pci_window_write32() and
ath11k_pci_get_window_start() doing. Here are the codes:
static u32 ath11k_pci_get_window_start(struct ath11k_base *ab,
u32 offset)
{
if (!ab->hw_params.static_window_map)
return ATH11K_PCI_WINDOW_START;
if ((offset ^ HAL_SEQ_WCSS_UMAC_OFFSET) <
ATH11K_PCI_WINDOW_RANGE_MASK)
/* if offset lies within DP register range, use
3rd window */
return 3 * ATH11K_PCI_WINDOW_START;
else if ((offset ^ HAL_SEQ_WCSS_UMAC_CE0_SRC_REG(ab)) <
ATH11K_PCI_WINDOW_RANGE_MASK)
/* if offset lies within CE register range, use
2nd window */
return 2 * ATH11K_PCI_WINDOW_START;
else
return ATH11K_PCI_WINDOW_START;
}
> What other fixes were considered (i.e. did you consider making all the
> register addresses u64?)
I also want to simplify the PCI register accesses. For dynamic window,
ATH11K_PCI_WINDOW_VALUE_MASK is used to decide window. But for
static window, we need to use these separated helper functions:
ath11k_pci_get_window_start() and ath11k_ahb_get_window_start_wcn6750().
What they do are:
1. decide window index, which can be store in middle bits of offset,
just as
ATH11K_PCI_WINDOW_VALUE_MASK doing.
2. for dynamic window, ath11k_pci_select_window() is need to switch
window.
for static window, just directly write/read (index *
ATH11K_PCI_WINDOW_START).
This is what commit 867f4eeee862 ("wifi: ath11k: Fix register
write failure on
QCN9074") talk about. But ab->hw_params.static_window_map can be used
to identify them. So it doesn't matter.
With all of these, we don't need any other remap function and be able to
store all
remap informations in {ipq,qca,qcn}xxx_regs.
> ...
> > + default:
> > + BUG();
>
> you can WARN but you can't BUG (and even WARN is being discouraged)
Ok.
> ...
> > + default:
> > + BUG();
>
> ditto
Ok.
> ...
Powered by blists - more mailing lists