linux-kernel - Re: [PATCH v2 ath-next 2/2] wifi: ath11k: fix HTC rx insufficient length

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]

Message-ID: <Z9PjjDFBuSJ7exVj@hovoldconsulting.com>
Date: Fri, 14 Mar 2025 09:06:36 +0100
From: Johan Hovold <johan@...nel.org>
To: Miaoqing Pan <quic_miaoqing@...cinc.com>
Cc: Jeff Johnson <jeff.johnson@....qualcomm.com>,
	ath11k@...ts.infradead.org, linux-wireless@...r.kernel.org,
	linux-kernel@...r.kernel.org, johan+linaro@...nel.org
Subject: Re: [PATCH v2 ath-next 2/2] wifi: ath11k: fix HTC rx insufficient
 length

On Fri, Mar 14, 2025 at 09:01:36AM +0800, Miaoqing Pan wrote:
> On 3/14/2025 12:14 AM, Johan Hovold wrote:
> > On Thu, Mar 13, 2025 at 09:31:56PM +0800, Miaoqing Pan wrote:
> >> On 3/13/2025 12:43 AM, Johan Hovold wrote:

> >>> +	/* Make sure descriptor is read after the head pointer. */
> >>> +	dma_rmb();
> >>> +
> >>>    	*nbytes = ath11k_hal_ce_dst_status_get_length(desc);
> >>>    	if (*nbytes == 0) {
> >>> +		WARN_ON_ONCE(1);	// FIXME: remove
> >>>    		ret = -EIO;
> >>>    		goto err;
> >>>    	}
> >>
> >> This issue can still be reproduced.
> >>
> >> [ 3283.687469] WARNING: CPU: 0 PID: 0 at
> >> /drivers/net/wireless/ath/ath11k/ce.c:405
> >> ath11k_ce_per_engine_service+0x228/0x3e4 [ath11k]
> > 
> > Thanks for verifying.
> > 
> > Which platform are you testing on and which kernel are you using?
> > 
> > I'm still waiting to hear back from some people testing my patch on the
> > X13s (sc8280xp).

> qcs615-ride, kernel 6.6.65.

Ok, so a downstream vendor kernel?

qcs615-ride does not even have PCIe enabled in mainline yet, but I
assume that's what you use here?
 
> I think the hardware has already ensured synchronization between 
> descriptor and head pointer, which isn't difficult to achieve. The issue 
> is likely caused by something else and requires further debugging.

Yeah, but you still need memory barriers on the kernel side.

It could be that we are looking at two different causes for those
zero-length descriptors.

The error handling for that obviously needs to be fixed either way, but
I haven't heard anyone hitting the corruption with the memory barriers
in place on the X13s yet (even if we'd need some more time to test
this).

Johan