[PATCH 0/5] scsi: lpfc: Remove all strlcat() uses

[PATCH 0/5] scsi: lpfc: Remove all strlcat() uses

[Ian Bridges]

In preparation for removing the strlcat() API[1], this series replaces
its 81 remaining call sites in the lpfc driver. The sites live in nine
string building functions across five files, and each patch converts
one source file.

Functions that accumulate a variable number of fragments move to
seq_buf. The three sysfs show functions move to sysfs_emit_at(), the
designated helper for sysfs output. lpfc_vport_symbolic_node_name()
builds five fixed fragments and becomes a single scnprintf() call.
The intermediate tmp buffers and the per fragment overflow checks
become unnecessary in every scheme. Each loop that appends keeps one
overflow exit, so a full buffer stops the iteration.

One cross-cutting behavior change applies to several patches. The old
code formatted each fragment into a fixed size tmp buffer before
appending it, so a fragment longer than that buffer was silently
truncated even when the destination had room for it. The replacements
format each fragment directly into the destination. Truncation is
still bounded by the destination size. The per patch changelogs call
out the affected functions.

The patch series was tested as follows. No hardware testing was
done. Testing on real adapters is welcome.

- W=1 builds of the whole driver directory, zero warnings.
- A userspace differential harness. The old and new function bodies
  are extracted verbatim from the two trees and compiled side by side
  against the real lib/seq_buf.c. 472000 randomized cases across all
  nine functions, including oversized inputs, undersized buffers and
  prefilled destinations, compared byte for byte under ASan and
  UBSan. All outputs are identical except two behavior changes.
  Those are the format string interpretation removed in patch 1 and
  the fragment cap removal in patch 2. The harness classifies every
  observed difference as exactly one of those two.
- A KUnit corpus. The nine functions run as compiled kernel code in a
  QEMU guest with KASAN, UBSAN and FORTIFY_SOURCE enabled, against
  fabricated adapter state covering both branches of every converted
  conditional that is compiled in. The LPFC_MXP_STAT debug block is
  disabled at compile time and was build tested with the macro
  defined. The same 40 test cases run on the unpatched base and
  on this series. The base run matches the old expected outputs, and
  the patched run is byte identical everywhere except the two
  documented changes.

[1] https://github.com/KSPP/linux/issues/370

Ian Bridges (5):
  scsi: lpfc: Replace strlcat() with seq_buf in lpfc_info()
  scsi: lpfc: Replace strlcat() with scnprintf() in
    lpfc_vport_symbolic_node_name()
  scsi: lpfc: Replace strlcat() with seq_buf in lpfc_rx_monitor_report()
  scsi: lpfc: Replace strlcat() with seq_buf in the debugfs dump helpers
  scsi: lpfc: Replace strlcat() with sysfs_emit_at() in the sysfs show
    functions

 drivers/scsi/lpfc/lpfc_attr.c    | 555 +++++++++++++------------------
 drivers/scsi/lpfc/lpfc_ct.c      |  29 +-
 drivers/scsi/lpfc/lpfc_debugfs.c | 202 +++++------
 drivers/scsi/lpfc/lpfc_scsi.c    |  49 +--
 drivers/scsi/lpfc/lpfc_scsi.h    |   3 -
 drivers/scsi/lpfc/lpfc_sli.c     |  43 +--
 6 files changed, 361 insertions(+), 520 deletions(-)

-- 
2.47.3

[PATCH 1/5] scsi: lpfc: Replace strlcat() with seq_buf in lpfc_info()

[Ian Bridges]

In preparation for removing the strlcat() API[1], replace its uses in
lpfc_info().

The function accumulates a variable number of optional fragments,
which is what seq_buf is for. The intermediate tmp buffer and the
per fragment overflow checks become unnecessary. seq_buf is memory
safe by construction and silently truncates in the same way as the
replaced pattern.

The old code passed phba->ModelDesc as the format string of the first
scnprintf() call. The model description comes from adapter VPD data.
seq_buf_printf() takes a format string, so the replacement prints it
through "%s". A model description containing conversion specifiers is
no longer interpreted.

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
A model description that contains percent characters is now emitted
literally. The old code interpreted it as a format string, which is
undefined behavior for any conversion that consumes an argument. The
differential harness confirmed byte identical output for 200000
randomized percent-free model strings across all fragment
combinations, and demonstrated the old interpretation with a directed
"%%" input. The KUnit corpus confirmed the same as compiled kernel
code.

Once the seq_buf overflows, later appends write nothing, so removing
the early exits does not change the produced bytes. The harness
confirmed identical output over the full field ranges.

 drivers/scsi/lpfc/lpfc_scsi.c | 49 +++++++++++------------------------
 1 file changed, 15 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index f2cab134af7f..8a795c65e3c3 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -21,6 +21,7 @@
  * included with this package.                                     *
  *******************************************************************/
 #include <linux/pci.h>
+#include <linux/seq_buf.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/export.h>
@@ -5103,57 +5104,37 @@ lpfc_info(struct Scsi_Host *host)
 	struct lpfc_hba   *phba = vport->phba;
 	int link_speed = 0;
 	static char lpfcinfobuf[384];
-	char tmp[384] = {0};
+	struct seq_buf s;
 
 	memset(lpfcinfobuf, 0, sizeof(lpfcinfobuf));
+	seq_buf_init(&s, lpfcinfobuf, sizeof(lpfcinfobuf));
 	if (phba && phba->pcidev){
 		/* Model Description */
-		scnprintf(tmp, sizeof(tmp), phba->ModelDesc);
-		if (strlcat(lpfcinfobuf, tmp, sizeof(lpfcinfobuf)) >=
-		    sizeof(lpfcinfobuf))
-			goto buffer_done;
+		seq_buf_printf(&s, "%s", phba->ModelDesc);
 
 		/* PCI Info */
-		scnprintf(tmp, sizeof(tmp),
-			  " on PCI bus %02x device %02x irq %d",
-			  phba->pcidev->bus->number, phba->pcidev->devfn,
-			  phba->pcidev->irq);
-		if (strlcat(lpfcinfobuf, tmp, sizeof(lpfcinfobuf)) >=
-		    sizeof(lpfcinfobuf))
-			goto buffer_done;
+		seq_buf_printf(&s, " on PCI bus %02x device %02x irq %d",
+			       phba->pcidev->bus->number, phba->pcidev->devfn,
+			       phba->pcidev->irq);
 
 		/* Port Number */
-		if (phba->Port[0]) {
-			scnprintf(tmp, sizeof(tmp), " port %s", phba->Port);
-			if (strlcat(lpfcinfobuf, tmp, sizeof(lpfcinfobuf)) >=
-			    sizeof(lpfcinfobuf))
-				goto buffer_done;
-		}
+		if (phba->Port[0])
+			seq_buf_printf(&s, " port %s", phba->Port);
 
 		/* Link Speed */
 		link_speed = lpfc_sli_port_speed_get(phba);
-		if (link_speed != 0) {
-			scnprintf(tmp, sizeof(tmp),
-				  " Logical Link Speed: %d Mbps", link_speed);
-			if (strlcat(lpfcinfobuf, tmp, sizeof(lpfcinfobuf)) >=
-			    sizeof(lpfcinfobuf))
-				goto buffer_done;
-		}
+		if (link_speed != 0)
+			seq_buf_printf(&s, " Logical Link Speed: %d Mbps",
+				       link_speed);
 
 		/* Support for BSG ioctls */
-		scnprintf(tmp, sizeof(tmp), " BSG");
-		if (strlcat(lpfcinfobuf, tmp, sizeof(lpfcinfobuf)) >=
-		    sizeof(lpfcinfobuf))
-			goto buffer_done;
+		seq_buf_printf(&s, " BSG");
 
 		/* PCI resettable */
-		if (!lpfc_check_pci_resettable(phba)) {
-			scnprintf(tmp, sizeof(tmp), " PCI resettable");
-			strlcat(lpfcinfobuf, tmp, sizeof(lpfcinfobuf));
-		}
+		if (!lpfc_check_pci_resettable(phba))
+			seq_buf_printf(&s, " PCI resettable");
 	}
 
-buffer_done:
 	return lpfcinfobuf;
 }
 
-- 
2.47.3

[PATCH 2/5] scsi: lpfc: Replace strlcat() with scnprintf() in lpfc_vport_symbolic_node_name()

[Ian Bridges]

In preparation for removing the strlcat() API[1], replace its uses in
lpfc_vport_symbolic_node_name().

The function builds five unconditional fragments, so one scnprintf()
call composes the whole string. The intermediate tmp buffer and the
per fragment overflow checks become unnecessary. scnprintf()
truncates at the buffer size and returns the number of bytes it
wrote, which equals the length that the removed strnlen() call
computed.

The old code capped every fragment at MAXHOSTNAMELEN bytes before
appending it, independently of the room left in the destination. The
replacement formats each fragment directly into the destination, so a
fragment longer than MAXHOSTNAMELEN is no longer truncated when the
destination has room for it.

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
The MAXHOSTNAMELEN cap can bind on three fragments. A model name
longer than 56 characters, a host name longer than 59 characters or
an OS name longer than 59 characters now reaches the symbolic node
name in full instead of being cut at the old tmp boundary. The
firmware revision and driver version fragments are too short for
the cap by their own bounds. The differential harness swept
eighteen buffer sizes between 1 and 300 across 4000 randomized
field sets and found the outputs identical everywhere the cap did
not bind, with every difference classified as this cap removal.
The KUnit corpus executed representative cases of the same
truncation behavior as compiled kernel code.

 drivers/scsi/lpfc/lpfc_ct.c | 29 +++++------------------------
 1 file changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
