Skip to contentSkip to conversation

qlge: pull NULL check ahead of dereference

PATCH Superseded Dan Carpenter 3 participants 6 messages Aug 19, 2010 – Aug 20, 2010

Subsystems (3)
Networking Network drivers Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 qlge: pull NULL check ahead of dereference

There was a dereference before NULL check issue introduced in 1e213303d "qlge: Add tx multiqueue support." I've pulled the NULL check of "net_rsp" forward a couple lines to restore the original semantics.

Signed-off-by Dan Carpenter <[email protected]>
drivers/net/qlge/qlge_main.c +5 -2
index 8d63f69..611fba4 100644
@@ -2223,9 +2223,12 @@ static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
 		prod = ql_read_sh_reg(rx_ring->prod_idx_sh_reg);
 	}
 	ql_write_cq_idx(rx_ring);
+
+	if (!net_rsp)
+		return 0;
+
 	tx_ring = &qdev->tx_ring[net_rsp->txq_idx];
-	if (__netif_subqueue_stopped(qdev->ndev, tx_ring->wq_id) &&
-					net_rsp != NULL) {
+	if (__netif_subqueue_stopped(qdev->ndev, tx_ring->wq_id)) {
 		if (atomic_read(&tx_ring->queue_stopped) &&
 		    (atomic_read(&tx_ring->tx_count) > (tx_ring->wq_len / 4)))
 			/*
#1 lore raw
Quoted 4 lines · Dan Carpenter

There was a dereference before NULL check issue introduced in 1e213303d "qlge: Add tx multiqueue support." I've pulled the NULL check of "net_rsp" forward a couple lines to restore the original semantics.

net_rsp is initialized to NULL and will never be NULL if entries are processed. It should be fine as it is currently coded.

#2 lore raw
Quoted 9 lines · Ron Mercer
Quoted 4 lines · Dan Carpenter

There was a dereference before NULL check issue introduced in 1e213303d "qlge: Add tx multiqueue support." I've pulled the NULL check of "net_rsp" forward a couple lines to restore the original semantics.

net_rsp is initialized to NULL and will never be NULL if entries are processed. It should be fine as it is currently coded.

On second thought you are right but the early exit should be above the index write.

	ql_write_cq_idx(rx_ring);
author
#3 lore raw

PATCH v2 1/1 qlge: pull NULL check ahead of dereference

There was a dereference before NULL check issue introduced in 1e213303d "qlge: Add tx multiqueue support." I've pulled the NULL check of "net_rsp" forward a couple lines to avoid that.

Also Ron Mercer says that the early exit should be above the index write. ql_write_cq_idx(rx_ring);

Signed-off-by Dan Carpenter <[email protected]>

---
v2: Moved the early exit ahead of the ql_write_cq_idx()

drivers/net/qlge/qlge_main.c +3 -2
index 8d63f69..c9f9754 100644
@@ -2222,10 +2222,11 @@ static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
 		ql_update_cq(rx_ring);
 		prod = ql_read_sh_reg(rx_ring->prod_idx_sh_reg);
 	}
+	if (!net_rsp)
+		return 0;
 	ql_write_cq_idx(rx_ring);
 	tx_ring = &qdev->tx_ring[net_rsp->txq_idx];
-	if (__netif_subqueue_stopped(qdev->ndev, tx_ring->wq_id) &&
-					net_rsp != NULL) {
+	if (__netif_subqueue_stopped(qdev->ndev, tx_ring->wq_id)) {
 		if (atomic_read(&tx_ring->queue_stopped) &&
 		    (atomic_read(&tx_ring->tx_count) > (tx_ring->wq_len / 4)))
 			/*
#4 lore raw
Quoted 7 lines · Dan Carpenter

There was a dereference before NULL check issue introduced in 1e213303d "qlge: Add tx multiqueue support." I've pulled the NULL check of "net_rsp" forward a couple lines to avoid that.

Also Ron Mercer says that the early exit should be above the index write. ql_write_cq_idx(rx_ring);

Signed-off-by Ron Mercer <[email protected]>
maintainer Applied
#5 lore raw

From: Ron Mercer <[email protected]>
Date: Thu, 19 Aug 2010 13:28:36 -0700

Quoted 10 lines · Dan Carpenter
Quoted 7 lines

There was a dereference before NULL check issue introduced in 1e213303d "qlge: Add tx multiqueue support." I've pulled the NULL check of "net_rsp" forward a couple lines to avoid that.

Also Ron Mercer says that the early exit should be above the index write. ql_write_cq_idx(rx_ring);

Signed-off-by Ron Mercer <[email protected]>

Applied to net-next-2.6, thanks guys.