scsi: lpfc: Remove all strlcat() uses
Subsystems (2)
cover scsi: lpfc: Remove all strlcat() uses
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(-)
Signature · 1 line
2.47.3
PATCH 1/5 scsi: lpfc: Replace strlcat() with seq_buf in lpfc_info()
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.
---
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(-)
drivers/scsi/lpfc/lpfc_scsi.c +15 -34
@@ -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; }
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 1/5] scsi: lpfc: Replace strlcat() with seq_buf in lpfc_info()
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 4 lines
drivers/scsi/lpfc/lpfc_scsi.c +0
-0
[ ... ]
Quoted 1 line
drivers/scsi/lpfc/lpfc_scsi.c +0
-0@@ -5103,57 +5104,37 @@ lpfc_info(struct Scsi_Host *host)
[ ... ]
drivers/scsi/lpfc/lpfc_scsi.c lpfc_info(struct Scsi_Host *host) · line 5134
Quoted 6 lines
drivers/scsi/lpfc/lpfc_scsi.c +1
-4/* 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?
Quoted 6 lines
drivers/scsi/lpfc/lpfc_scsi.c +1
-1+seq_buf_printf (&s," PCI resettable" ); } -buffer_done:return lpfcinfobuf; }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH 2/5 scsi: lpfc: Replace strlcat() with scnprintf() in lpfc_vport_symbolic_node_name()
In reply to Ian BridgesIn 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.
---
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(-)
drivers/scsi/lpfc/lpfc_ct.c +5 -24
@@ -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
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 2/5] scsi: lpfc: Replace strlcat() with scnprintf() in lpfc_vport_symbolic_node_name()
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 7 lines
drivers/scsi/lpfc/lpfc_ct.c +0
-0@@ -1823,34 +1823,15 @@ lpfc_vport_symbolic_node_name(struct lpfc_vport *vport, char *symbol, size_t size) {
drivers/scsi/lpfc/lpfc_ct.c lpfc_vport_symbolic_node_name(struct lpfc_vport *vport, char *symbol, · line 1827
Quoted 10 lines
drivers/scsi/lpfc/lpfc_ct.c +0
-7char 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.
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH 3/5 scsi: lpfc: Replace strlcat() with seq_buf in lpfc_rx_monitor_report()
In reply to Ian BridgesIn 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.
---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(-)
drivers/scsi/lpfc/lpfc_sli.c +24 -19
@@ -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 "
Signature · 1 line
2.47.3
PATCH 4/5 scsi: lpfc: Replace strlcat() with seq_buf in the debugfs dump helpers
In reply to Ian BridgesIn 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.
---
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(-)
drivers/scsi/lpfc/lpfc_debugfs.c +87 -115
@@ -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; }
Signature · 1 line
2.47.3
PATCH 5/5 scsi: lpfc: Replace strlcat() with sysfs_emit_at() in the sysfs show functions
In reply to Ian BridgesIn 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.
---
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(-)
drivers/scsi/lpfc/lpfc_attr.c +230 -325
@@ -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; }
drivers/scsi/lpfc/lpfc_scsi.h +0 -3
@@ -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 -
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 5/5] scsi: lpfc: Replace strlcat() with sysfs_emit_at() in the sysfs show functions
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 3 lines
drivers/scsi/lpfc/lpfc_attr.c +0
-0
[ ... ]
Quoted 10 lines
drivers/scsi/lpfc/lpfc_attr.c +0
-6@@ -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; - }
drivers/scsi/lpfc/lpfc_attr.c lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr, · line 655
Quoted 10 lines
drivers/scsi/lpfc/lpfc_attr.c +5
-4+ 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?
Quoted 6 lines
drivers/scsi/lpfc/lpfc_attr.c +2
-4-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);
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
Ian,
Quoted 4 lines
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.
Applied to 7.3/scsi-staging, thanks!
Signature · 1 line
Martin K. Petersen