index c7853e7fe071..0734ab3be3e3 100644
--- a/drivers/scsi/lpfc/lpfc_ct.c
+++ b/drivers/scsi/lpfc/lpfc_ct.c
@@ -1823,34 +1823,15 @@ lpfc_vport_symbolic_node_name(struct lpfc_vport *vport, char *symbol,
 	size_t size)
 {
 	char fwrev[FW_REV_STR_SIZE] = {0};
-	char tmp[MAXHOSTNAMELEN] = {0};
-
-	memset(symbol, 0, size);
-
-	scnprintf(tmp, sizeof(tmp), "Emulex %s", vport->phba->ModelName);
-	if (strlcat(symbol, tmp, size) >= size)
-		goto buffer_done;
 
 	lpfc_decode_firmware_rev(vport->phba, fwrev, 0);
-	scnprintf(tmp, sizeof(tmp), " FV%s", fwrev);
-	if (strlcat(symbol, tmp, size) >= size)
-		goto buffer_done;
-
-	scnprintf(tmp, sizeof(tmp), " DV%s", lpfc_release_version);
-	if (strlcat(symbol, tmp, size) >= size)
-		goto buffer_done;
-
-	scnprintf(tmp, sizeof(tmp), " HN:%s", vport->phba->os_host_name);
-	if (strlcat(symbol, tmp, size) >= size)
-		goto buffer_done;
 
+	memset(symbol, 0, size);
 	/* Note :- OS name is "Linux" */
-	scnprintf(tmp, sizeof(tmp), " OS:%s", init_utsname()->sysname);
-	strlcat(symbol, tmp, size);
-
-buffer_done:
-	return strnlen(symbol, size);
-
+	return scnprintf(symbol, size, "Emulex %s FV%s DV%s HN:%s OS:%s",
+			 vport->phba->ModelName, fwrev,
+			 lpfc_release_version, vport->phba->os_host_name,
+			 init_utsname()->sysname);
 }
 
 static uint32_t
-- 
2.47.3

[PATCH 3/5] scsi: lpfc: Replace strlcat() with seq_buf in lpfc_rx_monitor_report()

[Ian Bridges]

In preparation for removing the strlcat() API[1], replace its use in
lpfc_rx_monitor_report().

The function accumulates one line per ring entry, which is what
seq_buf is for. seq_buf tracks the write position, so the per entry
strlen() rescans of the destination are gone. Each record is still
formatted into the tmp buffer. seq_buf_puts() appends it only when
it fits whole, so the output keeps ending at the last complete
record. The loop still stops on overflow without consuming the
current entry, and the returned count and the ring head keep their
old meaning. The produced bytes are unchanged.

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
seq_buf_puts() accepts a record under exactly the same condition as
the replaced length check, so truncation keeps ending the output at
the last complete record and a partial record can never reach the
buffer. The differential harness compared 50000 randomized rings
and buffer sizes, including buffers that overflow mid ring, and
found byte identical output, entry counts and ring heads
everywhere. The KUnit corpus reproduced the overflow case with a
populated ring as compiled kernel code and confirmed the same.

 drivers/scsi/lpfc/lpfc_sli.c | 43 ++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 62a30a92b792..cfa4371169f1 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -25,6 +25,7 @@
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
+#include <linux/seq_buf.h>
 #include <linux/slab.h>
 #include <linux/lockdep.h>
 #include <linux/dmi.h>
@@ -8109,17 +8110,18 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
 	u32 cnt = 0;
 	char tmp[DBG_LOG_STR_SZ] = {0};
 	bool log_to_kmsg = (!buf || !buf_len) ? true : false;
+	struct seq_buf s;
 
 	if (!log_to_kmsg) {
 		/* clear the buffer to be sure */
 		memset(buf, 0, buf_len);
 
-		scnprintf(buf, buf_len, "\t%-16s%-16s%-16s%-16s%-8s%-8s%-8s"
-					"%-8s%-8s%-8s%-16s\n",
-					"MaxBPI", "Tot_Data_CMF",
-					"Tot_Data_Cmd", "Tot_Data_Cmpl",
-					"Lat(us)", "Avg_IO", "Max_IO", "Bsy",
-					"IO_cnt", "Info", "BWutil(ms)");
+		seq_buf_init(&s, buf, buf_len);
+		seq_buf_printf(&s, "\t%-16s%-16s%-16s%-16s%-8s%-8s%-8s%-8s%-8s%-8s%-16s\n",
+			       "MaxBPI", "Tot_Data_CMF",
+			       "Tot_Data_Cmd", "Tot_Data_Cmpl",
+			       "Lat(us)", "Avg_IO", "Max_IO", "Bsy",
+			       "IO_cnt", "Info", "BWutil(ms)");
 	}
 
 	/* Needs to be _irq because record is called from timer interrupt
@@ -8131,24 +8133,27 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
 
 		/* Read out this entry's data. */
 		if (!log_to_kmsg) {
-			/* If !log_to_kmsg, then store to buf. */
+			/*
+			 * Drop a record whole if it does not fit, without
+			 * consuming its ring entry.
+			 */
 			scnprintf(tmp, sizeof(tmp),
-				  "%03d:\t%-16llu%-16llu%-16llu%-16llu%-8llu"
-				  "%-8llu%-8llu%-8u%-8u%-8u%u(%u)\n",
-				  *head_idx, entry->max_bytes_per_interval,
-				  entry->cmf_bytes, entry->total_bytes,
-				  entry->rcv_bytes, entry->avg_io_latency,
-				  entry->avg_io_size, entry->max_read_cnt,
+				  "%03d:\t%-16llu%-16llu%-16llu%-16llu%-8llu%-8llu%-8llu%-8u%-8u%-8u%u(%u)\n",
+				  *head_idx,
+				  entry->max_bytes_per_interval,
+				  entry->cmf_bytes,
+				  entry->total_bytes,
+				  entry->rcv_bytes,
+				  entry->avg_io_latency,
+				  entry->avg_io_size,
+				  entry->max_read_cnt,
 				  entry->cmf_busy, entry->io_cnt,
-				  entry->cmf_info, entry->timer_utilization,
+				  entry->cmf_info,
+				  entry->timer_utilization,
 				  entry->timer_interval);
 
-			/* Check for buffer overflow */
-			if ((strlen(buf) + strlen(tmp)) >= buf_len)
+			if (seq_buf_puts(&s, tmp) < 0)
 				break;
-
-			/* Append entry's data to buffer */
-			strlcat(buf, tmp, buf_len);
 		} else {
 			lpfc_printf_log(phba, KERN_INFO, LOG_CGN_MGMT,
 					"4410 %02u: MBPI %llu Xmit %llu "
-- 
2.47.3

[PATCH 4/5] scsi: lpfc: Replace strlcat() with seq_buf in the debugfs dump helpers

[Ian Bridges]

In preparation for removing the strlcat() API[1], replace its uses in
lpfc_debugfs_multixripools_data(), lpfc_debugfs_scsistat_data() and
lpfc_debugfs_hdwqstat_data().

Each helper accumulates a variable number of lines into the debugfs
buffer, which is what seq_buf is for. The intermediate tmp buffers and
the per fragment overflow checks become unnecessary. Once a seq_buf
overflows, later writes to it do nothing, so dropping the early exits
does not change the produced bytes. Each loop that appends keeps one
seq_buf_has_overflowed() exit, so a full buffer stops the iteration.

lpfc_debugfs_multixripools_data() and lpfc_debugfs_hdwqstat_data()
append to whatever the buffer already holds, so their seq_buf is
anchored at the current end of the string. All three helpers keep
returning strnlen() because seq_buf_used() reports the full buffer
size after an overflow.

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
The differential harness compared 90000 randomized cases, including
full range counter values, prefilled buffers for the two anchored
helpers and undersized buffers, and found byte identical output and
return values everywhere. The KUnit corpus confirmed the same as
compiled kernel code.

Notes on the loop exits. Each loop checks seq_buf_has_overflowed()
once per iteration because every iteration makes several appends.
seq_buf itself writes nothing after an overflow, so the exits change
no bytes, which the harness verified. A userspace timing comparison
of the old and new function bodies, extracted verbatim from the two
trees, measured the conversion 1.2 to 2 times faster than the old
code at every queue and CPU count up to the driver maximums, with or
without the exits. The strlcat() rescans dominate the old cost. The
exits bound the remaining iteration work once the buffer is full.
That is measurable only for lpfc_debugfs_hdwqstat_data() at maximum
queue and CPU counts, where the per CPU walk is the largest loop.

 drivers/scsi/lpfc/lpfc_debugfs.c | 202 +++++++++++++------------------
 1 file changed, 87 insertions(+), 115 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 052023fc1733..30e3cc050aaa 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -27,6 +27,7 @@
 #include <linux/idr.h>
 #include <linux/interrupt.h>
 #include <linux/kthread.h>
+#include <linux/seq_buf.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
 #include <linux/spinlock.h>
@@ -462,7 +463,8 @@ lpfc_debugfs_multixripools_data(struct lpfc_hba *phba, char *buf, int size)
 	struct lpfc_pvt_pool *pvt_pool;
 	struct lpfc_pbl_pool *pbl_pool;
 	u32 txcmplq_cnt;
-	char tmp[LPFC_DEBUG_OUT_LINE_SZ] = {0};
+	size_t used;
+	struct seq_buf s;
 
 	if (phba->sli_rev != LPFC_SLI_REV4)
 		return 0;
@@ -475,6 +477,9 @@ lpfc_debugfs_multixripools_data(struct lpfc_hba *phba, char *buf, int size)
 		return i;
 	}
 
+	used = strnlen(buf, size);
+	seq_buf_init(&s, buf + used, size - used);
+
 	/*
 	 * Pbl: Current number of free XRIs in public pool
 	 * Pvt: Current number of free XRIs in private pool
@@ -484,10 +489,8 @@ lpfc_debugfs_multixripools_data(struct lpfc_hba *phba, char *buf, int size)
 	 * pbl_empty: Incremented by 1 when all pbl_pool are empty during
 	 *            IO submission
 	 */
-	scnprintf(tmp, sizeof(tmp),
-		  "HWQ:  Pbl  Pvt Busy  HWM |  pvt_empty  pbl_empty ");
-	if (strlcat(buf, tmp, size) >= size)
-		return strnlen(buf, size);
+	seq_buf_printf(&s,
+		       "HWQ:  Pbl  Pvt Busy  HWM |  pvt_empty  pbl_empty ");
 
 #ifdef LPFC_MXP_STAT
 	/*
@@ -501,25 +504,17 @@ lpfc_debugfs_multixripools_data(struct lpfc_hba *phba, char *buf, int size)
 	 * othPbl_hit: Incremented by 1 if successfully get a batch of XRI from
 	 *             other pbl_pool
 	 */
-	scnprintf(tmp, sizeof(tmp),
-		  "MAXH  above_lmt  below_lmt locPbl_hit othPbl_hit");
-	if (strlcat(buf, tmp, size) >= size)
-		return strnlen(buf, size);
+	seq_buf_printf(&s, "MAXH  above_lmt  below_lmt locPbl_hit othPbl_hit");
 
 	/*
 	 * sPbl: snapshot of Pbl 15 sec after stat gets cleared
 	 * sPvt: snapshot of Pvt 15 sec after stat gets cleared
 	 * sBusy: snapshot of Busy 15 sec after stat gets cleared
 	 */
-	scnprintf(tmp, sizeof(tmp),
-		  " | sPbl sPvt sBusy");
-	if (strlcat(buf, tmp, size) >= size)
-		return strnlen(buf, size);
+	seq_buf_printf(&s, " | sPbl sPvt sBusy");
 #endif
 
-	scnprintf(tmp, sizeof(tmp), "\n");
-	if (strlcat(buf, tmp, size) >= size)
-		return strnlen(buf, size);
+	seq_buf_printf(&s, "\n");
 
 	hwq_count = phba->cfg_hdw_queue;
 	for (i = 0; i < hwq_count; i++) {
@@ -531,36 +526,32 @@ lpfc_debugfs_multixripools_data(struct lpfc_hba *phba, char *buf, int size)
 		pvt_pool = &multixri_pool->pvt_pool;
 		txcmplq_cnt = qp->io_wq->pring->txcmplq_cnt;
 
-		scnprintf(tmp, sizeof(tmp),
-			  "%03d: %4d %4d %4d %4d | %10d %10d ",
-			  i, pbl_pool->count, pvt_pool->count,
-			  txcmplq_cnt, pvt_pool->high_watermark,
-			  qp->empty_io_bufs, multixri_pool->pbl_empty_count);
-		if (strlcat(buf, tmp, size) >= size)
-			break;
+		seq_buf_printf(&s,
+			       "%03d: %4d %4d %4d %4d | %10d %10d ",
+			       i, pbl_pool->count, pvt_pool->count,
+			       txcmplq_cnt, pvt_pool->high_watermark,
+			       qp->empty_io_bufs,
+			       multixri_pool->pbl_empty_count);
 
 #ifdef LPFC_MXP_STAT
-		scnprintf(tmp, sizeof(tmp),
-			  "%4d %10d %10d %10d %10d",
-			  multixri_pool->stat_max_hwm,
-			  multixri_pool->above_limit_count,
-			  multixri_pool->below_limit_count,
-			  multixri_pool->local_pbl_hit_count,
-			  multixri_pool->other_pbl_hit_count);
-		if (strlcat(buf, tmp, size) >= size)
-			break;
-
-		scnprintf(tmp, sizeof(tmp),
-			  " | %4d %4d %5d",
-			  multixri_pool->stat_pbl_count,
-			  multixri_pool->stat_pvt_count,
-			  multixri_pool->stat_busy_count);
-		if (strlcat(buf, tmp, size) >= size)
-			break;
+		seq_buf_printf(&s,
+			       "%4d %10d %10d %10d %10d",
+			       multixri_pool->stat_max_hwm,
+			       multixri_pool->above_limit_count,
+			       multixri_pool->below_limit_count,
+			       multixri_pool->local_pbl_hit_count,
+			       multixri_pool->other_pbl_hit_count);
+
+		seq_buf_printf(&s,
+			       " | %4d %4d %5d",
+			       multixri_pool->stat_pbl_count,
+			       multixri_pool->stat_pvt_count,
+			       multixri_pool->stat_busy_count);
 #endif
 
-		scnprintf(tmp, sizeof(tmp), "\n");
-		if (strlcat(buf, tmp, size) >= size)
+		seq_buf_printf(&s, "\n");
+
+		if (seq_buf_has_overflowed(&s))
 			break;
 	}
 	return strnlen(buf, size);
@@ -1261,13 +1252,14 @@ lpfc_debugfs_scsistat_data(struct lpfc_vport *vport, char *buf, int size)
 	u64 data1, data2, data3;
 	u64 tot, totin, totout;
 	int i;
-	char tmp[LPFC_MAX_SCSI_INFO_TMP_LEN] = {0};
+	struct seq_buf s;
 
 	if (!(vport->cfg_enable_fc4_type & LPFC_ENABLE_FCP) ||
 	    (phba->sli_rev != LPFC_SLI_REV4))
 		return 0;
 
-	scnprintf(buf, size, "SCSI HDWQ Statistics\n");
+	seq_buf_init(&s, buf, size);
+	seq_buf_printf(&s, "SCSI HDWQ Statistics\n");
 
 	totin = 0;
 	totout = 0;
@@ -1280,21 +1272,18 @@ lpfc_debugfs_scsistat_data(struct lpfc_vport *vport, char *buf, int size)
 		data3 = cstat->control_requests;
 		totout += (data1 + data2 + data3);
 
-		scnprintf(tmp, sizeof(tmp), "HDWQ (%d): Rd %016llx Wr %016llx "
-			  "IO %016llx ", i, data1, data2, data3);
-		if (strlcat(buf, tmp, size) >= size)
-			goto buffer_done;
+		seq_buf_printf(&s, "HDWQ (%d): Rd %016llx Wr %016llx IO %016llx ",
+			       i, data1, data2, data3);
+
+		seq_buf_printf(&s, "Cmpl %016llx OutIO %016llx\n",
+			       tot, ((data1 + data2 + data3) - tot));
 
-		scnprintf(tmp, sizeof(tmp), "Cmpl %016llx OutIO %016llx\n",
-			  tot, ((data1 + data2 + data3) - tot));
-		if (strlcat(buf, tmp, size) >= size)
-			goto buffer_done;
+		if (seq_buf_has_overflowed(&s))
+			break;
 	}
