2021-09-14 00:48:29

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets

From: Baochen Qiang <[email protected]>

For fragmented packets, ath11k 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
invalid payload will be reinjected to HW and then delivered to host.
What is more, since arbitrary memory could be allocated to the frame, we
don't know what kind of data is contained in the buffer reinjected.
Thus, as a bad result, private info may be leaked.

Note that this issue is only found on Intel platform.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Signed-off-by: Baochen Qiang <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
drivers/net/wireless/ath/ath11k/dp_rx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 90da56316e7e..0c27eead3e02 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -3434,7 +3434,7 @@ static int ath11k_dp_rx_h_defrag_reo_reinject(struct ath11k *ar, struct dp_rx_ti

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, paddr))
return -ENOMEM;

--
2.25.1


2021-09-14 00:48:29

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 4/5] ath11k: Fix memory leak in ath11k_qmi_driver_event_work

From: Baochen Qiang <[email protected]>

The buffer pointed to by event is not freed in case
ATH11K_FLAG_UNREGISTERING bit is set, resulting in
memory leak, so fix it.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Signed-off-by: Baochen Qiang <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
drivers/net/wireless/ath/ath11k/qmi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 7e82d03b0d87..0175e35849bf 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -2753,8 +2753,10 @@ static void ath11k_qmi_driver_event_work(struct work_struct *work)
list_del(&event->list);
spin_unlock(&qmi->event_lock);

- if (test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags))
+ if (test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags)) {
+ kfree(event);
return;
+ }

switch (event->type) {
case ATH11K_QMI_EVENT_SERVER_ARRIVE:
--
2.25.1

2021-09-14 00:49:19

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 2/5] ath11k: Drop MSDU with length error in DP rx path

From: Baochen Qiang <[email protected]>

There are MSDUs whose length are invalid. For example,
attackers may inject on purpose truncated A-MSDUs with
invalid MSDU length.

Such MSDUs are marked with an err bit set in rx attention
tlvs, so we can check and drop them.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
drivers/net/wireless/ath/ath11k/dp_rx.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 0c27eead3e02..c50f70913583 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -142,6 +142,18 @@ static u32 ath11k_dp_rx_h_attn_mpdu_err(struct rx_attention *attn)
return errmap;
}

+static bool ath11k_dp_rx_h_attn_msdu_len_err(struct ath11k_base *ab,
+ struct hal_rx_desc *desc)
+{
+ struct rx_attention *rx_attention;
+ u32 errmap;
+
+ rx_attention = ath11k_dp_rx_get_attention(ab, desc);
+ errmap = ath11k_dp_rx_h_attn_mpdu_err(rx_attention);
+
+ return errmap & DP_RX_MPDU_ERR_MSDU_LEN;
+}
+
static u16 ath11k_dp_rx_h_msdu_start_msdu_len(struct ath11k_base *ab,
struct hal_rx_desc *desc)
{
@@ -2525,6 +2537,12 @@ static int ath11k_dp_rx_process_msdu(struct ath11k *ar,
}

rx_desc = (struct hal_rx_desc *)msdu->data;
+ if (ath11k_dp_rx_h_attn_msdu_len_err(ab, rx_desc)) {
+ ath11k_warn(ar->ab, "msdu len not valid\n");
+ ret = -EIO;
+ goto free_out;
+ }
+
lrx_desc = (struct hal_rx_desc *)last_buf->data;
rx_attention = ath11k_dp_rx_get_attention(ab, lrx_desc);
if (!ath11k_dp_rx_h_attn_msdu_done(rx_attention)) {
--
2.25.1

2021-09-14 00:49:25

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 3/5] ath11k: Fix inaccessible debug registers

From: Baochen Qiang <[email protected]>

Current code clears debug registers after SOC global reset performed
in ath11k_pci_sw_reset. However at that time those registers are
not accessible due to reset, thus they are actually not cleared at all.
For WCN6855, it may cause target fail to initialize. This issue can be
fixed by moving clear action ahead.

In addition, on some specific platforms, need to add delay to wait
those registers to become accessible.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
drivers/net/wireless/ath/ath11k/pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index 5abb38cc3b55..7b3bce0ba76e 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -430,6 +430,8 @@ static void ath11k_pci_force_wake(struct ath11k_base *ab)

static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on)
{
+ mdelay(100);
+
if (power_on) {
ath11k_pci_enable_ltssm(ab);
ath11k_pci_clear_all_intrs(ab);
@@ -439,9 +441,9 @@ static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on)
}

ath11k_mhi_clear_vector(ab);
+ ath11k_pci_clear_dbg_registers(ab);
ath11k_pci_soc_global_reset(ab);
ath11k_mhi_set_mhictrl_reset(ab);
- ath11k_pci_clear_dbg_registers(ab);
}

int ath11k_pci_get_msi_irq(struct device *dev, unsigned int vector)
--
2.25.1

2021-09-14 01:24:34

by Peter Oh

[permalink] [raw]
Subject: Re: [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets


> diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
> index 90da56316e7e..0c27eead3e02 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
> @@ -3434,7 +3434,7 @@ static int ath11k_dp_rx_h_defrag_reo_reinject(struct ath11k *ar, struct dp_rx_ti
>
> 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, paddr))
> return -ENOMEM;
>

Need to update this line too?

err_unmap_dma:
    dma_unmap_single(ab->dev, paddr, defrag_skb->len +
skb_tailroom(defrag_skb),
             DMA_FROM_DEVICE);


Thanks,

Peter

2021-09-14 17:10:44

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets

Jouni Malinen <[email protected]> wrote:

> For fragmented packets, ath11k 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
> invalid payload will be reinjected to HW and then delivered to host.
> What is more, since arbitrary memory could be allocated to the frame, we
> don't know what kind of data is contained in the buffer reinjected.
> Thus, as a bad result, private info may be leaked.
>
> Note that this issue is only found on Intel platform.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Signed-off-by: Baochen Qiang <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Dropping due to the issue Peter found.

Patch set to Changes Requested.

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

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

2021-09-28 13:37:02

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/5] ath11k: Drop MSDU with length error in DP rx path

Jouni Malinen <[email protected]> wrote:

> There are MSDUs whose length are invalid. For example,
> attackers may inject on purpose truncated A-MSDUs with
> invalid MSDU length.
>
> Such MSDUs are marked with an err bit set in rx attention
> tlvs, so we can check and drop them.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>
> Signed-off-by: Baochen Qiang <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

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

cd18ed4cf805 ath11k: Drop MSDU with length error in DP rx path
8a0b899f169d ath11k: Fix inaccessible debug registers
72de799aa9e3 ath11k: Fix memory leak in ath11k_qmi_driver_event_work

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

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