[PATCH v5 03/12] scsi: lpfc: Print rx monitor records straight into the seq_buf

[PATCH v5 03/12] scsi: lpfc: Print rx monitor records straight into the seq_buf

[Kees Cook]

lpfc_rx_monitor_report() formats each record into a stack buffer, then
appends it with seq_buf_puts(), relying on seq_buf_puts() appending
nothing when a string does not fit: the debugfs output then ends at the
last whole record, and the record left out is read in full next time.
The next patch makes seq_buf_puts() copy as much as fits instead, as
seq_buf_printf() and strlcat() do.

Print each record with seq_buf_printf(), and when it does not fit, end
the string where the record began, since the reader takes the buffer up
to its NUL. This also takes the DBG_LOG_STR_SZ buffer off the stack.

Build tested ARCH=x86_64 with GCC 16.2.0, CONFIG_SCSI_LPFC=m and W=1.

Assisted-by: LLM
Signed-off-by: Kees Cook <kees@kernel.org>
---
 drivers/scsi/lpfc/lpfc_sli.c | 45 +++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index cfa4371169f1..a7b1ea67ed98 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -8108,7 +8108,6 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
 	spinlock_t *ring_lock = &rx_monitor->lock;
 	u32 ring_size = rx_monitor->entries;
 	u32 cnt = 0;
-	char tmp[DBG_LOG_STR_SZ] = {0};
 	bool log_to_kmsg = (!buf || !buf_len) ? true : false;
 	struct seq_buf s;
 
@@ -8133,27 +8132,37 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
 
 		/* Read out this entry's data. */
 		if (!log_to_kmsg) {
+			unsigned int len;
+
+			/* A header that did not fit leaves no room. */
+			if (seq_buf_has_overflowed(&s))
+				break;
+			len = seq_buf_used(&s);
+
+			seq_buf_printf(&s,
+				       "%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->timer_interval);
+
 			/*
 			 * Drop a record whole if it does not fit, without
-			 * consuming its ring entry.
+			 * consuming its ring entry: the reader takes the
+			 * string up to its NUL.
 			 */
-			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,
-				  entry->cmf_busy, entry->io_cnt,
-				  entry->cmf_info,
-				  entry->timer_utilization,
-				  entry->timer_interval);
-
-			if (seq_buf_puts(&s, tmp) < 0)
+			if (seq_buf_has_overflowed(&s)) {
+				buf[len] = '\0';
 				break;
+			}
 		} else {
 			lpfc_printf_log(phba, KERN_INFO, LOG_CGN_MGMT,
 					"4410 %02u: MBPI %llu Xmit %llu "
-- 
2.55.0

Re: [PATCH v5 03/12] scsi: lpfc: Print rx monitor records straight into the seq_buf

[David Laight]

On Mon,  5 Oct 2026 08:56:53 -0700
Kees Cook <kees@kernel.org> wrote:

> lpfc_rx_monitor_report() formats each record into a stack buffer, then
> appends it with seq_buf_puts(), relying on seq_buf_puts() appending
> nothing when a string does not fit: the debugfs output then ends at the
> last whole record, and the record left out is read in full next time.
> The next patch makes seq_buf_puts() copy as much as fits instead, as
> seq_buf_printf() and strlcat() do.
> 
> Print each record with seq_buf_printf(), and when it does not fit, end
> the string where the record began, since the reader takes the buffer up
> to its NUL. This also takes the DBG_LOG_STR_SZ buffer off the stack.
> 
> Build tested ARCH=x86_64 with GCC 16.2.0, CONFIG_SCSI_LPFC=m and W=1.
> 
> Assisted-by: LLM
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>  drivers/scsi/lpfc/lpfc_sli.c | 45 +++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index cfa4371169f1..a7b1ea67ed98 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -8108,7 +8108,6 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
>  	spinlock_t *ring_lock = &rx_monitor->lock;
>  	u32 ring_size = rx_monitor->entries;
>  	u32 cnt = 0;
> -	char tmp[DBG_LOG_STR_SZ] = {0};
>  	bool log_to_kmsg = (!buf || !buf_len) ? true : false;
>  	struct seq_buf s;
>  
> @@ -8133,27 +8132,37 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
>  
>  		/* Read out this entry's data. */
>  		if (!log_to_kmsg) {
> +			unsigned int len;
> +
> +			/* A header that did not fit leaves no room. */
> +			if (seq_buf_has_overflowed(&s))
> +				break;
> +			len = seq_buf_used(&s);
> +
> +			seq_buf_printf(&s,
> +				       "%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->timer_interval);
> +
>  			/*
>  			 * Drop a record whole if it does not fit, without
> -			 * consuming its ring entry.
> +			 * consuming its ring entry: the reader takes the
> +			 * string up to its NUL.
>  			 */
> -			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,
> -				  entry->cmf_busy, entry->io_cnt,
> -				  entry->cmf_info,
> -				  entry->timer_utilization,
> -				  entry->timer_interval);
> -
> -			if (seq_buf_puts(&s, tmp) < 0)
> +			if (seq_buf_has_overflowed(&s)) {
> +				buf[len] = '\0';

There should probably be a seq_buf_truncate() to do that.
Then a request for the length will be correct.

David 

>  				break;
> +			}
>  		} else {
>  			lpfc_printf_log(phba, KERN_INFO, LOG_CGN_MGMT,
>  					"4410 %02u: MBPI %llu Xmit %llu "

Re: [PATCH v5 03/12] scsi: lpfc: Print rx monitor records straight into the seq_buf

[Kees Cook]

On Mon, Oct 05, 2026 at 07:54:11PM +0100, David Laight wrote:
> On Mon,  5 Oct 2026 08:56:53 -0700
> Kees Cook <kees@kernel.org> wrote:
> 
> > lpfc_rx_monitor_report() formats each record into a stack buffer, then
> > appends it with seq_buf_puts(), relying on seq_buf_puts() appending
> > nothing when a string does not fit: the debugfs output then ends at the
> > last whole record, and the record left out is read in full next time.
> > The next patch makes seq_buf_puts() copy as much as fits instead, as
> > seq_buf_printf() and strlcat() do.
> > 
> > Print each record with seq_buf_printf(), and when it does not fit, end
> > the string where the record began, since the reader takes the buffer up
> > to its NUL. This also takes the DBG_LOG_STR_SZ buffer off the stack.
> > 
> > Build tested ARCH=x86_64 with GCC 16.2.0, CONFIG_SCSI_LPFC=m and W=1.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Kees Cook <kees@kernel.org>
> > ---
> >  drivers/scsi/lpfc/lpfc_sli.c | 45 +++++++++++++++++++++---------------
> >  1 file changed, 27 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> > index cfa4371169f1..a7b1ea67ed98 100644
> > --- a/drivers/scsi/lpfc/lpfc_sli.c
> > +++ b/drivers/scsi/lpfc/lpfc_sli.c
> > @@ -8108,7 +8108,6 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
> >  	spinlock_t *ring_lock = &rx_monitor->lock;
> >  	u32 ring_size = rx_monitor->entries;
> >  	u32 cnt = 0;
> > -	char tmp[DBG_LOG_STR_SZ] = {0};
> >  	bool log_to_kmsg = (!buf || !buf_len) ? true : false;
> >  	struct seq_buf s;
> >  
> > @@ -8133,27 +8132,37 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
> >  
> >  		/* Read out this entry's data. */
> >  		if (!log_to_kmsg) {
> > +			unsigned int len;
> > +
> > +			/* A header that did not fit leaves no room. */
> > +			if (seq_buf_has_overflowed(&s))
> > +				break;
> > +			len = seq_buf_used(&s);
> > +
> > +			seq_buf_printf(&s,
> > +				       "%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->timer_interval);
> > +
> >  			/*
> >  			 * Drop a record whole if it does not fit, without
> > -			 * consuming its ring entry.
> > +			 * consuming its ring entry: the reader takes the
> > +			 * string up to its NUL.
> >  			 */
> > -			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,
> > -				  entry->cmf_busy, entry->io_cnt,
> > -				  entry->cmf_info,
> > -				  entry->timer_utilization,
> > -				  entry->timer_interval);
> > -
> > -			if (seq_buf_puts(&s, tmp) < 0)
> > +			if (seq_buf_has_overflowed(&s)) {
> > +				buf[len] = '\0';
> 
> There should probably be a seq_buf_truncate() to do that.
> Then a request for the length will be correct.

We don't really have a "rewind" API. (Oh please don't make me add
another API ... I'm at 3 already in this series...)


-- 
Kees Cook