-	scnprintf(tmp, sizeof(tmp), "Total FCP Cmpl %016llx Issue %016llx "
-		  "OutIO %016llx\n", totin, totout, totout - totin);
-	strlcat(buf, tmp, size);
+	seq_buf_printf(&s, "Total FCP Cmpl %016llx Issue %016llx OutIO %016llx\n",
+		       totin, totout, totout - totin);
 
-buffer_done:
 	len = strnlen(buf, size);
 
 	return len;
@@ -1704,28 +1693,23 @@ lpfc_debugfs_hdwqstat_data(struct lpfc_vport *vport, char *buf, int size)
 	uint32_t tot_xmt;
 	uint32_t tot_rcv;
 	uint32_t tot_cmpl;
-	char tmp[LPFC_MAX_SCSI_INFO_TMP_LEN] = {0};
+	size_t used = strnlen(buf, size);
+	struct seq_buf s;
 
-	scnprintf(tmp, sizeof(tmp), "HDWQ Stats:\n\n");
-	if (strlcat(buf, tmp, size) >= size)
-		goto buffer_done;
+	seq_buf_init(&s, buf + used, size - used);
 
-	scnprintf(tmp, sizeof(tmp), "(NVME Accounting: %s) ",
-		  (phba->hdwqstat_on &
-		  (LPFC_CHECK_NVME_IO | LPFC_CHECK_NVMET_IO) ?
-		  "Enabled" : "Disabled"));
-	if (strlcat(buf, tmp, size) >= size)
-		goto buffer_done;
+	seq_buf_printf(&s, "HDWQ Stats:\n\n");
 
-	scnprintf(tmp, sizeof(tmp), "(SCSI Accounting: %s) ",
-		  (phba->hdwqstat_on & LPFC_CHECK_SCSI_IO ?
-		  "Enabled" : "Disabled"));
-	if (strlcat(buf, tmp, size) >= size)
-		goto buffer_done;
+	seq_buf_printf(&s, "(NVME Accounting: %s) ",
+		       (phba->hdwqstat_on &
+		       (LPFC_CHECK_NVME_IO | LPFC_CHECK_NVMET_IO) ?
+		       "Enabled" : "Disabled"));
 
-	scnprintf(tmp, sizeof(tmp), "\n\n");
-	if (strlcat(buf, tmp, size) >= size)
-		goto buffer_done;
+	seq_buf_printf(&s, "(SCSI Accounting: %s) ",
+		       (phba->hdwqstat_on & LPFC_CHECK_SCSI_IO ?
+		       "Enabled" : "Disabled"));
+
+	seq_buf_printf(&s, "\n\n");
 
 	for (i = 0; i < phba->cfg_hdw_queue; i++) {
 		tot_rcv = 0;
@@ -1744,62 +1728,50 @@ lpfc_debugfs_hdwqstat_data(struct lpfc_vport *vport, char *buf, int size)
 			    !c_stat->rcv_io)
 				continue;
 
-			if (!tot_xmt && !tot_cmpl && !tot_rcv) {
-				/* Print HDWQ string only the first time */
-				scnprintf(tmp, sizeof(tmp), "[HDWQ %d]:\t", i);
-				if (strlcat(buf, tmp, size) >= size)
-					goto buffer_done;
-			}
+			/* Print HDWQ string only the first time */
+			if (!tot_xmt && !tot_cmpl && !tot_rcv)
+				seq_buf_printf(&s, "[HDWQ %d]:\t", i);
 
 			tot_xmt += c_stat->xmt_io;
 			tot_cmpl += c_stat->cmpl_io;
 			if (phba->nvmet_support)
 				tot_rcv += c_stat->rcv_io;
 
