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

[David Laight <david.laight.linux@gmail.com>]

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 "