[PATCH 0/4] Request to queue zfcp enhancements for upstream v7.3 merge window

[PATCH 0/4] Request to queue zfcp enhancements for upstream v7.3 merge window

[Nihar Panda]

Hi Martin and James,

We have a small zfcp enhancement to improve zfcp trace logging that
 we would like to include in the v7.3 release.

The included fixup patch addresses the sashiko review comment. 

The other findings from the sashiko bot represent pre-existing issues 
rather than regressions introduced by this change, 
so we will address them in a separate, upcoming patch.

Chinmaya Kajagar (2):
  zfcp: Enhance fsf status read buffer tracing
  zfcp: trace return values of sysfs unit add store

Nihar Panda (1):
  fixup! zfcp: trace return values of sysfs unit add store

Steffen Maier (1):
  zfcp: Trace plogi and prli within open port response as payload

 drivers/s390/scsi/zfcp_dbf.c   | 80 +++++++++++++++++++++++++++++++---
 drivers/s390/scsi/zfcp_dbf.h   | 34 ++++++++++++++-
 drivers/s390/scsi/zfcp_ext.h   |  4 +-
 drivers/s390/scsi/zfcp_fsf.h   |  4 +-
 drivers/s390/scsi/zfcp_sysfs.c | 19 +++++---
 5 files changed, 126 insertions(+), 15 deletions(-)

-- 
2.53.0

[PATCH 1/4] zfcp: Enhance fsf status read buffer tracing

[Nihar Panda]

From: Chinmaya Kajagar <chinmayk@linux.ibm.com>

SRB trace records are logged through hba trace event zfcp_dbf_hba_fsf_uss.
Presently, this trace event has few missing fields in fsf status read buffer
trace records. To fully trace incoming fsf status read buffer (SRB), the
remaining fields are needed to be added to zfcp_dbf_hba_uss structure.

Append all the remaining SRB fields to the existing unsolicited status trace
records.

Extend driver to get 3 bytes source id s_id value from fsf status read
buffer's existing reserved field res3.

To display this change, we also change the external tool `zfcpdbf` in the
s390-tools package.

zfcpdbf tool trace example for HBA area after changes:

Timestamp      : 2025-08-22-05:52:04:171750
Area           : HBA
Subarea        : 00
Level          : 2
Exception      : -
CPU ID         : 0003
Caller         : 0x0000021e278c07c8
Record ID      : 2
Tag            : fssrh_4
Description    : fssrh_4 HBA, FSF unsolicited status
Request ID     : 0x0000000000004bfc
Request status : 0x00000000
FSF cmnd       : 0x00006305
FSF sequence no: 0x00000000
SRB stat type  : 0x00000002
SRB stat sub   : 0x00000000
SRB D_ID       : 0x00fffffd
SRB LUN        : 0x0000000000000000
SRB q-design.  : 0x0000000000000000
SRB length     : 0x0000004c
SRB res1       : 00000000
SRB res2       : 00
SRB class      : 0x00000000
SRB res3       : 00
SRB S_ID       : 0x0033c048
SRB res4       : 00000000 00000000 00000000 00000000
                 00000000
SRB pay length : 12
Payload time   : 2025-08-22-05:52:04:171743
SRB info       : 6104000c 0033c024 0033c02e

Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Chinmaya Kajagar <chinmayk@linux.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c |  7 +++++++
 drivers/s390/scsi/zfcp_dbf.h | 14 ++++++++++++++
 drivers/s390/scsi/zfcp_fsf.h |  4 +++-
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 71f625926ae1..89b859176b8b 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -220,6 +220,13 @@ void zfcp_dbf_hba_fsf_uss(char *tag, struct zfcp_fsf_req *req)
 	rec->u.uss.lun = srb->fcp_lun;
 	memcpy(&rec->u.uss.queue_designator, &srb->queue_designator,
 	       sizeof(rec->u.uss.queue_designator));
+	rec->u.uss.length = srb->length;
+	rec->u.uss.res1 = srb->res1;
+	rec->u.uss.res2 = srb->res2;
+	rec->u.uss.class = srb->class;
+	rec->u.uss.res3 = srb->res3;
+	rec->u.uss.s_id = ntoh24(srb->s_id);
+	memcpy(&rec->u.uss.res4, &srb->res4, sizeof(rec->u.uss.res4));
 
 	/* status read buffer payload length */
 	rec->pl_len = (!srb->length) ? 0 : srb->length -
diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index 4d1435c573bc..44ebad8c761c 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -149,6 +149,13 @@ struct zfcp_dbf_hba_res {
  * @d_id: destination ID
  * @lun: logical unit number
  * @queue_designator: queue designator
+ * @length: buffer length
+ * @res1: reserved field 1
+ * @res2: reserved field 2
+ * @class: class of service
+ * @res3: reserved field 3
+ * @s_id: source ID
+ * @res4: reserved field 4
  */
 struct zfcp_dbf_hba_uss {
 	u32 status_type;
@@ -156,6 +163,13 @@ struct zfcp_dbf_hba_uss {
 	u32 d_id;
 	u64 lun;
 	u64 queue_designator;
+	u32 length;
+	u32 res1;
+	u8 res2;
+	u32 class;
+	u8 res3;
+	u32 s_id;
+	u8 res4[20];
 } __packed;
 
 /**
diff --git a/drivers/s390/scsi/zfcp_fsf.h b/drivers/s390/scsi/zfcp_fsf.h
index 5e6b601af980..4b92e85ec71d 100644
--- a/drivers/s390/scsi/zfcp_fsf.h
+++ b/drivers/s390/scsi/zfcp_fsf.h
@@ -246,7 +246,9 @@ struct fsf_status_read_buffer {
 	u8 d_id[3];
 	u32 class;
 	u64 fcp_lun;
-	u8  res3[24];
+	u8 res3;
+	u8 s_id[3];
+	u8 res4[20];
 	union {
 		u8  data[FSF_STATUS_READ_PAYLOAD_SIZE];
 		u32 word[FSF_STATUS_READ_PAYLOAD_SIZE/sizeof(u32)];
-- 
2.53.0

[PATCH 2/4] zfcp: Trace plogi and prli within open port response as payload

[Nihar Panda]

From: Steffen Maier <maier@linux.ibm.com>

The FCP channel optionally returns the content of PLOGI and PRLI within
open port response. This information is needed to debug unexpected open
port responses. Pack both PLOGI and PRLI information back-to-back into a
PAYload trace record of type "fsf_els" within existing HBA trace record.

The length of both parts, and thus also the offset of the second part,
are added to the corresponding HBA trace record. Be extra careful
regarding bounds checking.

Since auto port scan in multi-initiator zoning environments can cause a
lot of failed open port responses and trace is enabled by default in the
HBA trace area, chose a trace level 4 above the default of 3 for the
corresponding PAYload trace record to contain PLOGI/PRLI data. This way,
it avoids flooding the PAY area by default.

In the spirit of commit 35f040df97fa ("zfcp: retain trace level for SCSI
and HBA FSF response records"), pass the level here. For this, introduce
an additional argument 'level' for zfcp_dbf_pl_write().

zfcpdbf tool partial trace example with PLOGI/PRLI log info after changes:

PLOGI length   : 116
PRLI length    : 20
Payload time   : 2026-01-29-06:19:15:626629
PLOGI/PRLIinfo : 02000000 00000000 80000800 000a0002
                 00000000 2002000e 1115c62f 2001000e
                 1115c62f 00000000 00000000 00000000
                 00000000 80000000 00000000 00000000
                 00000000 80000000 00000000 000a0000
                 00010000 00000000 00000000 00000000
                 00000000 00000000 00000000 00000000
                 00000000 02100014 08002100 00000000
                 00000000 00000112

Reviewed-by: M Nikhil <nikh1092@linux.ibm.com>
Reviewed-by: Nihar Panda <niharp@linux.ibm.com>
Signed-off-by: Steffen Maier <maier@linux.ibm.com>
Co-developed-by: Chinmaya Kajagar <chinmayk@linux.ibm.com>
Signed-off-by: Chinmaya Kajagar <chinmayk@linux.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c | 37 ++++++++++++++++++++++++++++++------
 drivers/s390/scsi/zfcp_dbf.h |  4 +++-
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 89b859176b8b..4217b74baa38 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -4,7 +4,7 @@
  *
  * Debug traces for zfcp.
  *
- * Copyright IBM Corp. 2002, 2023
+ * Copyright IBM Corp. 2002, 2026
  */
 
 #define pr_fmt(fmt) "zfcp: " fmt
@@ -35,13 +35,18 @@ static inline unsigned int zfcp_dbf_plen(unsigned int offset)
 	return sizeof(struct zfcp_dbf_pay) + offset - ZFCP_DBF_PAY_MAX_REC;
 }
 
+#define ZFCP_DBF_PAY_LEVEL 1
+
 static inline
 void zfcp_dbf_pl_write(struct zfcp_dbf *dbf, void *data, u16 length, char *area,
-		       u64 req_id)
+		       u64 req_id, int level)
 {
 	struct zfcp_dbf_pay *pl = &dbf->pay_buf;
 	u16 offset = 0, rec_length;
 
+	if (unlikely(!debug_level_enabled(dbf->pay, level)))
+		return;
+
 	spin_lock(&dbf->pay_lock);
 	memset(pl, 0, sizeof(*pl));
 	pl->fsf_req_id = req_id;
@@ -51,7 +56,7 @@ void zfcp_dbf_pl_write(struct zfcp_dbf *dbf, void *data, u16 length, char *area,
 		rec_length = min((u16) ZFCP_DBF_PAY_MAX_REC,
 				 (u16) (length - offset));
 		memcpy(pl->data, data + offset, rec_length);
-		debug_event(dbf->pay, 1, pl, zfcp_dbf_plen(rec_length));
+		debug_event(dbf->pay, level, pl, zfcp_dbf_plen(rec_length));
 
 		offset += rec_length;
 		pl->counter++;
@@ -96,7 +101,27 @@ void zfcp_dbf_hba_fsf_res(char *tag, int level, struct zfcp_fsf_req *req)
 
 	rec->pl_len = q_head->log_length;
 	zfcp_dbf_pl_write(dbf, (char *)q_pref + q_head->log_start,
-			  rec->pl_len, "fsf_res", req->req_id);
+			  rec->pl_len, "fsf_res", req->req_id,
+			  ZFCP_DBF_PAY_LEVEL);
+
+	if (q_head->fsf_command == FSF_QTCB_OPEN_PORT_WITH_DID) {
+		struct fsf_qtcb_bottom_support *q_bott =
+			&req->qtcb->bottom.support;
+		u32 plogi_len = 0, prli_len = 0;
+
+		if (q_bott->els1_length) {
+			rec->u.res.plogi_len = q_bott->els1_length;
+			plogi_len = min_t(u32, q_bott->els1_length,
+					  sizeof(q_bott->els));
+		}
+		if (q_bott->els2_length) {
+			rec->u.res.prli_len = q_bott->els2_length;
+			prli_len = min_t(u32, q_bott->els2_length,
+					 sizeof(q_bott->els) - plogi_len);
+		}
+		zfcp_dbf_pl_write(dbf, q_bott->els, plogi_len + prli_len,
+				  "fsf_els", req->req_id, 4);
+	}
 
 	debug_event(dbf->hba, level, rec, sizeof(*rec));
 	spin_unlock_irqrestore(&dbf->hba_lock, flags);
@@ -234,7 +259,7 @@ void zfcp_dbf_hba_fsf_uss(char *tag, struct zfcp_fsf_req *req)
 
 	if (rec->pl_len)
 		zfcp_dbf_pl_write(dbf, srb->payload.data, rec->pl_len,
-				  "fsf_uss", req->req_id);
+				  "fsf_uss", req->req_id, ZFCP_DBF_PAY_LEVEL);
 log:
 	debug_event(dbf->hba, level, rec, sizeof(*rec));
 	spin_unlock_irqrestore(&dbf->hba_lock, flags);