-			scnprintf(tmp, sizeof(tmp), "| [CPU %d]: ", j);
-			if (strlcat(buf, tmp, size) >= size)
-				goto buffer_done;
-
-			if (phba->nvmet_support) {
-				scnprintf(tmp, sizeof(tmp),
-					  "XMT 0x%x CMPL 0x%x RCV 0x%x |",
-					  c_stat->xmt_io, c_stat->cmpl_io,
-					  c_stat->rcv_io);
-				if (strlcat(buf, tmp, size) >= size)
-					goto buffer_done;
-			} else {
-				scnprintf(tmp, sizeof(tmp),
-					  "XMT 0x%x CMPL 0x%x |",
-					  c_stat->xmt_io, c_stat->cmpl_io);
-				if (strlcat(buf, tmp, size) >= size)
-					goto buffer_done;
-			}
+			seq_buf_printf(&s, "| [CPU %d]: ", j);
+
+			if (phba->nvmet_support)
+				seq_buf_printf(&s,
+					       "XMT 0x%x CMPL 0x%x RCV 0x%x |",
+					       c_stat->xmt_io, c_stat->cmpl_io,
+					       c_stat->rcv_io);
+			else
+				seq_buf_printf(&s,
+					       "XMT 0x%x CMPL 0x%x |",
+					       c_stat->xmt_io, c_stat->cmpl_io);
+
+			if (seq_buf_has_overflowed(&s))
+				break;
 		}
 
+		if (seq_buf_has_overflowed(&s))
+			break;
+
 		/* Check if nothing to display */
 		if (!tot_xmt && !tot_cmpl && !tot_rcv)
 			continue;
 
-		scnprintf(tmp, sizeof(tmp), "\t->\t[HDWQ Total: ");
-		if (strlcat(buf, tmp, size) >= size)
-			goto buffer_done;
+		seq_buf_printf(&s, "\t->\t[HDWQ Total: ");
 
-		if (phba->nvmet_support) {
-			scnprintf(tmp, sizeof(tmp),
-				  "XMT 0x%x CMPL 0x%x RCV 0x%x]\n\n",
-				  tot_xmt, tot_cmpl, tot_rcv);
-			if (strlcat(buf, tmp, size) >= size)
-				goto buffer_done;
-		} else {
-			scnprintf(tmp, sizeof(tmp),
-				  "XMT 0x%x CMPL 0x%x]\n\n",
-				  tot_xmt, tot_cmpl);
-			if (strlcat(buf, tmp, size) >= size)
-				goto buffer_done;
-		}
+		if (phba->nvmet_support)
+			seq_buf_printf(&s,
+				       "XMT 0x%x CMPL 0x%x RCV 0x%x]\n\n",
+				       tot_xmt, tot_cmpl, tot_rcv);
+		else
+			seq_buf_printf(&s,
+				       "XMT 0x%x CMPL 0x%x]\n\n",
+				       tot_xmt, tot_cmpl);
 	}
 
-buffer_done:
 	len = strnlen(buf, size);
 	return len;
 }
-- 
2.47.3

[PATCH 5/5] scsi: lpfc: Replace strlcat() with sysfs_emit_at() in the sysfs show functions

[Ian Bridges]

In preparation for removing the strlcat() API[1], replace its uses in
lpfc_cmf_info_show(), lpfc_nvme_info_show() and lpfc_scsi_stat_show().

The three functions build sysfs attribute output, and sysfs_emit_at()
is the designated helper for that. The single write paths become
sysfs_emit(), the offset zero form of the same helper. Each
intermediate tmp buffer and its per fragment overflow check become
unnecessary. Once the page is full, sysfs_emit_at() writes nothing
more, so dropping the early exits does not change the produced
bytes. Each loop that appends keeps one exit, so a full page stops
the iteration. In lpfc_nvme_info_show() the exit also releases the
fc_nodes_list_lock as it did before. The unlock_buf_done label loses
its last user and is removed.

The old code capped every fragment at LPFC_MAX_INFO_TMP_LEN or
LPFC_MAX_SCSI_INFO_TMP_LEN bytes before appending it. The replacement
formats each fragment directly into the page, so a fragment longer
than its old tmp buffer is no longer truncated when the page has room
for it. Both macros lose their last user and are removed.

The running length that sysfs_emit_at() maintains equals the length
that the removed strnlen() calls computed, so the "Could be more
info" overflow markers keep their trigger condition.

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
The differential harness compared 60000 randomized cases, including
remote port lists that overflow the page and exercise the overflow
markers, and found byte identical output and return values
everywhere. The fragment cap removal described in the changelog
produces no byte difference in these three functions because the
fixed field widths keep every fragment below the old caps. The
starting strnlen() preserves the append semantics of the replaced
strlcat() instead of assuming an empty page. The KUnit corpus
confirmed the same as compiled kernel code, including the page
overflow marker path and both the target and initiator branches of
lpfc_nvme_info_show().

