ath11k: modify null check logic in ath11k_ce_rx_post_pipe()

[PATCH] ath11k: modify null check logic in ath11k_ce_rx_post_pipe()

Posted by Mikhail Lobanov 2 months, 3 weeks ago

The previous logic in ath11k_ce_rx_post_pipe() incorrectly required both 
dest_ring and status_ring to be NULL in order to exit the function. 
This caused the function to continue even if only one of the pointers 
was NULL, potentially leading to null pointer dereferences in 
ath11k_ce_rx_buf_enqueue_pipe().

Fix the condition by modifying the logic so that the function returns 
early if either dest_ring or status_ring is NULL.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Signed-off-by: Mikhail Lobanov <m.lobanov@rosalinux.ru>
---
 drivers/net/wireless/ath/ath11k/ce.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/ce.c b/drivers/net/wireless/ath/ath11k/ce.c
index e66e86bdec20..9d4246d65d68 100644
--- a/drivers/net/wireless/ath/ath11k/ce.c
+++ b/drivers/net/wireless/ath/ath11k/ce.c
@@ -324,7 +324,7 @@ static int ath11k_ce_rx_post_pipe(struct ath11k_ce_pipe *pipe)
 	dma_addr_t paddr;
 	int ret = 0;
 
-	if (!(pipe->dest_ring || pipe->status_ring))
+	if (!pipe->dest_ring || !pipe->status_ring)
 		return 0;
 
 	spin_lock_bh(&ab->ce.ce_lock);
-- 
2.43.0

Re: [PATCH] ath11k: modify null check logic in ath11k_ce_rx_post_pipe()

Posted by Kalle Valo 2 months, 1 week ago

Mikhail Lobanov <m.lobanov@rosalinux.ru> wrote:

> The previous logic in ath11k_ce_rx_post_pipe() incorrectly required both 
> dest_ring and status_ring to be NULL in order to exit the function. 
> This caused the function to continue even if only one of the pointers 
> was NULL, potentially leading to null pointer dereferences in 
> ath11k_ce_rx_buf_enqueue_pipe().
> 
> Fix the condition by modifying the logic so that the function returns 
> early if either dest_ring or status_ring is NULL.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
> Signed-off-by: Mikhail Lobanov <m.lobanov@rosalinux.ru>

Jeff, what do you think?

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240909150824.28195-1-m.lobanov@rosalinux.ru/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
https://docs.kernel.org/process/submitting-patches.html

Re: [PATCH] ath11k: modify null check logic in ath11k_ce_rx_post_pipe()

Posted by Jeff Johnson 2 months, 1 week ago

On 9/19/2024 9:46 AM, Kalle Valo wrote:
> Mikhail Lobanov <m.lobanov@rosalinux.ru> wrote:
> 
>> The previous logic in ath11k_ce_rx_post_pipe() incorrectly required both 
>> dest_ring and status_ring to be NULL in order to exit the function. 
>> This caused the function to continue even if only one of the pointers 
>> was NULL, potentially leading to null pointer dereferences in 
>> ath11k_ce_rx_buf_enqueue_pipe().
>>
>> Fix the condition by modifying the logic so that the function returns 
>> early if either dest_ring or status_ring is NULL.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>
>> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
>> Signed-off-by: Mikhail Lobanov <m.lobanov@rosalinux.ru>
> 
> Jeff, what do you think?
> 
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

(could have just s/||/&&/ but this change is also ok)