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: 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

Powered by Openwall GNU/*/Linux Powered by OpenVZ