[PATCH ath-next] wifi: ath12k: fix stale skb pointers after aligned TX payload shift

[PATCH ath-next] wifi: ath12k: fix stale skb pointers after aligned TX payload shift

[Baochen Qiang]

ath12k_wifi7_dp_tx() caches hdr, eth, and skb_cb from the skb before
calling ath12k_dp_tx_align_payload(). That function may shift skb->data
in place (when headroom or tailroom is sufficient) or reallocate the
buffer entirely via skb_realloc_headroom(), freeing the original skb.
In either case hdr, eth, and skb_cb are left pointing into stale memory.

After alignment, only hdr is refreshed, leaving eth and skb_cb stale.
skb_cb is written immediately after (storing DMA addresses), and eth is
re-read on every TCL ring retry via the tcl_ring_sel goto, so both
accesses are use-after-free or stale-pointer bugs depending on which
alignment path was taken.

Refresh eth (conditionally, to preserve the encap-mode distinction) and
skb_cb alongside hdr after ath12k_dp_tx_align_payload() returns, so all
three point into the live skb for all subsequent accesses.

Issue found during code review, compile tested only.

Fixes: 38055789d151 ("wifi: ath12k: use 128 bytes aligned iova in transmit path for WCN7850")
Signed-off-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c b/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
index d2749de44553..6b8430260238 100644
--- a/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
+++ b/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
@@ -251,10 +251,15 @@ int ath12k_wifi7_dp_tx(struct ath12k_pdev_dp *dp_pdev, struct ath12k_link_vif *a
 			goto map;
 		}
 
-		/* hdr is pointing to a wrong place after alignment,
-		 * so refresh it for later use.
+		/*
+		 * The payload may have been shifted or even the entire buffer may have
+		 * been reallocated for alignment. In that case, hdr, eth and skb_cb
+		 * are stale pointers. Refresh them now for later dereference.
 		 */
 		hdr = (void *)skb->data;
+		if (eth)
+			eth = (struct ethhdr *)skb->data;
+		skb_cb = ATH12K_SKB_CB(skb);
 	}
 map:
 	ti.paddr = dma_map_single(dp->dev, skb->data, skb->len, DMA_TO_DEVICE);

---
base-commit: 4fa10e991f77b4c929d1959900a6ed422b9e2ac5
change-id: 20260811-ath12k-uaf-for-aligned-tx-a068d34b1548

Best regards,
-- 
Baochen Qiang <baochen.qiang@oss.qualcomm.com>

Re: [PATCH ath-next] wifi: ath12k: fix stale skb pointers after aligned TX payload shift

[Rameshkumar Sundaram]

On 8/18/2026 7:14 AM, Baochen Qiang wrote:
> ath12k_wifi7_dp_tx() caches hdr, eth, and skb_cb from the skb before
> calling ath12k_dp_tx_align_payload(). That function may shift skb->data
> in place (when headroom or tailroom is sufficient) or reallocate the
> buffer entirely via skb_realloc_headroom(), freeing the original skb.
> In either case hdr, eth, and skb_cb are left pointing into stale memory.
> 
> After alignment, only hdr is refreshed, leaving eth and skb_cb stale.
> skb_cb is written immediately after (storing DMA addresses), and eth is
> re-read on every TCL ring retry via the tcl_ring_sel goto, so both
> accesses are use-after-free or stale-pointer bugs depending on which
> alignment path was taken.
> 
> Refresh eth (conditionally, to preserve the encap-mode distinction) and
> skb_cb alongside hdr after ath12k_dp_tx_align_payload() returns, so all
> three point into the live skb for all subsequent accesses.
> 
> Issue found during code review, compile tested only.
> 
> Fixes: 38055789d151 ("wifi: ath12k: use 128 bytes aligned iova in transmit path for WCN7850")
> Signed-off-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>

Re: [PATCH ath-next] wifi: ath12k: fix stale skb pointers after aligned TX payload shift

[Jeff Johnson]

