Linux Hardening
 help / color / mirror / Atom feed
From: Ian Bridges <icb@fastmail.org>
To: Justin Tee <justin.tee@broadcom.com>,
	Paul Ely <paul.ely@broadcom.com>,
	"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org, Kees Cook <kees@kernel.org>,
	Ian Bridges <icb@fastmail.org>
Subject: [PATCH 1/5] scsi: lpfc: Replace strlcat() with seq_buf in lpfc_info()
Date: Wed, 29 Jul 2026 09:46:13 -0500	[thread overview]
Message-ID: <20260729144617.1388646-2-icb@fastmail.org> (raw)
In-Reply-To: <20260729144617.1388646-1-icb@fastmail.org>

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


  reply	other threads:[~2026-07-29 14:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29 14:46 [PATCH 0/5] scsi: lpfc: Remove all strlcat() uses Ian Bridges
2026-07-29 14:46 ` Ian Bridges [this message]
2026-07-29 14:46 ` [PATCH 2/5] scsi: lpfc: Replace strlcat() with scnprintf() in lpfc_vport_symbolic_node_name() Ian Bridges
2026-07-29 14:46 ` [PATCH 3/5] scsi: lpfc: Replace strlcat() with seq_buf in lpfc_rx_monitor_report() Ian Bridges
2026-07-29 14:46 ` [PATCH 4/5] scsi: lpfc: Replace strlcat() with seq_buf in the debugfs dump helpers Ian Bridges
2026-07-29 14:46 ` [PATCH 5/5] scsi: lpfc: Replace strlcat() with sysfs_emit_at() in the sysfs show functions Ian Bridges
2026-08-14  2:17 ` [PATCH 0/5] scsi: lpfc: Remove all strlcat() uses Martin K. Petersen (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260729144617.1388646-2-icb@fastmail.org \
    --to=icb@fastmail.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=justin.tee@broadcom.com \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=paul.ely@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox