2023-04-03 18:29:35

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH 0/3] wifi: ath11k: fix double free of peer rx_tid

During stability testing, double free crash was seen while handling
reo flush command. To handle the same, this patch series includes
changes to address the double free and to avoid the issue case

Harshitha Prem (3):
wifi: ath11k: fix double free of peer rx_tid during reo cmd failure
wifi: ath11k: Prevent REO cmd failures
wifi: ath11k: add peer mac information in failure cases

drivers/net/wireless/ath/ath11k/dp.h | 2 +-
drivers/net/wireless/ath/ath11k/dp_rx.c | 59 +++++++++++++++++--------
drivers/net/wireless/ath/ath11k/hw.c | 1 +
3 files changed, 43 insertions(+), 19 deletions(-)


base-commit: bea046575a2e6d7d1cf63cc7ab032647a3585de5
--
2.17.1


2023-04-03 18:29:47

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH 1/3] wifi: ath11k: fix double free of peer rx_tid during reo cmd failure

Peer rx_tid is locally copied thrice during peer_rx_tid_cleanup to
send REO_CMD_UPDATE_RX_QUEUE followed by REO_CMD_FLUSH_CACHE to flush
all aged REO descriptors from HW cache.

When sending REO_CMD_FLUSH_CACHE fails, we do dma unmap of already
mapped rx_tid->vaddr and free it. This is not checked during
reo_cmd_list_cleanup() and dp_reo_cmd_free() before trying to free and
unmap again.

Fix this by setting rx_tid->vaddr NULL in rx tid delete and also
wherever freeing it to check in reo_cmd_list_cleanup() and
reo_cmd_free() before trying to free again.

Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1

Signed-off-by: Sathishkumar Muruganandam <[email protected]>
Signed-off-by: Harshitha Prem <[email protected]>
---
drivers/net/wireless/ath/ath11k/dp_rx.c | 43 ++++++++++++++++++-------
1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 99859b59138e..e2320109dac0 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -668,13 +668,18 @@ void ath11k_dp_reo_cmd_list_cleanup(struct ath11k_base *ab)
struct ath11k_dp *dp = &ab->dp;
struct dp_reo_cmd *cmd, *tmp;
struct dp_reo_cache_flush_elem *cmd_cache, *tmp_cache;
+ struct dp_rx_tid *rx_tid;

spin_lock_bh(&dp->reo_cmd_lock);
list_for_each_entry_safe(cmd, tmp, &dp->reo_cmd_list, list) {
list_del(&cmd->list);
- dma_unmap_single(ab->dev, cmd->data.paddr,
- cmd->data.size, DMA_BIDIRECTIONAL);
- kfree(cmd->data.vaddr);
+ rx_tid = &cmd->data;
+ if (rx_tid->vaddr) {
+ dma_unmap_single(ab->dev, rx_tid->paddr,
+ rx_tid->size, DMA_BIDIRECTIONAL);
+ kfree(rx_tid->vaddr);
+ rx_tid->vaddr = NULL;
+ }
kfree(cmd);
}

@@ -682,9 +687,13 @@ void ath11k_dp_reo_cmd_list_cleanup(struct ath11k_base *ab)
&dp->reo_cmd_cache_flush_list, list) {
list_del(&cmd_cache->list);
dp->reo_cmd_cache_flush_count--;
- dma_unmap_single(ab->dev, cmd_cache->data.paddr,
- cmd_cache->data.size, DMA_BIDIRECTIONAL);
- kfree(cmd_cache->data.vaddr);
+ rx_tid = &cmd_cache->data;
+ if (rx_tid->vaddr) {
+ dma_unmap_single(ab->dev, rx_tid->paddr,
+ rx_tid->size, DMA_BIDIRECTIONAL);
+ kfree(rx_tid->vaddr);
+ rx_tid->vaddr = NULL;
+ }
kfree(cmd_cache);
}
spin_unlock_bh(&dp->reo_cmd_lock);
@@ -698,10 +707,12 @@ static void ath11k_dp_reo_cmd_free(struct ath11k_dp *dp, void *ctx,
if (status != HAL_REO_CMD_SUCCESS)
ath11k_warn(dp->ab, "failed to flush rx tid hw desc, tid %d status %d\n",
rx_tid->tid, status);
-
- dma_unmap_single(dp->ab->dev, rx_tid->paddr, rx_tid->size,
- DMA_BIDIRECTIONAL);
- kfree(rx_tid->vaddr);
+ if (rx_tid->vaddr) {
+ dma_unmap_single(dp->ab->dev, rx_tid->paddr, rx_tid->size,
+ DMA_BIDIRECTIONAL);
+ kfree(rx_tid->vaddr);
+ rx_tid->vaddr = NULL;
+ }
}

