[PATCH AUTOSEL 4.19 16/25] wifi: ath10k: reset pointer after memory free to avoid potential use-after-free

[PATCH AUTOSEL 4.19 16/25] wifi: ath10k: reset pointer after memory free to avoid potential use-after-free

[Sasha Levin]

From: Wen Gong <quic_wgong@quicinc.com>

[ Upstream commit 1e1cb8e0b73e6f39a9d4a7a15d940b1265387eb5 ]

When running suspend test, kernel crash happened in ath10k, and it is
fixed by commit b72a4aff947b ("ath10k: skip ath10k_halt during suspend
for driver state RESTARTING").

Currently the crash is fixed, but as a common code style, it is better
to set the pointer to NULL after memory is free.

This is to address the code style and it will avoid potential bug of
use-after-free.

Tested-on: QCA6174 hw3.2 PCI WLAN.RM.4.4.1-00110-QCARMSWP-1
Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
Link: https://lore.kernel.org/r/20220505092248.787-1-quic_wgong@quicinc.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 0a7551dc0f94..68728cba6df3 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -302,12 +302,16 @@ void ath10k_htt_rx_free(struct ath10k_htt *htt)
 			  ath10k_htt_get_vaddr_ring(htt),
 			  htt->rx_ring.base_paddr);
 
+	ath10k_htt_config_paddrs_ring(htt, NULL);
+
 	dma_free_coherent(htt->ar->dev,
 			  sizeof(*htt->rx_ring.alloc_idx.vaddr),
 			  htt->rx_ring.alloc_idx.vaddr,
 			  htt->rx_ring.alloc_idx.paddr);
+	htt->rx_ring.alloc_idx.vaddr = NULL;
 
 	kfree(htt->rx_ring.netbufs_ring);
+	htt->rx_ring.netbufs_ring = NULL;
 }
 
 static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt *htt)
@@ -641,8 +645,10 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
 			  ath10k_htt_get_rx_ring_size(htt),
 			  vaddr_ring,
 			  htt->rx_ring.base_paddr);
+	ath10k_htt_config_paddrs_ring(htt, NULL);
 err_dma_ring:
 	kfree(htt->rx_ring.netbufs_ring);
+	htt->rx_ring.netbufs_ring = NULL;
 err_netbuf:
 	return -ENOMEM;
 }
-- 
2.35.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH AUTOSEL 4.19 16/25] wifi: ath10k: reset pointer after memory free to avoid potential use-after-free

[Pavel Machek]

[-- Attachment #1.1: Type: text/plain, Size: 1991 bytes --]

Hi!

> From: Wen Gong <quic_wgong@quicinc.com>
> 
> [ Upstream commit 1e1cb8e0b73e6f39a9d4a7a15d940b1265387eb5 ]
> 
> When running suspend test, kernel crash happened in ath10k, and it is
> fixed by commit b72a4aff947b ("ath10k: skip ath10k_halt during suspend
> for driver state RESTARTING").
> 
> Currently the crash is fixed, but as a common code style, it is better
> to set the pointer to NULL after memory is free.
> 
> This is to address the code style and it will avoid potential bug of
> use-after-free.

We don't have this patch in 4.19:

b72a4aff947b ("ath10k: skip ath10k_halt during suspend for driver state RESTARTING").

We probably should take that one, as this may depend on it. On the
other hand, we don't need this one as it is just a cleanup...

Best regards,
								Pavel
								
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -302,12 +302,16 @@ void ath10k_htt_rx_free(struct ath10k_htt *htt)
>  			  ath10k_htt_get_vaddr_ring(htt),
>  			  htt->rx_ring.base_paddr);
>  
> +	ath10k_htt_config_paddrs_ring(htt, NULL);
> +
>  	dma_free_coherent(htt->ar->dev,
>  			  sizeof(*htt->rx_ring.alloc_idx.vaddr),
>  			  htt->rx_ring.alloc_idx.vaddr,
>  			  htt->rx_ring.alloc_idx.paddr);
> +	htt->rx_ring.alloc_idx.vaddr = NULL;
>  
>  	kfree(htt->rx_ring.netbufs_ring);
> +	htt->rx_ring.netbufs_ring = NULL;
>  }
>  
>  static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt *htt)
> @@ -641,8 +645,10 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
>  			  ath10k_htt_get_rx_ring_size(htt),
>  			  vaddr_ring,
>  			  htt->rx_ring.base_paddr);
> +	ath10k_htt_config_paddrs_ring(htt, NULL);
>  err_dma_ring:
>  	kfree(htt->rx_ring.netbufs_ring);
> +	htt->rx_ring.netbufs_ring = NULL;
>  err_netbuf:
>  	return -ENOMEM;
>  }
> -- 
> 2.35.1

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH AUTOSEL 4.19 16/25] wifi: ath10k: reset pointer after memory free to avoid potential use-after-free

[Greg KH]

On Tue, Oct 18, 2022 at 11:53:56AM +0200, Pavel Machek wrote:
> Hi!
> 
> > From: Wen Gong <quic_wgong@quicinc.com>
> > 
> > [ Upstream commit 1e1cb8e0b73e6f39a9d4a7a15d940b1265387eb5 ]
> > 
> > When running suspend test, kernel crash happened in ath10k, and it is
> > fixed by commit b72a4aff947b ("ath10k: skip ath10k_halt during suspend
> > for driver state RESTARTING").
> > 
> > Currently the crash is fixed, but as a common code style, it is better
> > to set the pointer to NULL after memory is free.
> > 
> > This is to address the code style and it will avoid potential bug of
> > use-after-free.
> 
> We don't have this patch in 4.19:
> 
> b72a4aff947b ("ath10k: skip ath10k_halt during suspend for driver state RESTARTING").
> 
> We probably should take that one, as this may depend on it. On the
> other hand, we don't need this one as it is just a cleanup...

I've dropped this from all queues now, thanks.

greg k-h

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k