On 8/17/2026 6:44 PM, Baochen Qiang wrote:
> ath12k_wifi7_dp_tx() caches hdr, eth, and skb_cb from the skb before
> calling ath12k_dp_tx_align_payload(). That function may shift skb->data
> in place (when headroom or tailroom is sufficient) or reallocate the
> buffer entirely via skb_realloc_headroom(), freeing the original skb.
> In either case hdr, eth, and skb_cb are left pointing into stale memory.
> 
> After alignment, only hdr is refreshed, leaving eth and skb_cb stale.
> skb_cb is written immediately after (storing DMA addresses), and eth is
> re-read on every TCL ring retry via the tcl_ring_sel goto, so both
> accesses are use-after-free or stale-pointer bugs depending on which
> alignment path was taken.
> 
> Refresh eth (conditionally, to preserve the encap-mode distinction) and
> skb_cb alongside hdr after ath12k_dp_tx_align_payload() returns, so all
> three point into the live skb for all subsequent accesses.
> 
> Issue found during code review, compile tested only.
> 
> Fixes: 38055789d151 ("wifi: ath12k: use 128 bytes aligned iova in transmit path for WCN7850")
> Signed-off-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c b/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
> index d2749de44553..6b8430260238 100644
> --- a/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
> +++ b/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
> @@ -251,10 +251,15 @@ int ath12k_wifi7_dp_tx(struct ath12k_pdev_dp *dp_pdev, struct ath12k_link_vif *a
>  			goto map;
>  		}
>  
> -		/* hdr is pointing to a wrong place after alignment,
> -		 * so refresh it for later use.
> +		/*
> +		 * The payload may have been shifted or even the entire buffer may have
> +		 * been reallocated for alignment. In that case, hdr, eth and skb_cb
> +		 * are stale pointers. Refresh them now for later dereference.
>  		 */
>  		hdr = (void *)skb->data;
> +		if (eth)
> +			eth = (struct ethhdr *)skb->data;
> +		skb_cb = ATH12K_SKB_CB(skb);

My review agent notes there is an additional issue possible if alignment
causes a new skb to be allocated. If there are any error returns beyond this
point then the caller will double free the original skb instead of freeing the
new skb. this is because the caller doesn't know the original skb was replaced.

So I'm taking this patch as-is since it fixes issues when the buffer is
shifted, but we need an additional fix to correctly handle when the original
skb is freed and there is a subsequent error return.

>  	}
>  map:
>  	ti.paddr = dma_map_single(dp->dev, skb->data, skb->len, DMA_TO_DEVICE);
> 
> ---
> base-commit: 4fa10e991f77b4c929d1959900a6ed422b9e2ac5
> change-id: 20260811-ath12k-uaf-for-aligned-tx-a068d34b1548
> 
> Best regards,

Re: [PATCH ath-next] wifi: ath12k: fix stale skb pointers after aligned TX payload shift

[Baochen Qiang]

