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

[Kees Cook <kees@kernel.org>]

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