linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] wifi: ath12k: fix issues in rx fragmentation path
@ 2024-05-20  7:00 P Praneesh
  2024-05-20  7:00 ` [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets P Praneesh
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: P Praneesh @ 2024-05-20  7:00 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, P Praneesh

This series addresses issues in rx fragmentation path.

P Praneesh (3):
  wifi: ath12k: change DMA direction while mapping reinjected packets
  wifi: ath12k: fix invalid memory access while processing fragmented
    packets
  wifi: ath12k: fix firmware crash during reo reinject

 drivers/net/wireless/ath/ath12k/dp_rx.c | 18 ++++++++----------
 drivers/net/wireless/ath/ath12k/hw.c    |  6 +-----
 2 files changed, 9 insertions(+), 15 deletions(-)


base-commit: 0ef7606c6012f05a1f5398e3a1964c35eb9c08a4
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets
  2024-05-20  7:00 [PATCH 0/3] wifi: ath12k: fix issues in rx fragmentation path P Praneesh
@ 2024-05-20  7:00 ` P Praneesh
  2024-05-20 23:38   ` Jeff Johnson
  2024-05-25  8:57   ` Kalle Valo
  2024-05-20  7:00 ` [PATCH 2/3] wifi: ath12k: fix invalid memory access while processing fragmented packets P Praneesh
  2024-05-20  7:00 ` [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject P Praneesh
  2 siblings, 2 replies; 12+ messages in thread
From: P Praneesh @ 2024-05-20  7:00 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, P Praneesh, Baochen Qiang

For fragmented packets, ath12k reassembles each fragment as a normal
packet and then reinjects it into HW ring. In this case, the DMA
direction should be DMA_TO_DEVICE, not DMA_FROM_DEVICE. Otherwise,
an invalid payload may be reinjected into the HW and
subsequently delivered to the host.

Given that arbitrary memory can be allocated to the skb buffer,
knowledge about the data contained in the reinjected buffer is lacking.
Consequently, there’s a risk of private information being leaked.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Co-developed-by: Baochen Qiang <quic_bqiang@quicinc.com>
Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/dp_rx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
index 37205e894afe..2bfcc19d15ea 100644
--- a/drivers/net/wireless/ath/ath12k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
@@ -3004,7 +3004,7 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
 
 	buf_paddr = dma_map_single(ab->dev, defrag_skb->data,
 				   defrag_skb->len + skb_tailroom(defrag_skb),
-				   DMA_FROM_DEVICE);
+				   DMA_TO_DEVICE);
 	if (dma_mapping_error(ab->dev, buf_paddr))
 		return -ENOMEM;
 