@@ -739,7 +764,7 @@ void zfcp_dbf_scsi_common(char *tag, int level, struct scsi_device *sdev,
 				min_t(u16, max_t(u16, rec->pl_len,
 						 ZFCP_DBF_PAY_MAX_REC),
 				      FSF_FCP_RSP_SIZE),
-				"fcp_riu", fsf->req_id);
+				"fcp_riu", fsf->req_id, ZFCP_DBF_PAY_LEVEL);
 	}
 
 	debug_event(dbf->scsi, level, rec, sizeof(*rec));
diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index 44ebad8c761c..c84f076440a8 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -3,7 +3,7 @@
  * zfcp device driver
  * debug feature declarations
  *
- * Copyright IBM Corp. 2008, 2020
+ * Copyright IBM Corp. 2008, 2026
  */
 
 #ifndef ZFCP_DBF_H
@@ -140,6 +140,8 @@ struct zfcp_dbf_hba_res {
 	u8  fsf_status_qual[FSF_STATUS_QUALIFIER_SIZE];
 	u32 port_handle;
 	u32 lun_handle;
+	u32 plogi_len;
+	u32 prli_len;
 } __packed;
 
 /**
-- 
2.53.0

[PATCH 3/4] zfcp: trace return values of sysfs unit add store

[Nihar Panda]

From: Chinmaya Kajagar <chinmayk@linux.ibm.com>

Sysfs unit add failures are seen during FCP devices manual SCSI LUN
scans, indicating the kernel cannot add a LUN, usually because the
device is offline, already exists, no memory or the target port is
incorrectly configured.

Add a new trace to debug zfcp sysfs unit add failures with tag id
ZFCP_DBF_HBA_UAS. This traces wwpn, fcp lun id, return value (error
condition) and associated hba of the device.

Typical unit add store failures as seen below example syslog messages,

Feb  2 10:47:25 systemd-udevd[823]: rport-1:0-2: /etc/udev/
rules.d/41-zfcp-lun-0.0.50c0:0x500507680b26c449:0x01d2000000000000.rules:10
Failed to write ATTR{/sys/devices/css0/0.0.0015/0.0.50c0/0x500507680b26c449
/unit_add}, ignoring: File exists

Feb  2 10:50:03 systemd-udevd[801]: rport-2:0-3: /etc/udev/
rules.d/41-zfcp-lun-0.0.50c0:0x500507680b26c448:0x01d2000000000000.rules:10
Failed to write ATTR{/sys/devices/css0/0.0.0015/0.0.50c0/0x500507680b26c448
/unit_add}, ignoring: Cannot allocate memory

Example zfcpdbf traces for both the errors:

Timestamp      : 2026-03-05-07:28:34:029797
Area           : HBA
Subarea        : 00
Level          : 3
Exception      : -
CPU ID         : 0002
Caller         : 0x000001fe345e6d0e
Record ID      : 6
Tag            : syuast2
Description    : syuast2 HBA, unit add, failed, unable to add unit
Request ID     : 0x00000000ffffffff
Request status : 0xffffffff
FSF cmnd       : 0xffffffff
FSF sequence no: 0xffffffff
WWPN           : 0x500507680b25c448
LUN            : 0x01d3000000000000
Return Value   : 0xfffffff4

Timestamp      : 2026-03-05-07:33:04:151807 <== the last record
Area           : HBA
Subarea        : 00
Level          : 3
Exception      : -
CPU ID         : 0002
Caller         : 0x000001fe345e6d0e
Record ID      : 6
Tag            : syuast2
Description    : syuast2 HBA, unit add, failed, unable to add unit
Request ID     : 0x00000000ffffffff
Request status : 0xffffffff
FSF cmnd       : 0xffffffff
FSF sequence no: 0xffffffff
WWPN           : 0x500507680b25c449
LUN            : 0x01d0000000000000
Return Value   : 0xfffffff4

Signed-off-by: Chinmaya Kajagar <chinmayk@linux.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c   | 36 ++++++++++++++++++++++++++++++++++
 drivers/s390/scsi/zfcp_dbf.h   | 16 +++++++++++++++
 drivers/s390/scsi/zfcp_ext.h   |  4 +++-
 drivers/s390/scsi/zfcp_sysfs.c | 17 +++++++++++-----
 4 files changed, 67 insertions(+), 6 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 4217b74baa38..81fb8af408e9 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -265,6 +265,42 @@ void zfcp_dbf_hba_fsf_uss(char *tag, struct zfcp_fsf_req *req)
 	spin_unlock_irqrestore(&dbf->hba_lock, flags);
 }
 
+/**
+ * zfcp_dbf_hba_uas - trace event for sysfs unit add store
+ * @tag: tag indicating which kind of unit add store condition occurred
+ * @level: debug trace level
+ * @adapter: pointer to struct zfcp_adapter
+ * @wwpn: remote port wwn
+ * @fcp_lun: FCP LUN
+ * @ret: return value
+ */
+void zfcp_dbf_hba_uas(char *tag, int level, struct zfcp_adapter *adapter,
+		      u64 wwpn, u64 fcp_lun, int ret)
+{
+	struct zfcp_dbf *dbf = adapter->dbf;
+	struct zfcp_dbf_hba *rec = &dbf->hba_buf;
+	unsigned long flags;
+
+	if (unlikely(!debug_level_enabled(dbf->hba, level)))
+		return;
+
+	spin_lock_irqsave(&dbf->hba_lock, flags);
+	memset(rec, 0, sizeof(*rec));
+
+	memcpy(rec->tag, tag, ZFCP_DBF_TAG_LEN);
+	rec->id = ZFCP_DBF_HBA_UAS;
+	rec->fsf_req_id = ~0u;
+	rec->fsf_req_status = ~0u;
+	rec->fsf_cmd = ~0u;
+	rec->fsf_seq_no = ~0u;
+	rec->u.uas.wwpn = wwpn;
+	rec->u.uas.fcp_lun = fcp_lun;
+	rec->u.uas.ret = ret;
+
+	debug_event(dbf->hba, level, rec, sizeof(*rec));
+	spin_unlock_irqrestore(&dbf->hba_lock, flags);
+}
+
 /**
  * zfcp_dbf_hba_bit_err - trace event for bit error conditions
  * @tag: tag indicating which kind of bit error unsolicited status was received
diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index c84f076440a8..79973fb24b1c 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -174,6 +174,18 @@ struct zfcp_dbf_hba_uss {
 	u8 res4[20];
 } __packed;
 
+/**
+ * struct zfcp_dbf_hba_uas - trace record for sysfs unit add store
+ * @wwpn: remote port wwn
+ * @fcp_lun: FCP LUN
+ * @ret: return value
+ */
+struct zfcp_dbf_hba_uas {
+	u64 wwpn;
+	u64 fcp_lun;
+	u32 ret;
+} __packed;
+
 /**
  * struct zfcp_dbf_hba_fces - trace record for FC Endpoint Security
  * @req_issued: timestamp when request was issued
@@ -200,6 +212,7 @@ struct zfcp_dbf_hba_fces {
  * @ZFCP_DBF_HBA_BIT: bit error trace record
  * @ZFCP_DBF_HBA_BASIC: basic adapter event, only trace tag, no other data
  * @ZFCP_DBF_HBA_FCES: FC Endpoint Security trace record
+ * @ZFCP_DBF_HBA_UAS: unit add store trace record
  */
 enum zfcp_dbf_hba_id {
 	ZFCP_DBF_HBA_RES	= 1,
@@ -207,6 +220,7 @@ enum zfcp_dbf_hba_id {
 	ZFCP_DBF_HBA_BIT	= 3,
 	ZFCP_DBF_HBA_BASIC	= 4,
 	ZFCP_DBF_HBA_FCES	= 5,
+	ZFCP_DBF_HBA_UAS        = 6,
 };
 
 /**
@@ -223,6 +237,7 @@ enum zfcp_dbf_hba_id {
  * @u.uss:  data for unsolicited status buffer
  * @u.be:   data for bit error unsolicited status buffer
  * @u.fces: data for FC Endpoint Security
+ * @u.uas:  data for unit add store
  */
 struct zfcp_dbf_hba {
 	u8 id;
@@ -237,6 +252,7 @@ struct zfcp_dbf_hba {
 		struct zfcp_dbf_hba_uss uss;
 		struct fsf_bit_error_payload be;
 		struct zfcp_dbf_hba_fces fces;
+		struct zfcp_dbf_hba_uas uas;
 	} u;
 } __packed;
 
diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h
index 9f5152b42b0e..40bd597fb4cd 100644
--- a/drivers/s390/scsi/zfcp_ext.h
+++ b/drivers/s390/scsi/zfcp_ext.h
@@ -4,7 +4,7 @@
  *
  * External function declarations.
  *
- * Copyright IBM Corp. 2002, 2023
+ * Copyright IBM Corp. 2002, 2026
  */
 
 #ifndef ZFCP_EXT_H
@@ -49,6 +49,8 @@ extern void zfcp_dbf_hba_fsf_fces(char *tag, const struct zfcp_fsf_req *req,
 extern void zfcp_dbf_hba_fsf_reqid(const char *const tag, const int level,
 				   struct zfcp_adapter *const adapter,
 				   const u64 req_id);
+extern void zfcp_dbf_hba_uas(char *tag, int level, struct zfcp_adapter *adapter,
+			     u64 wwpn, u64 fcp_lun, int ret);
 extern void zfcp_dbf_hba_bit_err(char *, struct zfcp_fsf_req *);
 extern void zfcp_dbf_hba_def_err(struct zfcp_adapter *, u64, u16, void **);
 extern void zfcp_dbf_san_req(char *, struct zfcp_fsf_req *, u32);
diff --git a/drivers/s390/scsi/zfcp_sysfs.c b/drivers/s390/scsi/zfcp_sysfs.c
index 42423549e511..729c9664a48f 100644
--- a/drivers/s390/scsi/zfcp_sysfs.c
+++ b/drivers/s390/scsi/zfcp_sysfs.c
@@ -4,7 +4,7 @@
  *
  * sysfs attributes.
  *
- * Copyright IBM Corp. 2008, 2020
+ * Copyright IBM Corp. 2008, 2026
  */
 
 #define pr_fmt(fmt) "zfcp: " fmt
@@ -442,17 +442,24 @@ static ssize_t zfcp_sysfs_unit_add_store(struct device *dev,
 					 const char *buf, size_t count)
 {
 	struct zfcp_port *port = container_of(dev, struct zfcp_port, dev);
+	struct zfcp_adapter *adapter = port->adapter;
 	u64 fcp_lun;
-	int retval;
+	int retval = -EINVAL;
 
-	if (kstrtoull(buf, 0, (unsigned long long *) &fcp_lun))
-		return -EINVAL;
+	if (kstrtoull(buf, 0, (unsigned long long *)&fcp_lun)) {
+		zfcp_dbf_hba_uas("syuast1", 3, adapter, port->wwpn,
+				 fcp_lun, retval);
+		return retval;
+	}
 
 	flush_work(&port->rport_work);
 
 	retval = zfcp_unit_add(port, fcp_lun);
-	if (retval)
+	if (retval) {
+		zfcp_dbf_hba_uas("syuast2", 3, adapter, port->wwpn,
+				 fcp_lun, retval);
 		return retval;
+	}
 
 	return count;
 }
-- 
2.53.0

[PATCH 4/4] fixup! zfcp: trace return values of sysfs unit add store

[Nihar Panda]

upstream discussions...

Signed-off-by: Nihar Panda <niharp@linux.ibm.com>
---
 drivers/s390/scsi/zfcp_sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_sysfs.c b/drivers/s390/scsi/zfcp_sysfs.c
index 729c9664a48f..597e04cdcc97 100644
--- a/drivers/s390/scsi/zfcp_sysfs.c
+++ b/drivers/s390/scsi/zfcp_sysfs.c
@@ -443,7 +443,7 @@ static ssize_t zfcp_sysfs_unit_add_store(struct device *dev,
 {
 	struct zfcp_port *port = container_of(dev, struct zfcp_port, dev);
 	struct zfcp_adapter *adapter = port->adapter;
-	u64 fcp_lun;
+	u64 fcp_lun = 0x0ULL;
 	int retval = -EINVAL;
 
 	if (kstrtoull(buf, 0, (unsigned long long *)&fcp_lun)) {
-- 
2.53.0

Re: [PATCH 3/4] zfcp: trace return values of sysfs unit add store

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized kernel stack memory (`fcp_lun`) is written to a debug trace buffer when `kstrtoull` parsing fails, causing an information leak to userspace.
--

--- Patch [3]: [PATCH 3/4] zfcp: trace return values of sysfs unit add store ---
commit 2f001159c00d2d39b83745c171b53d3cb2468df3
Author: Chinmaya Kajagar <chinmayk@linux.ibm.com>

zfcp: trace return values of sysfs unit add store

This commit adds tracing for unit_add sysfs attribute failures to help
debug issues where the kernel cannot add a LUN. It adds a new trace record
ZFCP_DBF_HBA_UAS and updates zfcp_sysfs_unit_add_store() to log these events.

> diff --git a/drivers/s390/scsi/zfcp_sysfs.c b/drivers/s390/scsi/zfcp_sysfs.c
> index 42423549e511d..729c9664a48f8 100644
> --- a/drivers/s390/scsi/zfcp_sysfs.c
> +++ b/drivers/s390/scsi/zfcp_sysfs.c
[ ... ]
> @@ -442,17 +442,24 @@ static ssize_t zfcp_sysfs_unit_add_store(struct device *dev,
>  					 const char *buf, size_t count)
>  {
>  	struct zfcp_port *port = container_of(dev, struct zfcp_port, dev);
> +	struct zfcp_adapter *adapter = port->adapter;
>  	u64 fcp_lun;
> -	int retval;
> +	int retval = -EINVAL;
>  
> -	if (kstrtoull(buf, 0, (unsigned long long *) &fcp_lun))
> -		return -EINVAL;
> +	if (kstrtoull(buf, 0, (unsigned long long *)&fcp_lun)) {
> +		zfcp_dbf_hba_uas("syuast1", 3, adapter, port->wwpn,
> +				 fcp_lun, retval);

[Severity: Medium]
Does this code leak uninitialized kernel stack memory into the trace buffer?

If kstrtoull() fails to parse the input, it returns an error and may not
write to fcp_lun. The uninitialized fcp_lun variable is then immediately
passed to zfcp_dbf_hba_uas(), which records it in the trace buffer that
can be read from userspace.

> +		return retval;
> +	}
>  
>  	flush_work(&port->rport_work);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724053934.2781087-1-niharp@linux.ibm.com?part=3

Re: [PATCH 2/4] zfcp: Trace plogi and prli within open port response as payload

[sashiko-bot]

> From: Steffen Maier <maier@linux.ibm.com>
> 
> The FCP channel optionally returns the content of PLOGI and PRLI within
> open port response. This information is needed to debug unexpected open
> port responses. Pack both PLOGI and PRLI information back-to-back into a
> PAYload trace record of type "fsf_els" within existing HBA trace record.
> 
> The length of both parts, and thus also the offset of the second part,
> are added to the corresponding HBA trace record. Be extra careful
> regarding bounds checking.
> 
> Since auto port scan in multi-initiator zoning environments can cause a
> lot of failed open port responses and trace is enabled by default in the
> HBA trace area, chose a trace level 4 above the default of 3 for the
> corresponding PAYload trace record to contain PLOGI/PRLI data. This way,
> [ ... ]
> Reviewed-by: M Nikhil <nikh1092@linux.ibm.com>
> Reviewed-by: Nihar Panda <niharp@linux.ibm.com>
> Signed-off-by: Steffen Maier <maier@linux.ibm.com>
> Co-developed-by: Chinmaya Kajagar <chinmayk@linux.ibm.com>
> Signed-off-by: Chinmaya Kajagar <chinmayk@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724053934.2781087-1-niharp@linux.ibm.com?part=2

Re: [PATCH 4/4] fixup! zfcp: trace return values of sysfs unit add store

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbalanced atomic_dec on error path causes state corruption in `zfcp_unit_add()`, permanently bricking the port.
--

--- Patch [4]: [PATCH 4/4] fixup! zfcp: trace return values of sysfs unit add store ---
commit ccd7f94929b859d2a706671b750a2197236c689b
Author: Nihar Panda <niharp@linux.ibm.com>

fixup! zfcp: trace return values of sysfs unit add store

upstream discussions...

> diff --git a/drivers/s390/scsi/zfcp_sysfs.c b/drivers/s390/scsi/zfcp_sysfs.c
> index 729c9664a48f8..597e04cdcc976 100644
> --- a/drivers/s390/scsi/zfcp_sysfs.c
> +++ b/drivers/s390/scsi/zfcp_sysfs.c
> @@ -443,7 +443,7 @@ static ssize_t zfcp_sysfs_unit_add_store(struct device *dev,
>  {
>  	struct zfcp_port *port = container_of(dev, struct zfcp_port, dev);
>  	struct zfcp_adapter *adapter = port->adapter;
> -	u64 fcp_lun;
> +	u64 fcp_lun = 0x0ULL;
>  	int retval = -EINVAL;
>  
>  	if (kstrtoull(buf, 0, (unsigned long long *)&fcp_lun)) {
>  		zfcp_dbf_hba_uas("syuast1", 3, adapter, port->wwpn,
>  				 fcp_lun, retval);
>  		return retval;
>  	}
> 
>  	flush_work(&port->rport_work);
> 
>  	retval = zfcp_unit_add(port, fcp_lun);

[Severity: High]
This is a pre-existing issue, but does zfcp_unit_add() have an unbalanced
atomic_dec() on its error path that can cause state corruption?

If device_register() fails in zfcp_unit_add(), put_device() is called which
invokes zfcp_unit_release():

drivers/s390/scsi/zfcp_unit.c:zfcp_unit_release() {
	...
	atomic_dec(&unit->port->units);
	kfree(unit);
}

However, atomic_inc(&port->units) is only called after a successful
device_register() in zfcp_unit_add():

drivers/s390/scsi/zfcp_unit.c:zfcp_unit_add() {
	...
	if (device_register(&unit->dev)) {
		put_device(&unit->dev);
		retval = -ENOMEM;
		goto out;
	}

	atomic_inc(&port->units);
	...
}

Because the decrement happens even if the increment hasn't, a failure in
device_register() would drop port->units to -1. Since -1 is used as a
sentinel by zfcp_sysfs_port_is_removing() to determine if a port is
permanently removed, would this failure permanently prevent all future
zfcp_unit_add() operations and SCSI scans on this port?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724053934.2781087-1-niharp@linux.ibm.com?part=4

Re: [PATCH 1/4] zfcp: Enhance fsf status read buffer tracing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] An integer underflow occurs when the hardware/firmware provides a non-zero `srb->length` smaller than the `payload` offset, leading to a massive out-of-bounds read and potential kernel panic.
- [High] Missing bounds check on hardware-provided `log_start` and `log_length` allows an out-of-bounds read and information leak.
--

--- Patch [1]: [PATCH 1/4] zfcp: Enhance fsf status read buffer tracing ---
commit 2318c7a31a1b9a647e0a90e8e4d57fd114b807d1
Author: Chinmaya Kajagar <chinmayk@linux.ibm.com>

zfcp: Enhance fsf status read buffer tracing

This commit appends the remaining SRB fields to the unsolicited status trace
records and extends the driver to get the 3-byte source ID from the reserved
field. This helps fully trace the incoming FSF status read buffer.

> diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
> index 71f625926ae19..89b859176b8bd 100644
> --- a/drivers/s390/scsi/zfcp_dbf.c
> +++ b/drivers/s390/scsi/zfcp_dbf.c

[ ... ]

> @@ -220,6 +220,13 @@ void zfcp_dbf_hba_fsf_uss(char *tag, struct zfcp_fsf_req *req)
>  	rec->u.uss.lun = srb->fcp_lun;
>  	memcpy(&rec->u.uss.queue_designator, &srb->queue_designator,
>  	       sizeof(rec->u.uss.queue_designator));
> +	rec->u.uss.length = srb->length;
> +	rec->u.uss.res1 = srb->res1;
> +	rec->u.uss.res2 = srb->res2;
> +	rec->u.uss.class = srb->class;
> +	rec->u.uss.res3 = srb->res3;
> +	rec->u.uss.s_id = ntoh24(srb->s_id);
> +	memcpy(&rec->u.uss.res4, &srb->res4, sizeof(rec->u.uss.res4));
>  
>  	/* status read buffer payload length */
>  	rec->pl_len = (!srb->length) ? 0 : srb->length -
> 			offsetof(struct fsf_status_read_buffer, payload);
> 
> 	if (rec->pl_len)
> 		zfcp_dbf_pl_write(dbf, srb->payload.data, rec->pl_len,
> 				  "fsf_uss", req->req_id, ZFCP_DBF_PAY_LEVEL);

[Severity: High]
This is a pre-existing issue, but is there a risk of an integer underflow
here if the firmware returns a non-zero srb->length that is smaller than the
payload offset?

If srb->length is less than offsetof(struct fsf_status_read_buffer, payload),
the subtraction will underflow. Because rec->pl_len is a u16, it would store a
large truncated positive integer.

Could this cause zfcp_dbf_pl_write() to perform a massive out-of-bounds read
beyond the srb buffer, potentially leading to a kernel panic from unmapped
memory access or leaking adjacent sensitive kernel heap data into the trace
ring buffer?

An identical instance of this calculation seems to exist in
zfcp_dbf_san_in_els() as well.


While reviewing this tracing code, I also noticed another pre-existing issue
in zfcp_dbf_hba_fsf_res():

drivers/s390/scsi/zfcp_dbf.c:zfcp_dbf_hba_fsf_res() {
	...
	if (q_head->log_length) {
		rec->pl_len = q_head->log_length;
		zfcp_dbf_pl_write(dbf, (char *)q_pref + q_head->log_start,
				  rec->pl_len, "fsf_res", req->req_id,
				  ZFCP_DBF_PAY_LEVEL);
	}
	...
}

[Severity: High]
This is also a pre-existing issue, but does this code adequately validate the
hardware-provided q_head->log_length and q_head->log_start values?

Since these values are read directly from the hardware-populated qtcb header,
is it possible for a malfunctioning or compromised firmware to provide
arbitrary offsets and lengths?

Without bounds checking to ensure they fall strictly within the allocated qtcb
structure, could zfcp_dbf_pl_write() perform an out-of-bounds read from the
kernel heap, potentially leaking sensitive data or causing a crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724053934.2781087-1-niharp@linux.ibm.com?part=1