lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240221-ps3-gelic-null-deref-v1-1-f4fe159c7cb0@kernel.org>
Date: Wed, 21 Feb 2024 16:56:47 +0000
From: Simon Horman <horms@...nel.org>
To: Geoff Levand <geoff@...radead.org>, 
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: Michael Ellerman <mpe@...erman.id.au>, 
 Nicholas Piggin <npiggin@...il.com>, 
 Christophe Leroy <christophe.leroy@...roup.eu>, 
 "Aneesh Kumar K.V" <aneesh.kumar@...nel.org>, 
 "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>, Jeff Garzik <jeff@...zik.org>, 
 Dan Carpenter <dan.carpenter@...aro.org>, netdev@...r.kernel.org, 
 linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: [PATCH RFC net] ps3/gelic: Fix possible NULL pointer dereference

Fix possible NULL pointer dereference in gelic_card_release_tx_chain()

The cited commit introduced a netdev variable to
gelic_card_release_tx_chain() which is set unconditionally on each
iteration of a for loop.

It is set to the value of tx_chain->tail->skb->dev.  However, in some
cases it is assumed that tx_chain->tail->skb may be NULL. And if that
occurs, setting netdev will cause a NULl pointer dereference.

Given the age of this code I do wonder if this can occur in practice.
But to be on the safe side this patch assumes that it can and aims to
avoid the dereference in the case where tx_chain->tail->skb may be NULL.

Flagged by Smatch.
Compile tested only.

Fixes: 589866f9f1cb ("PS3: gelic: Add support for dual network interface")
Signed-off-by: Simon Horman <horms@...nel.org>
---
 drivers/net/ethernet/toshiba/ps3_gelic_net.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index d5b75af163d3..f03489799f5d 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -549,14 +549,13 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
 {
 	struct gelic_descr_chain *tx_chain;
 	enum gelic_descr_dma_status status;
-	struct net_device *netdev;
 	int release = 0;
 
 	for (tx_chain = &card->tx_chain;
 	     tx_chain->head != tx_chain->tail && tx_chain->tail;
 	     tx_chain->tail = tx_chain->tail->next) {
 		status = gelic_descr_get_status(tx_chain->tail);
-		netdev = tx_chain->tail->skb->dev;
+
 		switch (status) {
 		case GELIC_DESCR_DMA_RESPONSE_ERROR:
 		case GELIC_DESCR_DMA_PROTECTION_ERROR:
@@ -566,11 +565,14 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
 					 "%s: forcing end of tx descriptor " \
 					 "with status %x\n",
 					 __func__, status);
-			netdev->stats.tx_dropped++;
+			tx_chain->tail->skb->dev->stats.tx_dropped++;
 			break;
 
 		case GELIC_DESCR_DMA_COMPLETE:
 			if (tx_chain->tail->skb) {
+				struct net_device *netdev;
+
+				netdev = tx_chain->tail->skb->dev;
 				netdev->stats.tx_packets++;
 				netdev->stats.tx_bytes +=
 					tx_chain->tail->skb->len;


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