linux-kernel - [PATCH 2/2] scsi: wd33c93: Avoid deferencing null pointer in interrupt handler

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240903135857.455818-2-daniel@0x0f.com>
Date: Tue,  3 Sep 2024 22:58:57 +0900
From: Daniel Palmer <daniel@...f.com>
To: linux-m68k@...ts.linux-m68k.org,
	linux-scsi@...r.kernel.org,
	geert@...ux-m68k.org,
	James.Bottomley@...senPartnership.com,
	martin.petersen@...cle.com
Cc: linux-kernel@...r.kernel.org,
	Daniel Palmer <daniel@...f.com>
Subject: [PATCH 2/2] scsi: wd33c93: Avoid deferencing null pointer in interrupt handler

I have no idea if this fix is appropriate, the code in this driver
makes my brain hurt just looking at it, but sometimes when getting
scsi_pointer from cmd cmd itself is actually null and we get a weird
garbage pointer for scsi_pointer.

When this is accessed later on bad things happen. For my machine
this happens when the SCSI bus is initially scanned.

With this "fix" SCSI on my MVME147 is happy again.

[   84.330000] wd33c93-0: chip=WD33c93B/13 no_sync=0xff no_dma=0
[   84.330000]  debug_flags=0x00
[   84.350000]            setup_args=
<snip>
[   84.490000]
[   84.510000]            Version 1.26++ - 10/Feb/2007
[   84.520000] scsi host0: MVME147 built-in SCSI
[   85.480000]  sending SDTR 0103015e00
[   85.480000] 01
[   85.490000] 03
[   85.500000] 01
[   85.510000] 00
[   85.520000] 00
[   85.520000]  sync_xfer=30
[   85.530000] scsi 0:0:5:0: Direct-Access     BlueSCSI HARDDRIVE        2.0  PQ: 0 ANSI: 2
[   85.820000] st: Version 20160209, fixed bufsize 32768, s/g segs 256
[   85.900000] sd 0:0:5:0: Attached scsi generic sg0 type 0

Signed-off-by: Daniel Palmer <daniel@...f.com>
---
 drivers/scsi/wd33c93.c | 10 +++++++---
 drivers/scsi/wd33c93.h |  2 ++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/wd33c93.c b/drivers/scsi/wd33c93.c
index a44b60c9004a..9789d852d541 100644
--- a/drivers/scsi/wd33c93.c
+++ b/drivers/scsi/wd33c93.c
@@ -733,7 +733,7 @@ transfer_bytes(const wd33c93_regs regs, struct scsi_cmnd *cmd,
 void
 wd33c93_intr(struct Scsi_Host *instance)
 {
-	struct scsi_pointer *scsi_pointer;
+	struct scsi_pointer *scsi_pointer = NULL;
 	struct WD33C93_hostdata *hostdata =
 	    (struct WD33C93_hostdata *) instance->hostdata;
 	const wd33c93_regs regs = hostdata->regs;
@@ -752,7 +752,9 @@ wd33c93_intr(struct Scsi_Host *instance)
 #endif
 
 	cmd = (struct scsi_cmnd *) hostdata->connected;	/* assume we're connected */
-	scsi_pointer = WD33C93_scsi_pointer(cmd);
+	/* cmd could be null */
+	if (cmd)
+		scsi_pointer = WD33C93_scsi_pointer(cmd);
 	sr = read_wd33c93(regs, WD_SCSI_STATUS);	/* clear the interrupt */
 	phs = read_wd33c93(regs, WD_COMMAND_PHASE);
 
@@ -828,8 +830,10 @@ wd33c93_intr(struct Scsi_Host *instance)
 		    (struct scsi_cmnd *) hostdata->selecting;
 		hostdata->selecting = NULL;
 
-		/* construct an IDENTIFY message with correct disconnect bit */
+		/* cmd should now be valid and we can get scsi_pointer */
+		scsi_pointer = WD33C93_scsi_pointer(cmd);
 
+		/* construct an IDENTIFY message with correct disconnect bit */
 		hostdata->outgoing_msg[0] = IDENTIFY(0, cmd->device->lun);
 		if (scsi_pointer->phase)
 			hostdata->outgoing_msg[0] |= 0x40;
diff --git a/drivers/scsi/wd33c93.h b/drivers/scsi/wd33c93.h
index e5e4254b1477..898c1c7d024d 100644
--- a/drivers/scsi/wd33c93.h
+++ b/drivers/scsi/wd33c93.h
@@ -259,6 +259,8 @@ struct WD33C93_hostdata {
 
 static inline struct scsi_pointer *WD33C93_scsi_pointer(struct scsi_cmnd *cmd)
 {
+	WARN_ON_ONCE(!cmd);
+
 	return scsi_cmd_priv(cmd);
 }
 
-- 
2.43.0