static void ath11k_dp_reo_cache_flush(struct ath11k_base *ab,
@@ -740,6 +751,7 @@ static void ath11k_dp_reo_cache_flush(struct ath11k_base *ab,
dma_unmap_single(ab->dev, rx_tid->paddr, rx_tid->size,
DMA_BIDIRECTIONAL);
kfree(rx_tid->vaddr);
+ rx_tid->vaddr = NULL;
}
}

@@ -792,6 +804,7 @@ static void ath11k_dp_rx_tid_del_func(struct ath11k_dp *dp, void *ctx,
dma_unmap_single(ab->dev, rx_tid->paddr, rx_tid->size,
DMA_BIDIRECTIONAL);
kfree(rx_tid->vaddr);
+ rx_tid->vaddr = NULL;
}

void ath11k_peer_rx_tid_delete(struct ath11k *ar,
@@ -804,6 +817,8 @@ void ath11k_peer_rx_tid_delete(struct ath11k *ar,
if (!rx_tid->active)
return;

+ rx_tid->active = false;
+
cmd.flag = HAL_REO_CMD_FLG_NEED_STATUS;
cmd.addr_lo = lower_32_bits(rx_tid->paddr);
cmd.addr_hi = upper_32_bits(rx_tid->paddr);
@@ -818,9 +833,11 @@ void ath11k_peer_rx_tid_delete(struct ath11k *ar,
dma_unmap_single(ar->ab->dev, rx_tid->paddr, rx_tid->size,
DMA_BIDIRECTIONAL);
kfree(rx_tid->vaddr);
+ rx_tid->vaddr = NULL;
}

- rx_tid->active = false;
+ rx_tid->paddr = 0;
+ rx_tid->size = 0;
}

static int ath11k_dp_rx_link_desc_return(struct ath11k_base *ab,
@@ -967,6 +984,7 @@ static void ath11k_dp_rx_tid_mem_free(struct ath11k_base *ab,
dma_unmap_single(ab->dev, rx_tid->paddr, rx_tid->size,
DMA_BIDIRECTIONAL);
kfree(rx_tid->vaddr);
+ rx_tid->vaddr = NULL;

rx_tid->active = false;

@@ -1067,7 +1085,8 @@ int ath11k_peer_rx_tid_setup(struct ath11k *ar, const u8 *peer_mac, int vdev_id,
return ret;

err_mem_free:
- kfree(vaddr);
+ kfree(rx_tid->vaddr);
+ rx_tid->vaddr = NULL;

return ret;
}
--
2.17.1

2023-04-19 14:25:53

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/3] wifi: ath11k: fix double free of peer rx_tid during reo cmd failure

Harshitha Prem <[email protected]> wrote:

> Peer rx_tid is locally copied thrice during peer_rx_tid_cleanup to
> send REO_CMD_UPDATE_RX_QUEUE followed by REO_CMD_FLUSH_CACHE to flush
> all aged REO descriptors from HW cache.
>
> When sending REO_CMD_FLUSH_CACHE fails, we do dma unmap of already
> mapped rx_tid->vaddr and free it. This is not checked during
> reo_cmd_list_cleanup() and dp_reo_cmd_free() before trying to free and
> unmap again.
>
> Fix this by setting rx_tid->vaddr NULL in rx tid delete and also
> wherever freeing it to check in reo_cmd_list_cleanup() and
> reo_cmd_free() before trying to free again.
>
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Sathishkumar Muruganandam <[email protected]>
> Signed-off-by: Harshitha Prem <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

3 patches applied to ath-next branch of ath.git, thanks.

93a91f40c25c wifi: ath11k: fix double free of peer rx_tid during reo cmd failure
a8ae833657a4 wifi: ath11k: Prevent REO cmd failures
20487cc3ff36 wifi: ath11k: add peer mac information in failure cases

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches