Thread (8 messages) 8 messages, 4 authors, 2025-03-03

Re: [PATCH RESEND] qed: Move a variable assignment behind a null pointer check in two functions

From: Michal Swiatkowski <hidden>
Date: 2025-03-03 06:25:20
Also in: cocci, kernel-janitors, lkml

On Sun, Mar 02, 2025 at 05:55:40PM +0100, Markus Elfring wrote:
quoted hunk ↗ jump to hunk
From: Markus Elfring <redacted>
Date: Tue, 11 Apr 2023 19:33:53 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the functions “qed_ll2_rxq_completion” and “qed_ll2_txq_completion”.

Thus avoid the risk for undefined behaviour by moving the assignment
for the variables “p_rx” and “p_tx” behind the null pointer check.

This issue was detected by using the Coccinelle software.

Fixes: 0a7fb11c23c0 ("qed: Add Light L2 support")
Signed-off-by: Markus Elfring <redacted>
---
 drivers/net/ethernet/qlogic/qed/qed_ll2.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
index 717a0b3f89bd..941c02fccaaf 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
@@ -346,7 +346,7 @@ static void qed_ll2_txq_flush(struct qed_hwfn *p_hwfn, u8 connection_handle)
 static int qed_ll2_txq_completion(struct qed_hwfn *p_hwfn, void *p_cookie)
 {
 	struct qed_ll2_info *p_ll2_conn = p_cookie;
-	struct qed_ll2_tx_queue *p_tx = &p_ll2_conn->tx_queue;
+	struct qed_ll2_tx_queue *p_tx;
 	u16 new_idx = 0, num_bds = 0, num_bds_in_packet = 0;
 	struct qed_ll2_tx_packet *p_pkt;
 	bool b_last_frag = false;
@@ -356,6 +356,7 @@ static int qed_ll2_txq_completion(struct qed_hwfn *p_hwfn, void *p_cookie)
 	if (!p_ll2_conn)
 		return rc;

+	p_tx = &p_ll2_conn->tx_queue;
 	spin_lock_irqsave(&p_tx->lock, flags);
 	if (p_tx->b_completing_packet) {
 		rc = -EBUSY;
@@ -523,7 +524,7 @@ qed_ll2_rxq_handle_completion(struct qed_hwfn *p_hwfn,
 static int qed_ll2_rxq_completion(struct qed_hwfn *p_hwfn, void *cookie)
 {
 	struct qed_ll2_info *p_ll2_conn = (struct qed_ll2_info *)cookie;
-	struct qed_ll2_rx_queue *p_rx = &p_ll2_conn->rx_queue;
+	struct qed_ll2_rx_queue *p_rx;
 	union core_rx_cqe_union *cqe = NULL;
 	u16 cq_new_idx = 0, cq_old_idx = 0;
 	unsigned long flags = 0;
@@ -532,6 +533,7 @@ static int qed_ll2_rxq_completion(struct qed_hwfn *p_hwfn, void *cookie)
 	if (!p_ll2_conn)
 		return rc;

+	p_rx = &p_ll2_conn->rx_queue;
 	spin_lock_irqsave(&p_rx->lock, flags);

 	if (!QED_LL2_RX_REGISTERED(p_ll2_conn)) {
For future submission plase specify the target kernel
[PATCH net] for fixes, [PATCH net-next] for other.

Looking at the code callback is always set together with cookie (which
is pointing to p_ll2_conn. I am not sure if this is fixing real issue,
but maybe there are a cases when callback is still connected and cookie
is NULL.

Anyway, with heaving this check for p_ll2_conn it is good to move
assigment after this check.

Reviewed-by: Michal Swiatkowski <redacted>
--
2.40.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help