Notes on the removed per append overflow checks. The checks were
added by commit afff0d2321ea ("scsi: lpfc: Add Buffer overflow
check, when nvme_info larger than PAGE_SIZE") to stop a real
overflow. The snprintf() size arithmetic used before that commit
could go negative. sysfs_emit_at() removes that failure class by
construction. The offset is validated, every write is bounded
internally, and the accumulated length cannot exceed PAGE_SIZE - 1.
The 6314 truncation message added by the same commit keeps its
trigger condition.

Commit 79080d349f7f ("scsi: lpfc: correct rcu unlock issue in
lpfc_nvme_info_show") made the exits inside the remote port loop
release the lock, fixing a leak. The kept loop exit preserves that
property. A full page stops the iteration and releases the
fc_nodes_list_lock as before. The exit checks once per port instead
of after every append, so on the boundary the lock is held for at
most one port's worth of writeless appends longer than before.

A userspace timing comparison of the old and new function bodies,
extracted verbatim from the two trees, measured the conversion 1.5
to 1.8 times faster wherever the page does not overflow. The
strlcat() rescans dominate the old cost. Without the loop exits the
overflow path grew linearly with the remote port count while
holding the fc_nodes_list_lock. With them it stays flat and faster
than the old code in every measured case. Each remaining writeless
append after a full page costs about 170 nanoseconds, bounded by
the fragment count at a few microseconds per overflowed read.

 drivers/scsi/lpfc/lpfc_attr.c | 555 ++++++++++++++--------------------
 drivers/scsi/lpfc/lpfc_scsi.h |   3 -
 2 files changed, 230 insertions(+), 328 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index f4e8164b94ab..bb5c84f0b787 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -57,7 +57,6 @@
 #define LPFC_MIN_DEVLOSS_TMO	1
 #define LPFC_MAX_DEVLOSS_TMO	255
 
-#define LPFC_MAX_INFO_TMP_LEN	100
 #define LPFC_INFO_MORE_STR	"\nCould be more info...\n"
 /*
  * Write key size should be multiple of 4. If write key is changed
@@ -126,133 +125,121 @@ lpfc_cmf_info_show(struct device *dev, struct device_attribute *attr,
 	int  len = 0;
 	int cpu;
 	u64 rcv, total;
-	char tmp[LPFC_MAX_INFO_TMP_LEN] = {0};
 
 	if (phba->cgn_i)
 		cp = (struct lpfc_cgn_info *)phba->cgn_i->virt;
 
-	scnprintf(tmp, sizeof(tmp),
-		  "Congestion Mgmt Info: E2Eattr %d Ver %d "
-		  "CMF %d cnt %d\n",
-		  phba->sli4_hba.pc_sli4_params.mi_cap,
-		  cp ? cp->cgn_info_version : 0,
-		  phba->sli4_hba.pc_sli4_params.cmf, phba->cmf_timer_cnt);
-
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
+	len = strnlen(buf, PAGE_SIZE);
+	len += sysfs_emit_at(buf, len,
+			     "Congestion Mgmt Info: E2Eattr %d Ver %d CMF %d cnt %d\n",
+			     phba->sli4_hba.pc_sli4_params.mi_cap,
+			     cp ? cp->cgn_info_version : 0,
+			     phba->sli4_hba.pc_sli4_params.cmf,
+			     phba->cmf_timer_cnt);
 
 	if (!phba->sli4_hba.pc_sli4_params.cmf)
 		goto buffer_done;
 
 	switch (phba->cgn_init_reg_signal) {
 	case EDC_CG_SIG_WARN_ONLY:
-		scnprintf(tmp, sizeof(tmp),
-			  "Register: Init:  Signal:WARN  ");
+		len += sysfs_emit_at(buf, len,
+				     "Register: Init:  Signal:WARN  ");
 		break;
 	case EDC_CG_SIG_WARN_ALARM:
-		scnprintf(tmp, sizeof(tmp),
-			  "Register: Init:  Signal:WARN|ALARM  ");
+		len += sysfs_emit_at(buf, len,
+				     "Register: Init:  Signal:WARN|ALARM  ");
 		break;
 	default:
-		scnprintf(tmp, sizeof(tmp),
-			  "Register: Init:  Signal:NONE  ");
+		len += sysfs_emit_at(buf, len,
+				     "Register: Init:  Signal:NONE  ");
 		break;
 	}
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
 
 	switch (phba->cgn_init_reg_fpin) {
 	case LPFC_CGN_FPIN_WARN:
-		scnprintf(tmp, sizeof(tmp),
-			  "FPIN:WARN\n");
+		len += sysfs_emit_at(buf, len, "FPIN:WARN\n");
 		break;
 	case LPFC_CGN_FPIN_ALARM:
-		scnprintf(tmp, sizeof(tmp),
-			  "FPIN:ALARM\n");
+		len += sysfs_emit_at(buf, len, "FPIN:ALARM\n");
 		break;
 	case LPFC_CGN_FPIN_BOTH:
-		scnprintf(tmp, sizeof(tmp),
-			  "FPIN:WARN|ALARM\n");
+		len += sysfs_emit_at(buf, len, "FPIN:WARN|ALARM\n");
 		break;
 	default:
-		scnprintf(tmp, sizeof(tmp),
-			  "FPIN:NONE\n");
+		len += sysfs_emit_at(buf, len, "FPIN:NONE\n");
 		break;
 	}
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
 
 	switch (phba->cgn_reg_signal) {
 	case EDC_CG_SIG_WARN_ONLY:
-		scnprintf(tmp, sizeof(tmp),
-			  "       Current:  Signal:WARN  ");
+		len += sysfs_emit_at(buf, len,
+				     "       Current:  Signal:WARN  ");
 		break;
 	case EDC_CG_SIG_WARN_ALARM:
-		scnprintf(tmp, sizeof(tmp),
-			  "       Current:  Signal:WARN|ALARM  ");
+		len += sysfs_emit_at(buf, len,
+				     "       Current:  Signal:WARN|ALARM  ");
 		break;
 	default:
-		scnprintf(tmp, sizeof(tmp),
-			  "       Current:  Signal:NONE  ");
+		len += sysfs_emit_at(buf, len,
+				     "       Current:  Signal:NONE  ");
 		break;
 	}
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
 
 	switch (phba->cgn_reg_fpin) {
 	case LPFC_CGN_FPIN_WARN:
-		scnprintf(tmp, sizeof(tmp),
-			  "FPIN:WARN  ACQEcnt:%d\n", phba->cgn_acqe_cnt);
+		len += sysfs_emit_at(buf, len,
+				     "FPIN:WARN  ACQEcnt:%d\n",
+				     phba->cgn_acqe_cnt);
 		break;
 	case LPFC_CGN_FPIN_ALARM:
-		scnprintf(tmp, sizeof(tmp),
-			  "FPIN:ALARM  ACQEcnt:%d\n", phba->cgn_acqe_cnt);
+		len += sysfs_emit_at(buf, len,
+				     "FPIN:ALARM  ACQEcnt:%d\n",
+				     phba->cgn_acqe_cnt);
 		break;
 	case LPFC_CGN_FPIN_BOTH:
-		scnprintf(tmp, sizeof(tmp),
-			  "FPIN:WARN|ALARM  ACQEcnt:%d\n", phba->cgn_acqe_cnt);
+		len += sysfs_emit_at(buf, len,
+				     "FPIN:WARN|ALARM  ACQEcnt:%d\n",
+				     phba->cgn_acqe_cnt);
 		break;
 	default:
-		scnprintf(tmp, sizeof(tmp),
-			  "FPIN:NONE  ACQEcnt:%d\n", phba->cgn_acqe_cnt);
+		len += sysfs_emit_at(buf, len,
+				     "FPIN:NONE  ACQEcnt:%d\n",
+				     phba->cgn_acqe_cnt);
 		break;
 	}
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
 
 	if (phba->cmf_active_mode != phba->cgn_p.cgn_param_mode) {
 		switch (phba->cmf_active_mode) {
 		case LPFC_CFG_OFF:
-			scnprintf(tmp, sizeof(tmp), "Active: Mode:Off\n");
+			len += sysfs_emit_at(buf, len, "Active: Mode:Off\n");
 			break;
 		case LPFC_CFG_MANAGED:
-			scnprintf(tmp, sizeof(tmp), "Active: Mode:Managed\n");
+			len += sysfs_emit_at(buf, len,
+					     "Active: Mode:Managed\n");
 			break;
 		case LPFC_CFG_MONITOR:
-			scnprintf(tmp, sizeof(tmp), "Active: Mode:Monitor\n");
+			len += sysfs_emit_at(buf, len,
+					     "Active: Mode:Monitor\n");
 			break;
 		default:
-			scnprintf(tmp, sizeof(tmp), "Active: Mode:Unknown\n");
+			len += sysfs_emit_at(buf, len,
+					     "Active: Mode:Unknown\n");
 		}
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
 	}
 
 	switch (phba->cgn_p.cgn_param_mode) {
 	case LPFC_CFG_OFF:
-		scnprintf(tmp, sizeof(tmp), "Config: Mode:Off  ");
+		len += sysfs_emit_at(buf, len, "Config: Mode:Off  ");
 		break;
 	case LPFC_CFG_MANAGED:
-		scnprintf(tmp, sizeof(tmp), "Config: Mode:Managed ");
+		len += sysfs_emit_at(buf, len, "Config: Mode:Managed ");
 		break;
 	case LPFC_CFG_MONITOR:
-		scnprintf(tmp, sizeof(tmp), "Config: Mode:Monitor ");
+		len += sysfs_emit_at(buf, len, "Config: Mode:Monitor ");
 		break;
 	default:
-		scnprintf(tmp, sizeof(tmp), "Config: Mode:Unknown ");
+		len += sysfs_emit_at(buf, len, "Config: Mode:Unknown ");
 	}
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
 
 	total = 0;
 	rcv = 0;
@@ -262,24 +249,18 @@ lpfc_cmf_info_show(struct device *dev, struct device_attribute *attr,
 		rcv += atomic64_read(&cgs->rcv_bytes);
 	}
 
-	scnprintf(tmp, sizeof(tmp),
-		  "IObusy:%d Info:%d Bytes: Rcv:x%llx Total:x%llx\n",
-		  atomic_read(&phba->cmf_busy),
-		  phba->cmf_active_info, rcv, total);
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
+	len += sysfs_emit_at(buf, len,
+			     "IObusy:%d Info:%d Bytes: Rcv:x%llx Total:x%llx\n",
+			     atomic_read(&phba->cmf_busy),
+			     phba->cmf_active_info, rcv, total);
 
-	scnprintf(tmp, sizeof(tmp),
-		  "Port_speed:%d  Link_byte_cnt:%ld  "
-		  "Max_byte_per_interval:%ld\n",
-		  lpfc_sli_port_speed_get(phba),
-		  (unsigned long)phba->cmf_link_byte_count,
-		  (unsigned long)phba->cmf_max_bytes_per_interval);
-	strlcat(buf, tmp, PAGE_SIZE);
+	len += sysfs_emit_at(buf, len,
+			     "Port_speed:%d  Link_byte_cnt:%ld  Max_byte_per_interval:%ld\n",
+			     lpfc_sli_port_speed_get(phba),
+			     (unsigned long)phba->cmf_link_byte_count,
+			     (unsigned long)phba->cmf_max_bytes_per_interval);
 
 buffer_done:
-	len = strnlen(buf, PAGE_SIZE);
-
 	if (unlikely(len >= (PAGE_SIZE - 1))) {
 		lpfc_printf_log(phba, KERN_INFO, LOG_CGN_MGMT,
 				"6312 Catching potential buffer "
@@ -480,17 +461,19 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
 	char *statep;
 	int i;
 	int len = 0;
-	char tmp[LPFC_MAX_INFO_TMP_LEN] = {0};
 
 	if (!(vport->cfg_enable_fc4_type & LPFC_ENABLE_NVME)) {
-		len = scnprintf(buf, PAGE_SIZE, "NVME Disabled\n");
+		len = sysfs_emit(buf, "NVME Disabled\n");
 		return len;
 	}
+
+	len = strnlen(buf, PAGE_SIZE);
+
 	if (phba->nvmet_support) {
 		if (!phba->targetport) {
-			len = scnprintf(buf, PAGE_SIZE,
-					"NVME Target: x%llx is not allocated\n",
-					wwn_to_u64(vport->fc_portname.u.wwn));
+			len = sysfs_emit(buf,
+					 "NVME Target: x%llx is not allocated\n",
+					 wwn_to_u64(vport->fc_portname.u.wwn));
 			return len;
 		}
 		/* Port state is only one of two values for now. */
@@ -498,167 +481,131 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
 			statep = "REGISTERED";
 		else
 			statep = "INIT";
-		scnprintf(tmp, sizeof(tmp),
-			  "NVME Target Enabled  State %s\n",
-			  statep);
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		scnprintf(tmp, sizeof(tmp),
-			  "%s%d WWPN x%llx WWNN x%llx DID x%06x\n",
-			  "NVME Target: lpfc",
-			  phba->brd_no,
-			  wwn_to_u64(vport->fc_portname.u.wwn),
-			  wwn_to_u64(vport->fc_nodename.u.wwn),
-			  phba->targetport->port_id);
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		if (strlcat(buf, "\nNVME Target: Statistics\n", PAGE_SIZE)
-		    >= PAGE_SIZE)
-			goto buffer_done;
+		len += sysfs_emit_at(buf, len,
+				     "NVME Target Enabled  State %s\n",
+				     statep);
+
+		len += sysfs_emit_at(buf, len,
+				     "%s%d WWPN x%llx WWNN x%llx DID x%06x\n",
+				     "NVME Target: lpfc",
+				     phba->brd_no,
+				     wwn_to_u64(vport->fc_portname.u.wwn),
+				     wwn_to_u64(vport->fc_nodename.u.wwn),
+				     phba->targetport->port_id);
+
+		len += sysfs_emit_at(buf, len, "\nNVME Target: Statistics\n");
 
 		tgtp = (struct lpfc_nvmet_tgtport *)phba->targetport->private;
-		scnprintf(tmp, sizeof(tmp),
-			  "LS: Rcv %08x Drop %08x Abort %08x\n",
-			  atomic_read(&tgtp->rcv_ls_req_in),
-			  atomic_read(&tgtp->rcv_ls_req_drop),
-			  atomic_read(&tgtp->xmt_ls_abort));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
+		len += sysfs_emit_at(buf, len,
+				     "LS: Rcv %08x Drop %08x Abort %08x\n",
+				     atomic_read(&tgtp->rcv_ls_req_in),
+				     atomic_read(&tgtp->rcv_ls_req_drop),
+				     atomic_read(&tgtp->xmt_ls_abort));
 
 		if (atomic_read(&tgtp->rcv_ls_req_in) !=
 		    atomic_read(&tgtp->rcv_ls_req_out)) {
-			scnprintf(tmp, sizeof(tmp),
-				  "Rcv LS: in %08x != out %08x\n",
-				  atomic_read(&tgtp->rcv_ls_req_in),
-				  atomic_read(&tgtp->rcv_ls_req_out));
-			if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-				goto buffer_done;
+			len += sysfs_emit_at(buf, len,
+					     "Rcv LS: in %08x != out %08x\n",
+					     atomic_read(&tgtp->rcv_ls_req_in),
+					     atomic_read(&tgtp->rcv_ls_req_out));
 		}
 
-		scnprintf(tmp, sizeof(tmp),
-			  "LS: Xmt %08x Drop %08x Cmpl %08x\n",
-			  atomic_read(&tgtp->xmt_ls_rsp),
-			  atomic_read(&tgtp->xmt_ls_drop),
-			  atomic_read(&tgtp->xmt_ls_rsp_cmpl));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		scnprintf(tmp, sizeof(tmp),
-			  "LS: RSP Abort %08x xb %08x Err %08x\n",
-			  atomic_read(&tgtp->xmt_ls_rsp_aborted),
-			  atomic_read(&tgtp->xmt_ls_rsp_xb_set),
-			  atomic_read(&tgtp->xmt_ls_rsp_error));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		scnprintf(tmp, sizeof(tmp),
-			  "FCP: Rcv %08x Defer %08x Release %08x "
-			  "Drop %08x\n",
-			  atomic_read(&tgtp->rcv_fcp_cmd_in),
-			  atomic_read(&tgtp->rcv_fcp_cmd_defer),
-			  atomic_read(&tgtp->xmt_fcp_release),
-			  atomic_read(&tgtp->rcv_fcp_cmd_drop));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
+		len += sysfs_emit_at(buf, len,
+				     "LS: Xmt %08x Drop %08x Cmpl %08x\n",
+				     atomic_read(&tgtp->xmt_ls_rsp),
+				     atomic_read(&tgtp->xmt_ls_drop),
+				     atomic_read(&tgtp->xmt_ls_rsp_cmpl));
+
+		len += sysfs_emit_at(buf, len,
+				     "LS: RSP Abort %08x xb %08x Err %08x\n",
+				     atomic_read(&tgtp->xmt_ls_rsp_aborted),
+				     atomic_read(&tgtp->xmt_ls_rsp_xb_set),
+				     atomic_read(&tgtp->xmt_ls_rsp_error));
+
+		len += sysfs_emit_at(buf, len,
+				     "FCP: Rcv %08x Defer %08x Release %08x Drop %08x\n",
+				     atomic_read(&tgtp->rcv_fcp_cmd_in),
+				     atomic_read(&tgtp->rcv_fcp_cmd_defer),
+				     atomic_read(&tgtp->xmt_fcp_release),
+				     atomic_read(&tgtp->rcv_fcp_cmd_drop));
 
 		if (atomic_read(&tgtp->rcv_fcp_cmd_in) !=
 		    atomic_read(&tgtp->rcv_fcp_cmd_out)) {
-			scnprintf(tmp, sizeof(tmp),
-				  "Rcv FCP: in %08x != out %08x\n",
-				  atomic_read(&tgtp->rcv_fcp_cmd_in),
-				  atomic_read(&tgtp->rcv_fcp_cmd_out));
-			if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-				goto buffer_done;
+			len += sysfs_emit_at(buf, len,
+					     "Rcv FCP: in %08x != out %08x\n",
+					     atomic_read(&tgtp->rcv_fcp_cmd_in),
+					     atomic_read(&tgtp->rcv_fcp_cmd_out));
 		}
 
-		scnprintf(tmp, sizeof(tmp),
-			  "FCP Rsp: RD %08x rsp %08x WR %08x rsp %08x "
-			  "drop %08x\n",
-			  atomic_read(&tgtp->xmt_fcp_read),
-			  atomic_read(&tgtp->xmt_fcp_read_rsp),
-			  atomic_read(&tgtp->xmt_fcp_write),
-			  atomic_read(&tgtp->xmt_fcp_rsp),
-			  atomic_read(&tgtp->xmt_fcp_drop));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		scnprintf(tmp, sizeof(tmp),
-			  "FCP Rsp Cmpl: %08x err %08x drop %08x\n",
-			  atomic_read(&tgtp->xmt_fcp_rsp_cmpl),
-			  atomic_read(&tgtp->xmt_fcp_rsp_error),
-			  atomic_read(&tgtp->xmt_fcp_rsp_drop));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		scnprintf(tmp, sizeof(tmp),
-			  "FCP Rsp Abort: %08x xb %08x xricqe  %08x\n",
-			  atomic_read(&tgtp->xmt_fcp_rsp_aborted),
-			  atomic_read(&tgtp->xmt_fcp_rsp_xb_set),
-			  atomic_read(&tgtp->xmt_fcp_xri_abort_cqe));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		scnprintf(tmp, sizeof(tmp),
-			  "ABORT: Xmt %08x Cmpl %08x\n",
-			  atomic_read(&tgtp->xmt_fcp_abort),
-			  atomic_read(&tgtp->xmt_fcp_abort_cmpl));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		scnprintf(tmp, sizeof(tmp),
-			  "ABORT: Sol %08x  Usol %08x Err %08x Cmpl %08x\n",
-			  atomic_read(&tgtp->xmt_abort_sol),
-			  atomic_read(&tgtp->xmt_abort_unsol),
-			  atomic_read(&tgtp->xmt_abort_rsp),
-			  atomic_read(&tgtp->xmt_abort_rsp_error));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-
-		scnprintf(tmp, sizeof(tmp),
-			  "DELAY: ctx %08x  fod %08x wqfull %08x\n",
-			  atomic_read(&tgtp->defer_ctx),
-			  atomic_read(&tgtp->defer_fod),
-			  atomic_read(&tgtp->defer_wqfull));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
+		len += sysfs_emit_at(buf, len,
+				     "FCP Rsp: RD %08x rsp %08x WR %08x rsp %08x drop %08x\n",
+				     atomic_read(&tgtp->xmt_fcp_read),
+				     atomic_read(&tgtp->xmt_fcp_read_rsp),
+				     atomic_read(&tgtp->xmt_fcp_write),
+				     atomic_read(&tgtp->xmt_fcp_rsp),
+				     atomic_read(&tgtp->xmt_fcp_drop));
+
+		len += sysfs_emit_at(buf, len,
+				     "FCP Rsp Cmpl: %08x err %08x drop %08x\n",
+				     atomic_read(&tgtp->xmt_fcp_rsp_cmpl),
+				     atomic_read(&tgtp->xmt_fcp_rsp_error),
+				     atomic_read(&tgtp->xmt_fcp_rsp_drop));
+
+		len += sysfs_emit_at(buf, len,
+				     "FCP Rsp Abort: %08x xb %08x xricqe  %08x\n",
+				     atomic_read(&tgtp->xmt_fcp_rsp_aborted),
+				     atomic_read(&tgtp->xmt_fcp_rsp_xb_set),
+				     atomic_read(&tgtp->xmt_fcp_xri_abort_cqe));
+
+		len += sysfs_emit_at(buf, len,
+				     "ABORT: Xmt %08x Cmpl %08x\n",
+				     atomic_read(&tgtp->xmt_fcp_abort),
+				     atomic_read(&tgtp->xmt_fcp_abort_cmpl));
+
+		len += sysfs_emit_at(buf, len,
+				     "ABORT: Sol %08x  Usol %08x Err %08x Cmpl %08x\n",
+				     atomic_read(&tgtp->xmt_abort_sol),
+				     atomic_read(&tgtp->xmt_abort_unsol),
+				     atomic_read(&tgtp->xmt_abort_rsp),
+				     atomic_read(&tgtp->xmt_abort_rsp_error));
+
+		len += sysfs_emit_at(buf, len,
+				     "DELAY: ctx %08x  fod %08x wqfull %08x\n",
+				     atomic_read(&tgtp->defer_ctx),
+				     atomic_read(&tgtp->defer_fod),
+				     atomic_read(&tgtp->defer_wqfull));
 
 		/* Calculate outstanding IOs */
 		tot = atomic_read(&tgtp->rcv_fcp_cmd_drop);
 		tot += atomic_read(&tgtp->xmt_fcp_release);
 		tot = atomic_read(&tgtp->rcv_fcp_cmd_in) - tot;
 
-		scnprintf(tmp, sizeof(tmp),
-			  "IO_CTX: %08x  WAIT: cur %08x tot %08x\n"
-			  "CTX Outstanding %08llx\n\n",
-			  phba->sli4_hba.nvmet_xri_cnt,
-			  phba->sli4_hba.nvmet_io_wait_cnt,
-			  phba->sli4_hba.nvmet_io_wait_total,
-			  tot);
-		strlcat(buf, tmp, PAGE_SIZE);
+		len += sysfs_emit_at(buf, len,
+				     "IO_CTX: %08x  WAIT: cur %08x tot %08x\n"
+				     "CTX Outstanding %08llx\n\n",
+				     phba->sli4_hba.nvmet_xri_cnt,
+				     phba->sli4_hba.nvmet_io_wait_cnt,
+				     phba->sli4_hba.nvmet_io_wait_total,
+				     tot);
 		goto buffer_done;
 	}
 
 	localport = vport->localport;
 	if (!localport) {
-		len = scnprintf(buf, PAGE_SIZE,
-				"NVME Initiator x%llx is not allocated\n",
-				wwn_to_u64(vport->fc_portname.u.wwn));
+		len = sysfs_emit(buf,
+				 "NVME Initiator x%llx is not allocated\n",
+				 wwn_to_u64(vport->fc_portname.u.wwn));
 		return len;
 	}
 	lport = (struct lpfc_nvme_lport *)localport->private;
-	if (strlcat(buf, "\nNVME Initiator Enabled\n", PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
+	len += sysfs_emit_at(buf, len, "\nNVME Initiator Enabled\n");
 
-	scnprintf(tmp, sizeof(tmp),
-		  "XRI Dist lpfc%d Total %d IO %d ELS %d\n",
-		  phba->brd_no,
-		  phba->sli4_hba.max_cfg_param.max_xri,
-		  phba->sli4_hba.io_xri_max,
-		  lpfc_sli4_get_els_iocb_cnt(phba));
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
+	len += sysfs_emit_at(buf, len,
+			     "XRI Dist lpfc%d Total %d IO %d ELS %d\n",
+			     phba->brd_no,
+			     phba->sli4_hba.max_cfg_param.max_xri,
+			     phba->sli4_hba.io_xri_max,
+			     lpfc_sli4_get_els_iocb_cnt(phba));
 
 	/* Port state is only one of two values for now. */
 	if (localport->port_id)
@@ -666,15 +613,13 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
 	else
 		statep = "UNKNOWN ";
 
-	scnprintf(tmp, sizeof(tmp),
-		  "%s%d WWPN x%llx WWNN x%llx DID x%06x %s\n",
-		  "NVME LPORT lpfc",
-		  phba->brd_no,
-		  wwn_to_u64(vport->fc_portname.u.wwn),
-		  wwn_to_u64(vport->fc_nodename.u.wwn),
-		  localport->port_id, statep);
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
+	len += sysfs_emit_at(buf, len,
+			     "%s%d WWPN x%llx WWNN x%llx DID x%06x %s\n",
+			     "NVME LPORT lpfc",
+			     phba->brd_no,
+			     wwn_to_u64(vport->fc_portname.u.wwn),
+			     wwn_to_u64(vport->fc_nodename.u.wwn),
+			     localport->port_id, statep);
 
 	spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
 
@@ -702,77 +647,55 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
 		}
 
 		/* Tab in to show lport ownership. */
-		if (strlcat(buf, "NVME RPORT       ", PAGE_SIZE) >= PAGE_SIZE)
-			goto unlock_buf_done;
-		if (phba->brd_no >= 10) {
-			if (strlcat(buf, " ", PAGE_SIZE) >= PAGE_SIZE)
-				goto unlock_buf_done;
-		}
+		len += sysfs_emit_at(buf, len, "NVME RPORT       ");
+		if (phba->brd_no >= 10)
+			len += sysfs_emit_at(buf, len, " ");
 
-		scnprintf(tmp, sizeof(tmp), "WWPN x%llx ",
-			  nrport->port_name);
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto unlock_buf_done;
+		len += sysfs_emit_at(buf, len, "WWPN x%llx ",
+				     nrport->port_name);
 
-		scnprintf(tmp, sizeof(tmp), "WWNN x%llx ",
-			  nrport->node_name);
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto unlock_buf_done;
+		len += sysfs_emit_at(buf, len, "WWNN x%llx ",
+				     nrport->node_name);
 
-		scnprintf(tmp, sizeof(tmp), "DID x%06x ",
-			  nrport->port_id);
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto unlock_buf_done;
+		len += sysfs_emit_at(buf, len, "DID x%06x ",
+				     nrport->port_id);
 
 		/* An NVME rport can have multiple roles. */
-		if (nrport->port_role & FC_PORT_ROLE_NVME_INITIATOR) {
-			if (strlcat(buf, "INITIATOR ", PAGE_SIZE) >= PAGE_SIZE)
-				goto unlock_buf_done;
-		}
-		if (nrport->port_role & FC_PORT_ROLE_NVME_TARGET) {
-			if (strlcat(buf, "TARGET ", PAGE_SIZE) >= PAGE_SIZE)
-				goto unlock_buf_done;
-		}
-		if (nrport->port_role & FC_PORT_ROLE_NVME_DISCOVERY) {
-			if (strlcat(buf, "DISCSRVC ", PAGE_SIZE) >= PAGE_SIZE)
-				goto unlock_buf_done;
-		}
+		if (nrport->port_role & FC_PORT_ROLE_NVME_INITIATOR)
+			len += sysfs_emit_at(buf, len, "INITIATOR ");
+		if (nrport->port_role & FC_PORT_ROLE_NVME_TARGET)
+			len += sysfs_emit_at(buf, len, "TARGET ");
+		if (nrport->port_role & FC_PORT_ROLE_NVME_DISCOVERY)
+			len += sysfs_emit_at(buf, len, "DISCSRVC ");
 		if (nrport->port_role & ~(FC_PORT_ROLE_NVME_INITIATOR |
 					  FC_PORT_ROLE_NVME_TARGET |
-					  FC_PORT_ROLE_NVME_DISCOVERY)) {
-			scnprintf(tmp, sizeof(tmp), "UNKNOWN ROLE x%x",
-				  nrport->port_role);
-			if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-				goto unlock_buf_done;
-		}
+					  FC_PORT_ROLE_NVME_DISCOVERY))
+			len += sysfs_emit_at(buf, len, "UNKNOWN ROLE x%x",
+					     nrport->port_role);
+
+		len += sysfs_emit_at(buf, len, "%s\n", statep);
 
