The saved Cppcheck report warns in isert_send_completion_handler:
iscsi-scst/kernel/isert-scst/iser_rdma.c:429: warning:
Either the condition 'iscsi_req_pdu' is redundant or there is overflow
in pointer subtraction. [nullPointerArithmeticRedundantCheck]
The parent request is optional, but container_of subtracts the member
offset before the NULL check. Resolve the parent container only after
confirming that the request exists and has a buffer. Preserve completion
handling for responses without a parent and for valid remote tags.
Manual review found that isert_poll_cq forms &cq->wc[err] when
ib_poll_cq returns a negative errno. This pointer formation already
indexes outside the completion array, even if the following loop does
not dereference it. No saved analyzer diagnostic directly reports it.
Stop on nonpositive results before forming the end pointer and preserve
the result for the caller. Keep the loop exit next to the poll result.
Add the scst-ISER-cq KUnit suite to isert-scst.ko. Call the production
polling function with a private ib_device poll callback and task-local
stubs for the success and error completion handlers. Cover initial
-EIO and -EINVAL results, an empty queue, full and short batches, and
an error after completions. Check poll termination, completion order
and dispatch to the correct handler. Negative-result cases also exercise
kernel UBSAN bounds checking when CONFIG_UBSAN_BOUNDS is enabled.
Include iSER in the existing KUnit module inventory and build it after
iscsi-scst with the RDMA compatibility probes. Propagate the common
KUnit flags through ordinary iSER builds as well, so analyzer builds
include the suite. Extend tooling checks for module selection, dependency
order, cleanup, private headers and in-tree source export.
The saved pre-fix Cppcheck log reports:
iscsi-scst/kernel/nthread.c:326: style:
Clarify calculation precedence for '+' and '?'. [clarifyCalculation]
close_conn calculates its shutdown deadline with this expression:
shut_start_waiting + conn->deleting ? CONN_DEL_SHUT_TIMEOUT :
CONN_REG_SHUT_TIMEOUT
Addition binds more tightly than the conditional operator. The expression
therefore tests shut_start_waiting + conn->deleting and selects an
absolute 10- or 125-second jiffies value, rather than adding an interval
to the start timestamp. On a host with sufficient uptime, the transport
can be closed immediately after shutdown starts.
Select the interval first, add it to shut_start_waiting, and retain the
wrap-safe time_after comparison. Move the check into a private helper so
its timing behavior can be covered without a live connection.
Add KUnit coverage for both deletion and normal shutdown, high uptime,
the ticks before/at/after expiry and jiffies wraparound. The tests retain
the original strict-after deadline convention.
Coccinelle emits "parse error" while recovering from BIO callbacks whose
opening braces occur separately in the two signature branches. The raw
log includes this fragment of scst/src/dev_handlers/scst_vdisk_main.c:
bad: static void vdev_flush_end_io(struct bio *bio, int error)
bad: {
bad: #else
Move the opening brace below the signature conditional in
vdev_flush_end_io, blockio_end_sync_io, blockio_endio and
blk_bio_map_kern_endio. Keep version-specific local error extraction
inside its existing compatibility condition.
Also bracket the conditional max_nr_vecs assignment in blockio_exec_rw
and the sendpage fast path in write_data. Initialize the legacy
sock_no_sendpage fallback before the fast path so an else no longer
appears in only one preprocessor branch.
These changes address parser failures, not runtime warnings. All kernel
compatibility conditions are retained. Preprocessed token comparisons
covering 56 kernel-version/SUSE combinations show unchanged C code.
The TX fixture assigns an ordinary integer to the restricted __be32
ddigest field, producing a Sparse type warning. Production digest code
stores little-endian CRC bytes in that field using an explicit __force
cast, despite its declared type.
Initialize the arbitrary test digest with cpu_to_le32 and the same cast.
This keeps its bytes unchanged on little-endian hosts and consistent on
big-endian hosts. The tests exercise send progress and offsets, not CRC
calculation or the contents of this placeholder.
Integrate iSCSI test objects in out-of-tree module builds.
Move transmit state names to a private header and link a separate
suite with task-scoped socket and completion boundaries.
Exercise data, padding and digest progress, retries, short sends and
TX_END completion. Preserve socket arguments and completion order
without creating a socket or initiator session.
Move the minimum kernel check into the shared backport header so all
kernel components reject Linux versions older than 3.10 and RHEL 6.
Simplify now-unreachable compatibility conditions and remove the unused
QLogic patch for Linux 2.6.26 and 2.6.27.
Procfs support was removed in 2019, but dead configuration guards,
patch specialization logic and unused declarations remained. Remove these
remnants and simplify paths that can no longer receive procfs-only objects.
Support for the following changes in the Linux kernel v7.0:
- e3b2cf6e5dba ("kernfs: pass struct ns_common instead of
const void * for namespace tags")
This is the result of running the Linux kernel Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci against the SCST tree.
This patch doesn't change any functionality.
Upstream workqueue changes introduce a new WQ_PERCPU flag and plan to
switch alloc_workqueue()'s default from per-CPU to unbound
To kepp SCST behaviour unchanged across kernels, this patch makes all
alloc_workqueue() users explicit about whether they want per-CPU or
unbound queues.
The calculation of the upper limit for queues does not depend solely on
the number of online CPUs; for example, the isolcpus kernel
command-line option must also be considered.
To account for this, the block layer provides a helper function to
retrieve the maximum number of queues. Use it to set an appropriate
upper queue number limit.
-fstack-protector-strong is controlled by the in-tree
CONFIG_STACKPROTECTOR_STRONG config option. If the flag is set without
the config option, scst fails to build:
ERROR: modpost: "__stack_chk_fail" [scst/src/scst.ko] undefined!
-fstack-clash-protection is disabled in the kernel's Makefile due to an
incompatibility with randomize_kstack_offset.
Fixes: 86d3d4bb1d ("kbuild: Enable additional compiler warnings")
Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
Kernel v6.15+ removed LIBCRC32C and switched in-tree users to CRC32.
Keep older kernels working by accepting either CRC32 or LIBCRC32C in
Kconfig and preprocessor guards, so crc32c() usage compiles and links
on both old and new kernels.
Replace combined sysfs_emit() calls with separate sysfs_emit() and
sysfs_emit_at() invocations when emitting SCST_SYSFS_KEY_MARK in sysfs
attribute show handlers.
This improves code clarity and consistency, explicitly handling the
conditional addition of SCST_SYSFS_KEY_MARK.
This patch does not change any functionality.
Replace scnprintf() with sysfs_emit() and sysfs_emit_at() in sysfs show handlers.
These helper functions are specifically designed for sysfs output, providing safer
handling of buffer lengths and consistency across kernel sysfs interfaces.
This patch does not change any functionality.
Replace snprintf() with scnprintf() in two places:
- code paths that build up a buffer incrementally
- sysfs attribute “show” handlers
scnprintf() guarantees its return value is the number of bytes actually
written (never exceeding the buffer), preventing potential overruns.
This patch does not change any functionality.
Replace all uses of sprintf() with scnprintf() in sysfs attribute
callbacks to eliminate the risk of buffer overflows.
This patch does not change any functionality.
The workqueue maintainer wants to remove the create*_workqueue() macros
because these macros always set the WQ_MEM_RECLAIM flag and because these
only support literal workqueue names. Hence this patch that replaces the
create*_workqueue() invocations with the definition of this macro. The
WQ_MEM_RECLAIM flag has been retained because I think that flag is necessary
for workqueues created by storage drivers.
Support for the following changes in the Linux kernel v6.12:
- 5f60d5f6bbc1 ("move asm/unaligned.h to linux/unaligned.h")
- cb787f4ac0c2 ("[tree-wide] finally take no_llseek out")
According to kernel documentation, this flag should be set if the
workqueue will be involved in the kernel's memory reclamation flow.
Since it is not, there is no need for the driver's WQ to have this
flag set so remove it.
create_workqueue() was replaced with alloc_workqueue() with max_active set
to 0. However, the original create_workqueue() implicitly set max_active
to 1.
This change has led to unexpected bugs because previously, work items
could only be executed one by one. With the change, they can now be
executed simultaneously.
This patch fixes the issue by restoring max_active to 1.
Fixes: f4686e9102 ("scst: Remove deprecated create_workqueue()")
Fixes: https://github.com/SCST-project/scst/issues/179
strlcpy() reads the entire source buffer first. This read may exceed the
destination size limit. This is both inefficient and can lead to linear
read overflows if a source string is not NULL-terminated [1].
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
Use sendmsg() conditionally with MSG_SPLICE_PAGES in write_data()
rather than calling sendpage().
Support for the following net layer changes in the Linux kernel v6.5:
- dc97391e6610 ("sock: Remove ->sendpage*() in favour of
sendmsg(MSG_SPLICE_PAGES)")
This patch carries out a refactoring of the sendpage functionality in
the write_data() function:
1. Reorganize the logic used to select the sock_sendpage function.
2. Streamline the data sending loop by reducing conditional branches and
eliminating labels.
3. Adjust the error handling for -EINTR and -EAGAIN to make the code
cleaner and easier to follow.
This patch doesn't change any functionality.
This patch introduces several improvements to the 'write iop loop' in
the write_data() function:
1. Move iop-related variables under the scope of the 'write iop loop'.
2. Eliminate the 'retry' label, use 'continue' instead for simplicity.
3. Remove the redundant 'rest' variable, use just 'res' instead.
This patch doesn't change any functionality.
This patch introduces several improvements to the write_data() function:
1. Remove the redundant 'sendpage' function pointer variable.
2. Update variables related to size to use the size_t type for better
type correctness and safety.
3. Introduce a new variable, 'parent_req', to store the
'write_cmnd->parent_req' pointer and reduce redundant accesses.
4. Fix several checkpatch warnings.
This patch doesn't change any functionality.
This patch changes the processing threads to use INTERRUPTIBLE sleep
states in the scst_wait_event_...() functions. This aims to avoid
warnings from the hung task detection checker and to prevent
unnecessary load counting.
Fixes: d8894cbd11 ("scst.h: Refactor wait_event_locked() to enhance usability and clarity")
Gleb Chesnokov
Brian MandGleb Chesnokov
Tony BattersbyandGleb Chesnokov