[PATCH] sfc: farch: Potential Null Pointer Dereference in ef4_farch_handle_tx_event()

[PATCH] sfc: farch: Potential Null Pointer Dereference in ef4_farch_handle_tx_event()

[Chen Yufeng]

A patch similar to 83b09a180741("sfc: farch: fix TX queue lookup in TX 
 event handling").

The code was using ef4_channel_get_tx_queue() function with a TXQ label 
parameter, when it should have been using direct queue access via 
channel->tx_queue. This mismatch could result in NULL pointer returns, 
leading to system crashes.

Signed-off-by: Chen Yufeng <chenyufeng@iie.ac.cn>
---
 drivers/net/ethernet/sfc/falcon/farch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sfc/falcon/farch.c b/drivers/net/ethernet/sfc/falcon/farch.c
index 01017c41338e..29b34fb9fb24 100644
--- a/drivers/net/ethernet/sfc/falcon/farch.c
+++ b/drivers/net/ethernet/sfc/falcon/farch.c
@@ -838,16 +838,16 @@ ef4_farch_handle_tx_event(struct ef4_channel *channel, ef4_qword_t *event)
 		/* Transmit completion */
 		tx_ev_desc_ptr = EF4_QWORD_FIELD(*event, FSF_AZ_TX_EV_DESC_PTR);
 		tx_ev_q_label = EF4_QWORD_FIELD(*event, FSF_AZ_TX_EV_Q_LABEL);
-		tx_queue = ef4_channel_get_tx_queue(
-			channel, tx_ev_q_label % EF4_TXQ_TYPES);
+		tx_queue = channel->tx_queue +
+			(tx_ev_q_label % EF4_TXQ_TYPES);
 		tx_packets = ((tx_ev_desc_ptr - tx_queue->read_count) &
 			      tx_queue->ptr_mask);
 		ef4_xmit_done(tx_queue, tx_ev_desc_ptr);
 	} else if (EF4_QWORD_FIELD(*event, FSF_AZ_TX_EV_WQ_FF_FULL)) {
 		/* Rewrite the FIFO write pointer */
 		tx_ev_q_label = EF4_QWORD_FIELD(*event, FSF_AZ_TX_EV_Q_LABEL);
-		tx_queue = ef4_channel_get_tx_queue(
-			channel, tx_ev_q_label % EF4_TXQ_TYPES);
+		tx_queue = channel->tx_queue +
+			(tx_ev_q_label % EF4_TXQ_TYPES);
 
 		netif_tx_lock(efx->net_dev);
 		ef4_farch_notify_tx_desc(tx_queue);
-- 
2.34.1

Re: [PATCH] sfc: farch: Potential Null Pointer Dereference in ef4_farch_handle_tx_event()

[Edward Cree]

On 05/09/2025 04:07, Chen Yufeng wrote:
> A patch similar to 83b09a180741("sfc: farch: fix TX queue lookup in TX 
>  event handling").
> 
> The code was using ef4_channel_get_tx_queue() function with a TXQ label 
> parameter, when it should have been using direct queue access via 
> channel->tx_queue. This mismatch could result in NULL pointer returns, 
> leading to system crashes.
> 
> Signed-off-by: Chen Yufeng <chenyufeng@iie.ac.cn>

This patch doesn't make sense; ef4_channel_get_tx_queue expands to
 &channel->tx_queue[type], which is an equivalent expression to the
 channel->tx_queue + type you're replacing it with.
Where do you think a NULL comes from, and why do you think this fixes it?
-Ed

> ---
>  drivers/net/ethernet/sfc/falcon/farch.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/falcon/farch.c b/drivers/net/ethernet/sfc/falcon/farch.c
> index 01017c41338e..29b34fb9fb24 100644
> --- a/drivers/net/ethernet/sfc/falcon/farch.c
> +++ b/drivers/net/ethernet/sfc/falcon/farch.c
> @@ -838,16 +838,16 @@ ef4_farch_handle_tx_event(struct ef4_channel *channel, ef4_qword_t *event)
>  		/* Transmit completion */
>  		tx_ev_desc_ptr = EF4_QWORD_FIELD(*event, FSF_AZ_TX_EV_DESC_PTR);
>  		tx_ev_q_label = EF4_QWORD_FIELD(*event, FSF_AZ_TX_EV_Q_LABEL);
> -		tx_queue = ef4_channel_get_tx_queue(
> -			channel, tx_ev_q_label % EF4_TXQ_TYPES);
> +		tx_queue = channel->tx_queue +
> +			(tx_ev_q_label % EF4_TXQ_TYPES);
>  		tx_packets = ((tx_ev_desc_ptr - tx_queue->read_count) &
>  			      tx_queue->ptr_mask);
>  		ef4_xmit_done(tx_queue, tx_ev_desc_ptr);
>  	} else if (EF4_QWORD_FIELD(*event, FSF_AZ_TX_EV_WQ_FF_FULL)) {
>  		/* Rewrite the FIFO write pointer */
>  		tx_ev_q_label = EF4_QWORD_FIELD(*event, FSF_AZ_TX_EV_Q_LABEL);
> -		tx_queue = ef4_channel_get_tx_queue(
> -			channel, tx_ev_q_label % EF4_TXQ_TYPES);
> +		tx_queue = channel->tx_queue +
> +			(tx_ev_q_label % EF4_TXQ_TYPES);
>  
>  		netif_tx_lock(efx->net_dev);
>  		ef4_farch_notify_tx_desc(tx_queue);