-		scnprintf(tmp, sizeof(tmp), "%s\n", statep);
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto unlock_buf_done;
+		if (len >= PAGE_SIZE - 1)
+			break;
 	}
 	spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
 
 	if (!lport)
 		goto buffer_done;
 
-	if (strlcat(buf, "\nNVME Statistics\n", PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
+	len += sysfs_emit_at(buf, len, "\nNVME Statistics\n");
 
-	scnprintf(tmp, sizeof(tmp),
-		  "LS: Xmt %010x Cmpl %010x Abort %08x\n",
-		  atomic_read(&lport->fc4NvmeLsRequests),
-		  atomic_read(&lport->fc4NvmeLsCmpls),
-		  atomic_read(&lport->xmt_ls_abort));
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
+	len += sysfs_emit_at(buf, len,
+			     "LS: Xmt %010x Cmpl %010x Abort %08x\n",
+			     atomic_read(&lport->fc4NvmeLsRequests),
+			     atomic_read(&lport->fc4NvmeLsCmpls),
+			     atomic_read(&lport->xmt_ls_abort));
 
-	scnprintf(tmp, sizeof(tmp),
-		  "LS XMIT: Err %08x  CMPL: xb %08x Err %08x\n",
-		  atomic_read(&lport->xmt_ls_err),
-		  atomic_read(&lport->cmpl_ls_xb),
-		  atomic_read(&lport->cmpl_ls_err));
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
+	len += sysfs_emit_at(buf, len,
+			     "LS XMIT: Err %08x  CMPL: xb %08x Err %08x\n",
+			     atomic_read(&lport->xmt_ls_err),
+			     atomic_read(&lport->cmpl_ls_xb),
+			     atomic_read(&lport->cmpl_ls_err));
 
 	totin = 0;
 	totout = 0;
@@ -785,40 +708,25 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
 		data3 = cstat->control_requests;
 		totout += (data1 + data2 + data3);
 	}
-	scnprintf(tmp, sizeof(tmp),
-		  "Total FCP Cmpl %016llx Issue %016llx "
-		  "OutIO %016llx\n",
-		  totin, totout, totout - totin);
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
-
-	scnprintf(tmp, sizeof(tmp),
-		  "\tabort %08x noxri %08x nondlp %08x qdepth %08x "
-		  "wqerr %08x err %08x\n",
-		  atomic_read(&lport->xmt_fcp_abort),
-		  atomic_read(&lport->xmt_fcp_noxri),
-		  atomic_read(&lport->xmt_fcp_bad_ndlp),
-		  atomic_read(&lport->xmt_fcp_qdepth),
-		  atomic_read(&lport->xmt_fcp_wqerr),
-		  atomic_read(&lport->xmt_fcp_err));
-	if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-		goto buffer_done;
-
-	scnprintf(tmp, sizeof(tmp),
-		  "FCP CMPL: xb %08x Err %08x\n",
-		  atomic_read(&lport->cmpl_fcp_xb),
-		  atomic_read(&lport->cmpl_fcp_err));
-	strlcat(buf, tmp, PAGE_SIZE);
-
-	/* host_lock is already unlocked. */
-	goto buffer_done;
-
- unlock_buf_done:
-	spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
+	len += sysfs_emit_at(buf, len,
+			     "Total FCP Cmpl %016llx Issue %016llx OutIO %016llx\n",
+			     totin, totout, totout - totin);
+
+	len += sysfs_emit_at(buf, len,
+			     "\tabort %08x noxri %08x nondlp %08x qdepth %08x wqerr %08x err %08x\n",
+			     atomic_read(&lport->xmt_fcp_abort),
+			     atomic_read(&lport->xmt_fcp_noxri),
+			     atomic_read(&lport->xmt_fcp_bad_ndlp),
+			     atomic_read(&lport->xmt_fcp_qdepth),
+			     atomic_read(&lport->xmt_fcp_wqerr),
+			     atomic_read(&lport->xmt_fcp_err));
+
+	len += sysfs_emit_at(buf, len,
+			     "FCP CMPL: xb %08x Err %08x\n",
+			     atomic_read(&lport->cmpl_fcp_xb),
+			     atomic_read(&lport->cmpl_fcp_err));
 
  buffer_done:
-	len = strnlen(buf, PAGE_SIZE);
-
 	if (unlikely(len >= (PAGE_SIZE - 1))) {
 		lpfc_printf_log(phba, KERN_INFO, LOG_NVME,
 				"6314 Catching potential buffer "
@@ -844,13 +752,12 @@ lpfc_scsi_stat_show(struct device *dev, struct device_attribute *attr,
 	u64 data1, data2, data3;
 	u64 tot, totin, totout;
 	int i;
-	char tmp[LPFC_MAX_SCSI_INFO_TMP_LEN] = {0};
 
 	if (!(vport->cfg_enable_fc4_type & LPFC_ENABLE_FCP) ||
 	    (phba->sli_rev != LPFC_SLI_REV4))
 		return 0;
 
-	scnprintf(buf, PAGE_SIZE, "SCSI HDWQ Statistics\n");
+	len = sysfs_emit(buf, "SCSI HDWQ Statistics\n");
 
 	totin = 0;
 	totout = 0;
@@ -863,22 +770,20 @@ lpfc_scsi_stat_show(struct device *dev, struct device_attribute *attr,
 		data3 = cstat->control_requests;
 		totout += (data1 + data2 + data3);
 
-		scnprintf(tmp, sizeof(tmp), "HDWQ (%d): Rd %016llx Wr %016llx "
-			  "IO %016llx ", i, data1, data2, data3);
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
+		len += sysfs_emit_at(buf, len,
+				     "HDWQ (%d): Rd %016llx Wr %016llx IO %016llx ",
+				     i, data1, data2, data3);
 
-		scnprintf(tmp, sizeof(tmp), "Cmpl %016llx OutIO %016llx\n",
-			  tot, ((data1 + data2 + data3) - tot));
-		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
-			goto buffer_done;
-	}
-	scnprintf(tmp, sizeof(tmp), "Total FCP Cmpl %016llx Issue %016llx "
-		  "OutIO %016llx\n", totin, totout, totout - totin);
-	strlcat(buf, tmp, PAGE_SIZE);
+		len += sysfs_emit_at(buf, len,
+				     "Cmpl %016llx OutIO %016llx\n",
+				     tot, ((data1 + data2 + data3) - tot));
 
-buffer_done:
-	len = strnlen(buf, PAGE_SIZE);
+		if (len >= PAGE_SIZE - 1)
+			break;
+	}
+	len += sysfs_emit_at(buf, len,
+			     "Total FCP Cmpl %016llx Issue %016llx OutIO %016llx\n",
+			     totin, totout, totout - totin);
 
 	return len;
 }
diff --git a/drivers/scsi/lpfc/lpfc_scsi.h b/drivers/scsi/lpfc/lpfc_scsi.h
index a05d203e4777..3e0937cf3ff3 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.h
+++ b/drivers/scsi/lpfc/lpfc_scsi.h
@@ -158,6 +158,3 @@ struct fcp_cmnd32 {
 
 #define TXRDY_PAYLOAD_LEN	12
 
-/* For sysfs/debugfs tmp string max len */
-#define LPFC_MAX_SCSI_INFO_TMP_LEN	79
-
-- 
2.47.3

Re: [PATCH 1/5] scsi: lpfc: Replace strlcat() with seq_buf in lpfc_info()

[sashiko-bot]

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

Pre-existing issues:
- [High] Unsafe lockless iteration over the `pdev->bus->devices` list causes use-after-free vulnerabilities.
--

--- Patch [1]: [PATCH 1/5] scsi: lpfc: Replace strlcat() with seq_buf in lpfc_info() ---
commit f8c11b7b237ed1d074a50074665c19f1c5567a78
Author: Ian Bridges <icb@fastmail.org>
Subject: scsi: lpfc: Replace strlcat() with seq_buf in lpfc_info()

This commit replaces strlcat() and intermediate buffers with the seq_buf API
in lpfc_info(). This simplifies string construction while maintaining memory
safety and silent truncation behavior.

Link: https://github.com/KSPP/linux/issues/370 [1]

> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index f2cab134af7f1..8a795c65e3c3e 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
[ ... ]
> @@ -5103,57 +5104,37 @@ lpfc_info(struct Scsi_Host *host)
[ ... ]
>  		/* PCI resettable */
> -		if (!lpfc_check_pci_resettable(phba)) {
> -			scnprintf(tmp, sizeof(tmp), " PCI resettable");
> -			strlcat(lpfcinfobuf, tmp, sizeof(lpfcinfobuf));
> -		}
> +		if (!lpfc_check_pci_resettable(phba))

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call
lpfc_check_pci_resettable() without holding the pci_bus_sem read lock?

Looking at lpfc_check_pci_resettable() in drivers/scsi/lpfc/lpfc_scsi.c,
it iterates over the pdev->bus->devices list:

drivers/scsi/lpfc/lpfc_scsi.c:lpfc_check_pci_resettable() {
    ...
	/* Walk the list of devices on the pci_dev's bus */
	list_for_each_entry(ptr, &pdev->bus->devices, bus_list) {
		/* Check for Emulex Vendor ID */
    ...
}

If a concurrent PCI hot-remove operation occurs, could the bus_list pointers
be freed while list_for_each_entry is following the next pointer, leading
to a use-after-free?

> +			seq_buf_printf(&s, " PCI resettable");
>  	}
>  
> -buffer_done:
>  	return lpfcinfobuf;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729144617.1388646-1-icb@fastmail.org?part=1

Re: [PATCH 2/5] scsi: lpfc: Replace strlcat() with scnprintf() in lpfc_vport_symbolic_node_name()

[sashiko-bot]

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

Pre-existing issues:
- [High] Pre-existing out-of-bounds read and stack buffer overflow in `lpfc_decode_firmware_rev` due to partial array initialization and missing null-termination.
--

--- Patch [2]: [PATCH 2/5] scsi: lpfc: Replace strlcat() with scnprintf() in lpfc_vport_symbolic_node_name() ---
commit ccac320efc7231c941f0ae4e81fa116ae23ad75e
Author: Ian Bridges <icb@fastmail.org>

scsi: lpfc: Replace strlcat() with scnprintf() in lpfc_vport_symbolic_node_name()

This commit replaces strlcat() API uses with a single scnprintf() in
lpfc_vport_symbolic_node_name(). The replacement formats all fragments
directly into the destination buffer, avoiding intermediate truncation.

Link: https://github.com/KSPP/linux/issues/370 [1]

> diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
> index c7853e7fe0719..0734ab3be3e30 100644
> --- a/drivers/scsi/lpfc/lpfc_ct.c
> +++ b/drivers/scsi/lpfc/lpfc_ct.c
> @@ -1823,34 +1823,15 @@ lpfc_vport_symbolic_node_name(struct lpfc_vport *vport, char *symbol,
>  	size_t size)
>  {
>  	char fwrev[FW_REV_STR_SIZE] = {0};
> -	char tmp[MAXHOSTNAMELEN] = {0};
> -
> -	memset(symbol, 0, size);
> -
> -	scnprintf(tmp, sizeof(tmp), "Emulex %s", vport->phba->ModelName);
> -	if (strlcat(symbol, tmp, size) >= size)
> -		goto buffer_done;
>  
>  	lpfc_decode_firmware_rev(vport->phba, fwrev, 0);

[Severity: High]
This is a pre-existing issue, but can lpfc_decode_firmware_rev() cause a
stack buffer overflow when called from other paths with flag = 1?

Looking at lpfc_decode_firmware_rev() in drivers/scsi/lpfc/lpfc_ct.c, the
stack array str is only partially initialized:

drivers/scsi/lpfc/lpfc_ct.c:lpfc_decode_firmware_rev() {
        uint32_t *ptr, str[4];
        ...
                for (i = 0; i < 3; i++)
                        str[i] = be32_to_cpu(*ptr++);

                if (c == 0) {
                        if (flag)
                                sprintf(fwrevision, "%d.%d%d (%s)",
                                        b1, b2, b3, (char *)str);
                        else
...
}

Only 3 elements (12 bytes) of the 4-element str array are initialized.

If the firmware name does not contain any spaces to be converted to null
bytes in the first 12 bytes, won't sprintf() read past str[2] into
uninitialized stack memory?

Since this reads until a null byte is encountered, couldn't this overflow
the caller's fwrevision destination buffer?

This appears reachable when triggered via sysfs where untrusted device
firmware might provide maliciously crafted data.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729144617.1388646-1-icb@fastmail.org?part=2

Re: [PATCH 5/5] scsi: lpfc: Replace strlcat() with sysfs_emit_at() in the sysfs show functions

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-after-free of `nrport` because it is dereferenced outside of its protecting lock.
--

--- Patch [5]: [PATCH 5/5] scsi: lpfc: Replace strlcat() with sysfs_emit_at() in the sysfs show functions ---
commit b85263748e0a979a8db2d3fc65264e450144763e
Author: Ian Bridges <icb@fastmail.org>
scsi: lpfc: Replace strlcat() with sysfs_emit_at() in the sysfs show functions

This commit replaces strlcat() with sysfs_emit_at() in several lpfc sysfs
show functions. It also removes the intermediate tmp buffers and per-fragment
overflow checks.

Link: https://github.com/KSPP/linux/issues/370 [1]

> diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
> --- a/drivers/scsi/lpfc/lpfc_attr.c
> +++ b/drivers/scsi/lpfc/lpfc_attr.c

[ ... ]

> @@ -702,77 +647,55 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
>  		}
>  
>  		/* Tab in to show lport ownership. */
> -		if (strlcat(buf, "NVME RPORT       ", PAGE_SIZE) >= PAGE_SIZE)
> -			goto unlock_buf_done;
> -		if (phba->brd_no >= 10) {
> -			if (strlcat(buf, " ", PAGE_SIZE) >= PAGE_SIZE)
> -				goto unlock_buf_done;
> -		}
> +		len += sysfs_emit_at(buf, len, "NVME RPORT       ");
> +		if (phba->brd_no >= 10)
> +			len += sysfs_emit_at(buf, len, " ");
>  
> -		scnprintf(tmp, sizeof(tmp), "WWPN x%llx ",
> -			  nrport->port_name);
> -		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
> -			goto unlock_buf_done;
> +		len += sysfs_emit_at(buf, len, "WWPN x%llx ",
> +				     nrport->port_name);

[Severity: High]
This is a pre-existing issue, but could this result in a use-after-free of the
nrport pointer?

Looking at lpfc_nvme_info_show() in drivers/scsi/lpfc/lpfc_attr.c, the nrport
pointer is fetched from rport->remoteport under ndlp->lock. However, the lock
is released right before nrport is dereferenced to access fields like
port_state, port_name, node_name, and port_id.

If lpfc_nvme_unregister_port() runs concurrently (such as during a connection
reset or hot-unplug), it explicitly does not hold fc_nodes_list_lock when
retrieving rport->remoteport and calling nvme_fc_unregister_remoteport().

Does anything prevent the remote port from being unregistered and freed by the
NVMe FC transport while this sysfs file is being read, potentially leaving
nrport as a dangling pointer here?

> -		scnprintf(tmp, sizeof(tmp), "WWNN x%llx ",
> -			  nrport->node_name);
> -		if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
> -			goto unlock_buf_done;
> +		len += sysfs_emit_at(buf, len, "WWNN x%llx ",
> +				     nrport->node_name);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729144617.1388646-1-icb@fastmail.org?part=5