user/sven/linux.git/drivers/scsi/libfc, branch v6.10.14

scsi: libfc: replace deprecated strncpy() with memcpy()

2024-02-27T02:21:23Z

strncpy() is deprecated [1] and as such we should use different apis to copy string data. We can see that ct is NUL-initialized with fc_ct_hdr_fill: | ct = fc_ct_hdr_fill(fp, op, sizeof(struct fc_ns_rspn) + len, ... In fc_ct_hdr_fill(): | memset(ct, 0, ct_plen); We also calculate the length of the source string: | len = strnlen(fc_host_symbolic_name(lport->host), 255); ...then this argument is used in strncpy(), which is bad because the pattern of (dest, src, strlen(src)) usually leaves the destination buffer without NUL-termination. However, it looks as though we do not require NUL-termination since fr_name is part of a seq_buf-like structure wherein its length is monitored: | struct fc_ns_rspn { | struct fc_ns_fid fr_fid; /* port ID object */ | __u8 fr_name_len; | char fr_name[]; | } __attribute__((__packed__)); So, this is really just a byte copy into a length-bounded buffer. Let's use memcpy(). Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt Link: https://lore.kernel.org/r/20240221-strncpy-drivers-scsi-libfc-fc_encode-h-v2-1-019a0889c5ca@google.com Reviewed-by: Kees Cook Signed-off-by: Martin K. Petersen

scsi: libfc: Map FC_TIMED_OUT to DID_TIME_OUT

2023-12-06T02:32:11Z

When an exchange is completed with FC_TIMED_OUT we should map it to DID_TIME_OUT to inform the SCSI midlayer that this was a command timeout; DID_BUS_BUSY implies that the command was never sent which is not the case here. Signed-off-by: Hannes Reinecke Link: https://lore.kernel.org/r/20231129165832.224100-4-hare@kernel.org Reviewed-by: Christoph Hellwig Signed-off-by: Martin K. Petersen

scsi: libfc: Fix up timeout error in fc_fcp_rec_error()

2023-12-06T02:32:11Z

We should set the status to FC_TIMED_OUT when a timeout error is passed to fc_fcp_rec_error(). Signed-off-by: Hannes Reinecke Link: https://lore.kernel.org/r/20231129165832.224100-3-hare@kernel.org Reviewed-by: Christoph Hellwig Signed-off-by: Martin K. Petersen

scsi: libfc: Don't schedule abort twice

2023-12-06T02:32:11Z

The current FC error recovery is sending up to three REC (recovery) frames in 10 second intervals, and as a final step sending an ABTS after 30 seconds for the command itself. Unfortunately sending an ABTS is also the action for the SCSI abort handler, and the default timeout for SCSI commands is also 30 seconds. This causes two ABTS to be scheduled, with the libfc one slightly earlier. The ABTS scheduled by SCSI EH then sees the command to be already aborted, and will always return with a 'GOOD' status irrespective on the actual result from the first ABTS. This causes the SCSI EH abort handler to always succeed, and SCSI EH never to be engaged. Fix this by not issuing an ABTS when a SCSI command is present for the exchange, but rather wait for the abort scheduled from SCSI EH. And warn if an abort is already scheduled to avoid similar errors in the future. Signed-off-by: Hannes Reinecke Link: https://lore.kernel.org/r/20231129165832.224100-2-hare@kernel.org Reviewed-by: Christoph Hellwig Signed-off-by: Martin K. Petersen

scsi: libfc: Fix potential NULL pointer dereference in fc_lport_ptp_setup()

2023-10-13T18:15:54Z

fc_lport_ptp_setup() did not check the return value of fc_rport_create() which can return NULL and would cause a NULL pointer dereference. Address this issue by checking return value of fc_rport_create() and log error message on fc_rport_create() failed. Signed-off-by: Wenchao Hao Link: https://lore.kernel.org/r/20231011130350.819571-1-haowenchao2@huawei.com Reviewed-by: Simon Horman Signed-off-by: Martin K. Petersen

scsi: libfc: Include the correct header

2022-12-01T03:03:36Z

This file does not use rcu, so there is no point in including . The dependency has been removed in commit fa519f701d27 ("scsi: libfc: fixup 'sleeping function called from invalid context'") It turned a list_for_each_entry_rcu() into a list_for_each_entry(). So just #include now. Signed-off-by: Christophe JAILLET Link: https://lore.kernel.org/r/960f34418358f0c35e645aa2cf7e0ec7fe6b60b9.1669461197.git.christophe.jaillet@wanadoo.fr Signed-off-by: Martin K. Petersen

scsi: libfc: Remove redundant variable ev_qual

2022-11-17T18:14:14Z

Variable ev_qual is being assigned and modified but the end result is never used. The variable is redundant and can be removed. Signed-off-by: Colin Ian King Link: https://lore.kernel.org/r/20221111170824.558250-1-colin.i.king@gmail.com Signed-off-by: Martin K. Petersen

scsi: libfc: Remove get_cpu() semantics in fc_exch_em_alloc()

2022-05-17T01:26:50Z

The get_cpu() in fc_exch_em_alloc() was introduced in commit f018b73af6db ("[SCSI] libfc, libfcoe, fcoe: use smp_processor_id() only when preempt disabled") for no other reason than to simply use smp_processor_id() without getting a warning, because everything is done with the pool->lock held anyway. However, get_cpu(), by disabling preemption, does not play well with PREEMPT_RT, particularly when acquiring a regular (and thus sleepable) spinlock. Therefore remove the get_cpu() and just use the unstable value as we will have CPU locality guarantees next by taking the lock. The window of migration, as noted by Sebastian, is small and even if it happens the result is correct. Link: https://lore.kernel.org/r/20211117025956.79616-2-dave@stgolabs.net Link: https://lore.kernel.org/r/20220506105758.283887-4-bigeasy@linutronix.de Acked-by: Sebastian Andrzej Siewior Signed-off-by: Davidlohr Bueso Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Martin K. Petersen

scsi: fcoe: Use per-CPU API to update per-CPU statistics

2022-05-17T01:26:50Z

The per-CPU statistics (struct fc_stats) is updated by getting a stable per-CPU pointer via get_cpu() + per_cpu_ptr() and then performing the increment. This can be optimized by using this_cpu_*() which will do whatever is needed on the architecture to perform the update safe and efficient. The read out of the individual value (fc_get_host_stats()) should be done by using READ_ONCE() instead of a plain-C access. The difference is that READ_ONCE() will always perform a single access while the plain-C access can be split by the compiler into two loads if it appears beneficial. The usage of u64 has the side-effect that it is also 64bit wide on 32bit architectures and the read is always split into two loads. The can lead to strange values if the read happens during an update which alters both 32bit parts of the 64bit value. This can be circumvented by either using a 32bit variables on 32bit architecures or extending the statistics with a sequence counter. Use this_cpu_*() API to update the statistics and READ_ONCE() to read it. Link: https://lore.kernel.org/r/20220506105758.283887-3-bigeasy@linutronix.de Reviewed-by: Davidlohr Bueso Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Martin K. Petersen

scsi: libfc: Fix use after free in fc_exch_abts_resp()

2022-03-09T03:32:31Z

fc_exch_release(ep) will decrease the ep's reference count. When the reference count reaches zero, it is freed. But ep is still used in the following code, which will lead to a use after free. Return after the fc_exch_release() call to avoid use after free. Link: https://lore.kernel.org/r/20220303015115.459778-1-niejianglei2021@163.com Reviewed-by: Hannes Reinecke Signed-off-by: Jianglei Nie Signed-off-by: Martin K. Petersen