@@ -3090,7 +3090,7 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
 	spin_unlock_bh(&dp->rx_desc_lock);
 err_unmap_dma:
 	dma_unmap_single(ab->dev, buf_paddr, defrag_skb->len + skb_tailroom(defrag_skb),
-			 DMA_FROM_DEVICE);
+			 DMA_TO_DEVICE);
 	return ret;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] wifi: ath12k: fix invalid memory access while processing fragmented packets
  2024-05-20  7:00 [PATCH 0/3] wifi: ath12k: fix issues in rx fragmentation path P Praneesh
  2024-05-20  7:00 ` [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets P Praneesh
@ 2024-05-20  7:00 ` P Praneesh
  2024-05-20 23:38   ` Jeff Johnson
  2024-05-20  7:00 ` [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject P Praneesh
  2 siblings, 1 reply; 12+ messages in thread
From: P Praneesh @ 2024-05-20  7:00 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, P Praneesh

The monitor ring and the reo reinject ring share the same ring mask index.
When the driver receives an interrupt for the reo reinject ring, the
monitor ring is also processed, leading to invalid memory access. Since
monitor support is not yet enabled in ath12k, the ring mask for the monitor
ring should be removed.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/hw.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/hw.c b/drivers/net/wireless/ath/ath12k/hw.c
index 06f443216488..ffe06ebdc992 100644
--- a/drivers/net/wireless/ath/ath12k/hw.c
+++ b/drivers/net/wireless/ath/ath12k/hw.c
@@ -544,9 +544,6 @@ static const struct ath12k_hw_ring_mask ath12k_hw_ring_mask_qcn9274 = {
 	},
 	.rx_mon_dest = {
 		0, 0, 0,
-		ATH12K_RX_MON_RING_MASK_0,
-		ATH12K_RX_MON_RING_MASK_1,
-		ATH12K_RX_MON_RING_MASK_2,
 	},
 	.rx = {
 		0, 0, 0, 0,
@@ -572,8 +569,7 @@ static const struct ath12k_hw_ring_mask ath12k_hw_ring_mask_qcn9274 = {
 		ATH12K_HOST2RXDMA_RING_MASK_0,
 	},
 	.tx_mon_dest = {
-		ATH12K_TX_MON_RING_MASK_0,
-		ATH12K_TX_MON_RING_MASK_1,
+		0, 0, 0,
 	},
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject
  2024-05-20  7:00 [PATCH 0/3] wifi: ath12k: fix issues in rx fragmentation path P Praneesh
  2024-05-20  7:00 ` [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets P Praneesh
  2024-05-20  7:00 ` [PATCH 2/3] wifi: ath12k: fix invalid memory access while processing fragmented packets P Praneesh
@ 2024-05-20  7:00 ` P Praneesh
  2024-05-20 23:38   ` Jeff Johnson
  2024-05-21  8:50   ` Nicolas Escande
  2 siblings, 2 replies; 12+ messages in thread
From: P Praneesh @ 2024-05-20  7:00 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, P Praneesh

When handling fragmented packets, the ath12k driver reassembles each
fragment into a normal packet and then reinjects it into the HW ring.
However, a firmware crash occurs during this reinjection process.
The issue arises because the driver populates peer metadata in
reo_ent_ring->queue_addr_lo, while the firmware expects the physical
address obtained from the corresponding peer’s queue descriptor. Fix it
by filling peer's queue descriptor's physical address in queue_addr_lo.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/dp_rx.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
index 2bfcc19d15ea..2adb6c7d4a42 100644
--- a/drivers/net/wireless/ath/ath12k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
@@ -2967,7 +2967,7 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
 	struct hal_srng *srng;
 	dma_addr_t link_paddr, buf_paddr;
 	u32 desc_bank, msdu_info, msdu_ext_info, mpdu_info;
-	u32 cookie, hal_rx_desc_sz, dest_ring_info0;
+	u32 cookie, hal_rx_desc_sz, dest_ring_info0, queue_addr_hi;
 	int ret;
 	struct ath12k_rx_desc_info *desc_info;
 	enum hal_rx_buf_return_buf_manager idle_link_rbm = dp->idle_link_rbm;
@@ -3060,13 +3060,11 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
 	reo_ent_ring->rx_mpdu_info.peer_meta_data =
 		reo_dest_ring->rx_mpdu_info.peer_meta_data;
 
-	/* Firmware expects physical address to be filled in queue_addr_lo in
-	 * the MLO scenario and in case of non MLO peer meta data needs to be
-	 * filled.
-	 * TODO: Need to handle for MLO scenario.
-	 */
-	reo_ent_ring->queue_addr_lo = reo_dest_ring->rx_mpdu_info.peer_meta_data;
-	reo_ent_ring->info0 = le32_encode_bits(dst_ind,
+	reo_ent_ring->queue_addr_lo = cpu_to_le32(lower_32_bits(rx_tid->paddr));
+	queue_addr_hi = upper_32_bits(rx_tid->paddr);
+	reo_ent_ring->info0 = le32_encode_bits(queue_addr_hi,
+					       HAL_REO_ENTR_RING_INFO0_QUEUE_ADDR_HI) |
+			      le32_encode_bits(dst_ind,
 					       HAL_REO_ENTR_RING_INFO0_DEST_IND);
 
 	reo_ent_ring->info1 = le32_encode_bits(rx_tid->cur_sn,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets
  2024-05-20  7:00 ` [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets P Praneesh
@ 2024-05-20 23:38   ` Jeff Johnson
  2024-05-25  8:57   ` Kalle Valo
  1 sibling, 0 replies; 12+ messages in thread
From: Jeff Johnson @ 2024-05-20 23:38 UTC (permalink / raw)
  To: P Praneesh, ath12k; +Cc: linux-wireless, Baochen Qiang

On 5/20/2024 12:00 AM, P Praneesh wrote:
> For fragmented packets, ath12k reassembles each fragment as a normal
> packet and then reinjects it into HW ring. In this case, the DMA
> direction should be DMA_TO_DEVICE, not DMA_FROM_DEVICE. Otherwise,
> an invalid payload may be reinjected into the HW and
> subsequently delivered to the host.
> 
> Given that arbitrary memory can be allocated to the skb buffer,
> knowledge about the data contained in the reinjected buffer is lacking.
> Consequently, there’s a risk of private information being leaked.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Co-developed-by: Baochen Qiang <quic_bqiang@quicinc.com>
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>

Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] wifi: ath12k: fix invalid memory access while processing fragmented packets
  2024-05-20  7:00 ` [PATCH 2/3] wifi: ath12k: fix invalid memory access while processing fragmented packets P Praneesh
@ 2024-05-20 23:38   ` Jeff Johnson
  0 siblings, 0 replies; 12+ messages in thread
From: Jeff Johnson @ 2024-05-20 23:38 UTC (permalink / raw)
  To: P Praneesh, ath12k; +Cc: linux-wireless

On 5/20/2024 12:00 AM, P Praneesh wrote:
> The monitor ring and the reo reinject ring share the same ring mask index.
> When the driver receives an interrupt for the reo reinject ring, the
> monitor ring is also processed, leading to invalid memory access. Since
> monitor support is not yet enabled in ath12k, the ring mask for the monitor
> ring should be removed.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>

Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject
  2024-05-20  7:00 ` [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject P Praneesh
@ 2024-05-20 23:38   ` Jeff Johnson
  2024-05-21  8:50   ` Nicolas Escande
  1 sibling, 0 replies; 12+ messages in thread
From: Jeff Johnson @ 2024-05-20 23:38 UTC (permalink / raw)
  To: P Praneesh, ath12k; +Cc: linux-wireless

On 5/20/2024 12:00 AM, P Praneesh wrote:
> When handling fragmented packets, the ath12k driver reassembles each
> fragment into a normal packet and then reinjects it into the HW ring.
> However, a firmware crash occurs during this reinjection process.
> The issue arises because the driver populates peer metadata in
> reo_ent_ring->queue_addr_lo, while the firmware expects the physical
> address obtained from the corresponding peer’s queue descriptor. Fix it
> by filling peer's queue descriptor's physical address in queue_addr_lo.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject
  2024-05-20  7:00 ` [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject P Praneesh
  2024-05-20 23:38   ` Jeff Johnson
@ 2024-05-21  8:50   ` Nicolas Escande
  2024-05-22  6:59     ` Ping-Ke Shih
  2024-05-22  7:15     ` Praneesh P
  1 sibling, 2 replies; 12+ messages in thread
From: Nicolas Escande @ 2024-05-21  8:50 UTC (permalink / raw)
  To: P Praneesh, ath12k; +Cc: linux-wireless

On Mon May 20, 2024 at 9:00 AM CEST, P Praneesh wrote:
> When handling fragmented packets, the ath12k driver reassembles each
> fragment into a normal packet and then reinjects it into the HW ring.
> However, a firmware crash occurs during this reinjection process.
> The issue arises because the driver populates peer metadata in
> reo_ent_ring->queue_addr_lo, while the firmware expects the physical
> address obtained from the corresponding peer’s queue descriptor. Fix it
> by filling peer's queue descriptor's physical address in queue_addr_lo.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1
>
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/dp_rx.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
> index 2bfcc19d15ea..2adb6c7d4a42 100644
> --- a/drivers/net/wireless/ath/ath12k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
> @@ -2967,7 +2967,7 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
>  	struct hal_srng *srng;
>  	dma_addr_t link_paddr, buf_paddr;
>  	u32 desc_bank, msdu_info, msdu_ext_info, mpdu_info;
> -	u32 cookie, hal_rx_desc_sz, dest_ring_info0;
> +	u32 cookie, hal_rx_desc_sz, dest_ring_info0, queue_addr_hi;
>  	int ret;
>  	struct ath12k_rx_desc_info *desc_info;
>  	enum hal_rx_buf_return_buf_manager idle_link_rbm = dp->idle_link_rbm;
> @@ -3060,13 +3060,11 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
>  	reo_ent_ring->rx_mpdu_info.peer_meta_data =
>  		reo_dest_ring->rx_mpdu_info.peer_meta_data;
>  
> -	/* Firmware expects physical address to be filled in queue_addr_lo in
> -	 * the MLO scenario and in case of non MLO peer meta data needs to be
> -	 * filled.
> -	 * TODO: Need to handle for MLO scenario.
> -	 */
> -	reo_ent_ring->queue_addr_lo = reo_dest_ring->rx_mpdu_info.peer_meta_data;
> -	reo_ent_ring->info0 = le32_encode_bits(dst_ind,
> +	reo_ent_ring->queue_addr_lo = cpu_to_le32(lower_32_bits(rx_tid->paddr));
> +	queue_addr_hi = upper_32_bits(rx_tid->paddr);
Shouldn't there be a cpu_to_le32 somewhere here ? It just seems asymetrical
between the two values extracted from rx_tid->paddr
> +	reo_ent_ring->info0 = le32_encode_bits(queue_addr_hi,
> +					       HAL_REO_ENTR_RING_INFO0_QUEUE_ADDR_HI) |
> +			      le32_encode_bits(dst_ind,
>  					       HAL_REO_ENTR_RING_INFO0_DEST_IND);
>  
>  	reo_ent_ring->info1 = le32_encode_bits(rx_tid->cur_sn,


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject
  2024-05-21  8:50   ` Nicolas Escande
@ 2024-05-22  6:59     ` Ping-Ke Shih
  2024-05-22  7:15     ` Praneesh P
  1 sibling, 0 replies; 12+ messages in thread
From: Ping-Ke Shih @ 2024-05-22  6:59 UTC (permalink / raw)
  To: Nicolas Escande, P Praneesh, ath12k@lists.infradead.org
  Cc: linux-wireless@vger.kernel.org

Nicolas Escande <nico.escande@gmail.com> wrote:

[...]

> > -     reo_ent_ring->queue_addr_lo = reo_dest_ring->rx_mpdu_info.peer_meta_data;
> > -     reo_ent_ring->info0 = le32_encode_bits(dst_ind,
> > +     reo_ent_ring->queue_addr_lo = cpu_to_le32(lower_32_bits(rx_tid->paddr));
> > +     queue_addr_hi = upper_32_bits(rx_tid->paddr);
> Shouldn't there be a cpu_to_le32 somewhere here ? It just seems asymetrical
> between the two values extracted from rx_tid->paddr
> > +     reo_ent_ring->info0 = le32_encode_bits(queue_addr_hi,

le32_encode_bits() will convert queue_addr_hi from cpu-order to le-order.

> > +                                            HAL_REO_ENTR_RING_INFO0_QUEUE_ADDR_HI) |
> > +                           le32_encode_bits(dst_ind,
> >                                              HAL_REO_ENTR_RING_INFO0_DEST_IND);
> >
> >       reo_ent_ring->info1 = le32_encode_bits(rx_tid->cur_sn,
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject
  2024-05-21  8:50   ` Nicolas Escande
  2024-05-22  6:59     ` Ping-Ke Shih
@ 2024-05-22  7:15     ` Praneesh P
  2024-05-22  7:28       ` Nicolas Escande
  1 sibling, 1 reply; 12+ messages in thread
From: Praneesh P @ 2024-05-22  7:15 UTC (permalink / raw)
  To: Nicolas Escande, ath12k; +Cc: linux-wireless



On 5/21/2024 2:20 PM, Nicolas Escande wrote:
> On Mon May 20, 2024 at 9:00 AM CEST, P Praneesh wrote:
>> When handling fragmented packets, the ath12k driver reassembles each
>> fragment into a normal packet and then reinjects it into the HW ring.
>> However, a firmware crash occurs during this reinjection process.
>> The issue arises because the driver populates peer metadata in
>> reo_ent_ring->queue_addr_lo, while the firmware expects the physical
>> address obtained from the corresponding peer’s queue descriptor. Fix it
>> by filling peer's queue descriptor's physical address in queue_addr_lo.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1
>>
>> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
>> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/dp_rx.c | 14 ++++++--------
>>   1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
>> index 2bfcc19d15ea..2adb6c7d4a42 100644
>> --- a/drivers/net/wireless/ath/ath12k/dp_rx.c
>> +++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
>> @@ -2967,7 +2967,7 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
>>   	struct hal_srng *srng;
>>   	dma_addr_t link_paddr, buf_paddr;
>>   	u32 desc_bank, msdu_info, msdu_ext_info, mpdu_info;
>> -	u32 cookie, hal_rx_desc_sz, dest_ring_info0;
>> +	u32 cookie, hal_rx_desc_sz, dest_ring_info0, queue_addr_hi;
>>   	int ret;
>>   	struct ath12k_rx_desc_info *desc_info;
>>   	enum hal_rx_buf_return_buf_manager idle_link_rbm = dp->idle_link_rbm;
>> @@ -3060,13 +3060,11 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
>>   	reo_ent_ring->rx_mpdu_info.peer_meta_data =
>>   		reo_dest_ring->rx_mpdu_info.peer_meta_data;
>>   
>> -	/* Firmware expects physical address to be filled in queue_addr_lo in
>> -	 * the MLO scenario and in case of non MLO peer meta data needs to be
>> -	 * filled.
>> -	 * TODO: Need to handle for MLO scenario.
>> -	 */
>> -	reo_ent_ring->queue_addr_lo = reo_dest_ring->rx_mpdu_info.peer_meta_data;
>> -	reo_ent_ring->info0 = le32_encode_bits(dst_ind,
>> +	reo_ent_ring->queue_addr_lo = cpu_to_le32(lower_32_bits(rx_tid->paddr));
>> +	queue_addr_hi = upper_32_bits(rx_tid->paddr);
> Shouldn't there be a cpu_to_le32 somewhere here ? It just seems asymetrical
> between the two values extracted from rx_tid->paddr
le32_encode_bits of queue_addr_hi does that conversion, so there is no 
need to explicitly convert cpu_to_le32 while fetching rx_tid->paddr's 
upper 32 bits.
>> +	reo_ent_ring->info0 = le32_encode_bits(queue_addr_hi,
>> +					       HAL_REO_ENTR_RING_INFO0_QUEUE_ADDR_HI) |
>> +			      le32_encode_bits(dst_ind,
>>   					       HAL_REO_ENTR_RING_INFO0_DEST_IND);
>>   
>>   	reo_ent_ring->info1 = le32_encode_bits(rx_tid->cur_sn,
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject
  2024-05-22  7:15     ` Praneesh P
@ 2024-05-22  7:28       ` Nicolas Escande
  0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Escande @ 2024-05-22  7:28 UTC (permalink / raw)
  To: Praneesh P, ath12k; +Cc: linux-wireless

On Wed May 22, 2024 at 9:15 AM CEST, Praneesh P wrote:
>
>
> On 5/21/2024 2:20 PM, Nicolas Escande wrote:
> > On Mon May 20, 2024 at 9:00 AM CEST, P Praneesh wrote:
> >> When handling fragmented packets, the ath12k driver reassembles each
> >> fragment into a normal packet and then reinjects it into the HW ring.
> >> However, a firmware crash occurs during this reinjection process.
> >> The issue arises because the driver populates peer metadata in
> >> reo_ent_ring->queue_addr_lo, while the firmware expects the physical
> >> address obtained from the corresponding peer’s queue descriptor. Fix it
> >> by filling peer's queue descriptor's physical address in queue_addr_lo.
> >>
> >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1
> >>
> >> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> >> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
> >> ---
> >>   drivers/net/wireless/ath/ath12k/dp_rx.c | 14 ++++++--------
> >>   1 file changed, 6 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
> >> index 2bfcc19d15ea..2adb6c7d4a42 100644
> >> --- a/drivers/net/wireless/ath/ath12k/dp_rx.c
> >> +++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
> >> @@ -2967,7 +2967,7 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
> >>   	struct hal_srng *srng;
> >>   	dma_addr_t link_paddr, buf_paddr;
> >>   	u32 desc_bank, msdu_info, msdu_ext_info, mpdu_info;
> >> -	u32 cookie, hal_rx_desc_sz, dest_ring_info0;
> >> +	u32 cookie, hal_rx_desc_sz, dest_ring_info0, queue_addr_hi;
> >>   	int ret;
> >>   	struct ath12k_rx_desc_info *desc_info;
> >>   	enum hal_rx_buf_return_buf_manager idle_link_rbm = dp->idle_link_rbm;
> >> @@ -3060,13 +3060,11 @@ static int ath12k_dp_rx_h_defrag_reo_reinject(struct ath12k *ar,
> >>   	reo_ent_ring->rx_mpdu_info.peer_meta_data =
> >>   		reo_dest_ring->rx_mpdu_info.peer_meta_data;
> >>   
> >> -	/* Firmware expects physical address to be filled in queue_addr_lo in
> >> -	 * the MLO scenario and in case of non MLO peer meta data needs to be
> >> -	 * filled.
> >> -	 * TODO: Need to handle for MLO scenario.
> >> -	 */
> >> -	reo_ent_ring->queue_addr_lo = reo_dest_ring->rx_mpdu_info.peer_meta_data;
> >> -	reo_ent_ring->info0 = le32_encode_bits(dst_ind,
> >> +	reo_ent_ring->queue_addr_lo = cpu_to_le32(lower_32_bits(rx_tid->paddr));
> >> +	queue_addr_hi = upper_32_bits(rx_tid->paddr);
> > Shouldn't there be a cpu_to_le32 somewhere here ? It just seems asymetrical
> > between the two values extracted from rx_tid->paddr
> le32_encode_bits of queue_addr_hi does that conversion, so there is no 
> need to explicitly convert cpu_to_le32 while fetching rx_tid->paddr's 
> upper 32 bits.
OK, got it,
> >> +	reo_ent_ring->info0 = le32_encode_bits(queue_addr_hi,
> >> +					       HAL_REO_ENTR_RING_INFO0_QUEUE_ADDR_HI) |
> >> +			      le32_encode_bits(dst_ind,
> >>   					       HAL_REO_ENTR_RING_INFO0_DEST_IND);
> >>   
> >>   	reo_ent_ring->info1 = le32_encode_bits(rx_tid->cur_sn,
> > 
Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets
  2024-05-20  7:00 ` [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets P Praneesh
  2024-05-20 23:38   ` Jeff Johnson
@ 2024-05-25  8:57   ` Kalle Valo
  1 sibling, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2024-05-25  8:57 UTC (permalink / raw)
  To: P Praneesh; +Cc: ath12k, linux-wireless, P Praneesh, Baochen Qiang

P Praneesh <quic_ppranees@quicinc.com> wrote:

> For fragmented packets, ath12k reassembles each fragment as a normal
> packet and then reinjects it into HW ring. In this case, the DMA
> direction should be DMA_TO_DEVICE, not DMA_FROM_DEVICE. Otherwise,
> an invalid payload may be reinjected into the HW and
> subsequently delivered to the host.
> 
> Given that arbitrary memory can be allocated to the skb buffer,
> knowledge about the data contained in the reinjected buffer is lacking.
> Consequently, there’s a risk of private information being leaked.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00209-QCAHKSWPL_SILICONZ-1
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Co-developed-by: Baochen Qiang <quic_bqiang@quicinc.com>
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

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

33322e3ef074 wifi: ath12k: change DMA direction while mapping reinjected packets
073f9f249eec wifi: ath12k: fix invalid memory access while processing fragmented packets
a57ab7cced45 wifi: ath12k: fix firmware crash during reo reinject

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240520070045.631029-2-quic_ppranees@quicinc.com/

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-05-25  8:57 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-20  7:00 [PATCH 0/3] wifi: ath12k: fix issues in rx fragmentation path P Praneesh
2024-05-20  7:00 ` [PATCH 1/3] wifi: ath12k: change DMA direction while mapping reinjected packets P Praneesh
2024-05-20 23:38   ` Jeff Johnson
2024-05-25  8:57   ` Kalle Valo
2024-05-20  7:00 ` [PATCH 2/3] wifi: ath12k: fix invalid memory access while processing fragmented packets P Praneesh
2024-05-20 23:38   ` Jeff Johnson
2024-05-20  7:00 ` [PATCH 3/3] wifi: ath12k: fix firmware crash during reo reinject P Praneesh
2024-05-20 23:38   ` Jeff Johnson
2024-05-21  8:50   ` Nicolas Escande
2024-05-22  6:59     ` Ping-Ke Shih
2024-05-22  7:15     ` Praneesh P
2024-05-22  7:28       ` Nicolas Escande

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).