On 9/9/2026 4:55 AM, Jeff Johnson wrote:
> On 8/17/2026 6:44 PM, Baochen Qiang wrote:
>> ath12k_wifi7_dp_tx() caches hdr, eth, and skb_cb from the skb before
>> calling ath12k_dp_tx_align_payload(). That function may shift skb->data
>> in place (when headroom or tailroom is sufficient) or reallocate the
>> buffer entirely via skb_realloc_headroom(), freeing the original skb.
>> In either case hdr, eth, and skb_cb are left pointing into stale memory.
>>
>> After alignment, only hdr is refreshed, leaving eth and skb_cb stale.
>> skb_cb is written immediately after (storing DMA addresses), and eth is
>> re-read on every TCL ring retry via the tcl_ring_sel goto, so both
>> accesses are use-after-free or stale-pointer bugs depending on which
>> alignment path was taken.
>>
>> Refresh eth (conditionally, to preserve the encap-mode distinction) and
>> skb_cb alongside hdr after ath12k_dp_tx_align_payload() returns, so all
>> three point into the live skb for all subsequent accesses.
>>
>> Issue found during code review, compile tested only.
>>
>> Fixes: 38055789d151 ("wifi: ath12k: use 128 bytes aligned iova in transmit path for WCN7850")
>> Signed-off-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
>> ---
>>  drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c b/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
>> index d2749de44553..6b8430260238 100644
>> --- a/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
>> +++ b/drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c
>> @@ -251,10 +251,15 @@ int ath12k_wifi7_dp_tx(struct ath12k_pdev_dp *dp_pdev, struct ath12k_link_vif *a
>>  			goto map;
>>  		}
>>  
>> -		/* hdr is pointing to a wrong place after alignment,
>> -		 * so refresh it for later use.
>> +		/*
>> +		 * The payload may have been shifted or even the entire buffer may have
>> +		 * been reallocated for alignment. In that case, hdr, eth and skb_cb
>> +		 * are stale pointers. Refresh them now for later dereference.
>>  		 */
>>  		hdr = (void *)skb->data;
>> +		if (eth)
>> +			eth = (struct ethhdr *)skb->data;
>> +		skb_cb = ATH12K_SKB_CB(skb);
> 
> My review agent notes there is an additional issue possible if alignment
> causes a new skb to be allocated. If there are any error returns beyond this
> point then the caller will double free the original skb instead of freeing the
> new skb. this is because the caller doesn't know the original skb was replaced.

Yeah, indeed the issue is true. I guess we need to pass &skb instead to
ath12k_wifi7_dp_tx() and replace it with the new one.

> 
> So I'm taking this patch as-is since it fixes issues when the buffer is
> shifted, but we need an additional fix to correctly handle when the original
> skb is freed and there is a subsequent error return.

agree, it is a different issue hence deserves a separate patch. Do you want me to submit
it or you will do it yourself?

> 
>>  	}
>>  map:
>>  	ti.paddr = dma_map_single(dp->dev, skb->data, skb->len, DMA_TO_DEVICE);
>>
>> ---
>> base-commit: 4fa10e991f77b4c929d1959900a6ed422b9e2ac5
>> change-id: 20260811-ath12k-uaf-for-aligned-tx-a068d34b1548
>>
>> Best regards,
> 

Re: [PATCH ath-next] wifi: ath12k: fix stale skb pointers after aligned TX payload shift

[Jeff Johnson]

On 9/8/2026 8:45 PM, Baochen Qiang wrote:
> On 9/9/2026 4:55 AM, Jeff Johnson wrote:
>> On 8/17/2026 6:44 PM, Baochen Qiang wrote:
>> So I'm taking this patch as-is since it fixes issues when the buffer is
>> shifted, but we need an additional fix to correctly handle when the original
>> skb is freed and there is a subsequent error return.
> 
> agree, it is a different issue hence deserves a separate patch. Do you want me to submit
> it or you will do it yourself?

I'd like you to do so. I'm buried.

Re: [PATCH ath-next] wifi: ath12k: fix stale skb pointers after aligned TX payload shift

[Jeff Johnson]

On Tue, 18 Aug 2026 09:44:48 +0800, Baochen Qiang wrote:
> ath12k_wifi7_dp_tx() caches hdr, eth, and skb_cb from the skb before
> calling ath12k_dp_tx_align_payload(). That function may shift skb->data
> in place (when headroom or tailroom is sufficient) or reallocate the
> buffer entirely via skb_realloc_headroom(), freeing the original skb.
> In either case hdr, eth, and skb_cb are left pointing into stale memory.
> 
> After alignment, only hdr is refreshed, leaving eth and skb_cb stale.
> skb_cb is written immediately after (storing DMA addresses), and eth is
> re-read on every TCL ring retry via the tcl_ring_sel goto, so both
> accesses are use-after-free or stale-pointer bugs depending on which
> alignment path was taken.
> 
> [...]

Applied, thanks!

[1/1] wifi: ath12k: fix stale skb pointers after aligned TX payload shift
      commit: 6c40719489c8d799798989b15bc07b0700c132f8

Best regards,
-- 
Jeff Johnson <jeff.johnson@oss.qualcomm.com>