The saved Cppcheck report identifies the lost directory-read result:
scst/src/scst_dlm.c:784: style: Variable 'ret' is reassigned a value
before the old one has been used. [redundantAssignment]
scst_dlm_update_nodeids overwrites the result of iterate_dir or
vfs_readdir with -ENOMEM. An empty or partial listing after a read error
can therefore replace the existing membership and report success.
Check the initial allocation and return directory errors before changing
node IDs, participant count or the local node ID. Isolate directory
access behind a task-scoped KUnit stub while retaining both kernel APIs.
Cover EIO, EACCES and ENOMEM with an existing membership and verify its
array ownership and contents survive. Include the private test header
in generated kernel patches as well as out-of-tree builds.
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.
Manual review found missing bounds checks in print_scn_pdu and
qry_rsp_handle; no saved analyzer diagnostic directly reports these
bugs. Both handlers read a TLV header before proving it is present.
Clamping an oversized value can underflow the remaining length or
accept a truncated attribute. Query handling also reads a missing
status word and can discard the old ACL before noticing bad input.
Validate every TLV header, value length and alignment before processing
any attributes. Check and decode the response status in host byte order.
Reject malformed tails before modifying names or replacing the ACL.
Add an isolated Ginkgo probe of both production handlers with payloads
ending at a guard page. Cover short headers/status, oversized and
unaligned values, malformed tails, delimiters and valid ACL replacement.
Manual review found that chap_rand ignores read errors and short reads,
then returns a partially or wholly uninitialized integer. No saved
analyzer diagnostic directly reports this defect. A negative signed
value can also reduce the intended 25-to-49-byte challenge length.
Read a complete unsigned value, retry EINTR and return failures to the
caller. Reject challenge creation on entropy failure and free a partial
challenge before returning, without publishing CHAP_C.
Add an isolated Ginkgo suite using the production CHAP implementation
with deterministic entropy I/O. Cover length bounds, interrupted and
short reads, open/read failures, EOF and partial-challenge cleanup.
Prepare interface headers privately so probes need no preceding build.
Enable Cppcheck alongside the other analyzers on master pushes and
manual dispatch, using the existing container and KUnit-enabled runner.
Keep failures for source findings and incomplete analysis, and list the
enabled job in the tooling guide. Use only analyzer names for job titles;
KUnit source coverage remains mandatory.
The Ginkgo VM job reports this runner warning:
Node.js 20 is deprecated. The following actions target Node.js 20
but are being forced to run on Node.js 24: actions/checkout@v4,
actions/upload-artifact@v4.
KUnit uses the same action versions. Update checkout and upload-artifact
to v7 in both VM workflows so they explicitly select Node.js 24, matching
the analyzer workflow. Retain checkout and artifact collection settings.
Manual review found that the explicit scatterlist argument is checked
but not used to initialize traversal. The saved pre-fix Cppcheck log
also contains this style diagnostic for the helper:
scst/include/scst.h:5012: style:
Parameter 'sg' can be declared as pointer to const
[constParameterPointer]
That message describes the argument's limited use; it does not diagnose
the wrong-list traversal or NULL dereference fixed here.
scst_get_sg_buf_first() checks its sg argument and uses sg_cnt as the
iteration bound, but initializes the cursor from cmd->sg. Supplying a
different list therefore reads the wrong buffers with an unrelated
length limit. A valid explicit list with cmd->sg == NULL also reaches a
NULL scatterlist in __scst_get_buf().
Initialize the cursor from sg, keeping the list and its element count
paired. The public helper's signature and command-buffer helpers remain
unchanged. The in-tree scst_zero_write_rest() caller selects write_sg,
which can be out_sg for bidirectional commands, although its own call
site is currently disabled by #if 0. This fixes the helper contract; it
does not establish a currently reachable in-tree I/O corruption path.
Add LIB KUnit coverage for a two-entry explicit list when cmd->sg points
to another list or is NULL, including end-of-list and DMA-sync state.
Also cover NULL and zero-length selections. This source-level defect
was confirmed while reviewing the historical analyzer fixes; no new
analyzer model or suppression is required.
The saved pre-fix Cppcheck log reports:
scst/src/scst_lib.c:4170: style:
Redundant initialization for 'res'. The initialized value is
overwritten before it is read. [redundantInitialization]
The diagnostic points to the lost -EOVERFLOW result. Manual review of
the same function additionally found the cursor wraparound failures
below; those are not separate analyzer diagnostics.
gen_relative_target_port_id has two boundary failures. Returning ID
65535 increments the static cursor to 65536 without wrapping it. The
next uniqueness check truncates that value to u16, allowing reserved ID
zero to be returned. If the range is occupied, the invalid saved cursor
can also prevent the search from completing a full cycle.
The initial -EOVERFLOW value is separately overwritten by successful
mutex_lock_interruptible. Exhausting a valid range then returns success
without assigning an ID, allowing target enable to continue incorrectly.
Advance and wrap the cursor before every successful return. Search each
valid candidate once and return -EOVERFLOW if none is available, leaving
the caller's output and cursor unchanged on failure. Keep serialization
under scst_mutex in the existing entry point.
Add KUnit cases for the first candidate, occupied candidates, allocation
of the last ID, wrap to ID 1 and complete exhaustion. Stub uniqueness
checks per test task so the tests do not modify the global target list.
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.
The saved pre-fix Cppcheck log reports an unreachable NULL check:
scst/src/dev_handlers/scst_user_main.c:1943: style:
Condition '!cmd' is always false [knownConditionTrueFalse]
The extended-CDB ioctl acquires ucmd_ref before checking whether the
command has core state, is still executable, and has been published to
userspace or submitted for background execution. A rejected state jumps
to the unlock-only path, leaking the reference on every invalid request.
The retained command can then prevent device cleanup from completing.
Keep hash lookup, state validation and paired user/core reference
acquisition in a private helper. On rejection, drop the acquired user
reference after releasing cmd_list_lock: its final put can take the same
lock while removing the command from the hash. Return a core command
only on success, eliminating the unreachable NULL-command cleanup path.
The reference leak was found by manual review of the same ioctl while
investigating its always-true assertion and error paths. The Cppcheck
message above identifies the unreachable check removed here; it does
not diagnose the leaked reference.
Add eight KUnit cases for sent, background and parsing commands, absent
core state, unpublished and completed commands, missing handles and
zero-reference hash entries. Verify both counters and the returned core
pointer so rejection cannot silently retain either reference.
The saved pre-fix Cppcheck log reports:
scst/src/dev_handlers/scst_user_main.c:1951: style:
Same expression on both sides of '=='. [duplicateExpression]
The extended-CDB ioctl contains an always-true assertion:
EXTRACHECKS_BUG_ON(cmd->cdb_buf == cmd->cdb_buf);
Any otherwise valid command longer than the inline CDB buffer reaches
that assertion. With CONFIG_SCST_EXTRACHECKS enabled it triggers BUG
instead of copying the extension to userspace. The intended invariant
is that a long command uses external CDB storage.
Compare cmd->cdb with cmd->cdb_buf and share tail selection through a
private helper. Keep the existing inline-length handling, copy length
and ioctl error behavior.
Add a USER-cdb KUnit suite covering every inline length and external
lengths through 32 bytes. The external-buffer cases exercise the path
that previously asserted, without user-memory access or a live handler.
Include the user ABI header needed by the private command definitions.
When every valid fallback tag is occupied, find_first_zero_bit returns
tag_num. The old loop calls test_and_set_bit before checking that
sentinel. If the bit was clear, it even exits the loop without checking
the range and uses tag_num as an index into tgt_cmd_map.
For word-aligned pool sizes this touches a word beyond the bitmap. For
other sizes it can claim padding in the last word. Both cases can return
an out-of-range command-map entry and let memset overwrite memory beyond
the map. Manual review found this bounds bug while auditing the fallback
allocation flagged by Coccinelle null/kmerr.cocci. No separate analyzer
diagnostic reported the invalid bitmap or command-map access.
Check the sentinel before test_and_set_bit and return -ENOENT when no tag
is available. Preserve the retry when another allocator wins the bit.
Expose the helper to the QLA KUnit suite in qla2x00tgt.ko.
Add KUnit cases for exhausted pools at word and non-word boundaries, an
empty pool, unchanged guard words and reuse of the final valid tag. A
spare guard word makes the old invalid access observable without needing
an actual out-of-bounds access in the test fixture.
The saved Coccinelle null/kmerr.cocci report flags the unchecked fallback
allocation in sqa_init_scst_tgt before the resource-helper refactoring:
qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c:1354:1-22:
alloc with no test, possible model on line 1368
PTR_ERR_OR_ZERO(NULL) returns success, so allocation failure is treated
as an initialized tag pool. Later command allocation dereferences NULL.
Check the allocation result explicitly and return -ENOMEM instead.
Manual review also found that BITS_TO_LONGS(tag_num) is passed as a byte
count; the diagnostic above does not report this size error. For 2048
tags on a 64-bit kernel this allocates 32 bytes instead of 256,
allowing bitmap operations to access beyond the allocation. Use kcalloc
with sizeof(unsigned long) to allocate and zero the complete bitmap.
Keep the fallback allocation helper available to KUnit on modern kernels.
Cover one tag, word boundaries, the default 2048-tag pool, zeroed storage
and injected allocation failure. Check ksize before touching the last bit
so an undersized allocation is detected even without KASAN.
Manual review of the allocation path flagged by Coccinelle's
null/kmerr.cocci rule found two resource leaks. No separate analyzer
diagnostic is recorded for these leaks. Failed SCST target registration
frees only sqa_tgt, and normal target release also leaves its command
map and tag pool behind. Repeated registration failures or target
recreation retain those allocations indefinitely.
Move allocation and cleanup into SCST-owned resource helpers. Unwind the
command map if tag initialization fails, and destroy both resources if
registration fails or the target is released. Preserve the bitmap,
percpu_ida and sbitmap backends selected by the existing kernel guards.
Target stop phase 1 uses bounded session waits. Before dropping lport
callbacks and releasing command storage, wait for the SCST session list
to drain. SCST removes each session after its commands and unregistration
callback have completed, so late command releases can still return tags.
Keep the helpers in scst_qla2xxx.c and link the QLA KUnit suite into
qla2x00tgt.ko, following the existing SCST module layout. Expose private
helpers only in test builds and use task-scoped registration and
allocation stubs. Cover map allocation failure, tag initialization
failure, registration failure and normal resource destruction.
Build and record the matching qla2xxx_scst.ko dependency for the runner,
and clean both module builds together. Extend the build and cleanup
fixtures to check this dependency and failure propagation.
The dependency has no tests of its own. Requiring every supplied module
to have a KUnit manifest made the runner abort before loading any module:
Module has no KUnit label manifest; rebuild with kunit-build
Accept modules without suites only when they are dependencies of supplied
test owners. Keep rejecting missing or malformed test manifests and
unrelated modules. Exercise native metadata parsing, label selection,
dependency load order and reverse cleanup in isolated runner fixtures.
The fixtures need no real target, but loading their owner still
initializes the QLogic drivers. Run them in a disposable VM without an
HBA; hardware session teardown requires separate integration validation.
The analyzer image and runner already support Coccinelle, but its matrix
entry is commented out. Pushes to master and manual workflow runs
therefore skip semantic-patch analysis entirely.
Enable coccicheck as a parallel matrix job, using the existing runner to
invoke make coccicheck with KUnit sources included. Remaining source
findings fail the job, and parser/tool failures retain the separate
incomplete-analysis status. Existing diagnostics remain visible without
adding exclusions or changing analyzer models.
Update the CI guide to list the enabled target. Cppcheck remains disabled,
and workflow triggers remain push to master and manual dispatch.
Coccinelle null/badzero.cocci reports:
iscsi-scst/usr/config.c:868:15-16: WARNING comparing pointer to 0
iscsi-scst/usr/config.c:1035:16-17: WARNING comparing pointer to 0
Both expressions compare struct target pointers to the integer zero.
Use !target for the failed lookup in config_params_set and for the
discovery-account flag passed to __config_account_add in
config_parse_main.
Preserve lookup failure handling and discovery-account selection. This
is a pointer-test cleanup, with no change to the represented NULL value.
Coccinelle misc/semicolon.cocci reports:
iscsi-scst/usr/iscsi_scstd.c:958:2-3: Unneeded semicolon
iscsi-scst/usr/iscsid.c:367:4-5: Unneeded semicolon
scst/src/scst_event.c:772:2-3: Unneeded semicolon
usr/stpgd/stpgd_main.c:269:3-4: Unneeded semicolon
Remove the trailing semicolons after the iSCSI netlink-open check, login
parameter check, event-size read check and STPG child-cleanup loop. Each
semicolon is
an empty statement after a completed block, so removing it preserves
control flow and requires no analyzer exception.
Coccinelle misc/cond_no_effect.cocci reports:
usr/include/debug.c:121:1-3:
WARNING: possible condition with no effect (if == else)
Buffer tracing prints the same final line whether or not a full row has
already been printed. Remove that conditional and its unused j state.
Also remove the identical immediate/non-immediate branches in fileio
cache synchronization. Both branches already call exec_fsync, so retain
that call and the TODO for future background execution. This second
cleanup was found while reviewing the same pattern; it is not a separate
finding in the current report.
The trace output and synchronous cache-flush behavior are unchanged.
Coccinelle misc/array_size.cocci reports:
iscsi-scst/usr/iscsi_adm.c:595:33-34: WARNING: Use ARRAY_SIZE
Move the existing iSNS ARRAY_SIZE helper to misc.h and use it for the
user-command dispatch table in iscsi-scst-adm. Both users already include
that header, so they can share the definition.
Preserve the size_t comparison, including rejection of negative operation
values, and the unsupported-operation path. The table index is used
only after the bounds check succeeds.
Coccinelle reports "parse error" while parsing SHARED_OPCODE_DESCRIPTORS
and VDISK_OPCODE_DESCRIPTORS in scst_vdisk_main.c. For the shared list,
the raw log marks its final element:
BAD:!!!!! &scst_op_descr_verify16,
The trailing comma leaves an incomplete expression when the macro is
parsed on its own. Remove it from each private macro and add the comma
at every initializer use instead.
Token comparisons confirm that vdisk_opcode_descriptors,
vdisk_opcode_descriptors_type2 and vcdrom_opcode_descriptors expand to
the same C tokens. The public SCST_OPCODE_DESCRIPTORS convention is
unchanged. This fixes these parser failures without a macro model.
Use the existing kzalloc_obj and kzalloc_objs helpers for PR abort state,
copy-manager list IDs, WRITE SAME state, latency statistics and UNMAP
fixtures. Drop the redundant default GFP_KERNEL argument in the ordering
fixture.
For single objects, the helper derives the allocation size from the
object expression. For arrays, keep the element count and checked
multiplication provided by the shared compatibility helper. Preserve
zeroing and explicit allocation flags, including the GFP_ATOMIC path.
These are allocation cleanups carried over from the historical analyzer
series; they do not fix an observed report-mode diagnostic in the
current Coccinelle run. No allocator model or exclusion is introduced.
Coccinelle free/ifnullfree.cocci reports:
scst/src/scst_lib.c:15441:2-7:
WARNING: NULL check before some freeing functions is not needed.
Remove the redundant guard around vfree(buf) in scst_copy_file and the
same guard around vfree(mem_verify) in vdev_verify. Both pointers may be
NULL on cleanup paths, which vfree accepts throughout the supported
kernel range.
Leave the NULL guards in backport.h intact: they protect older teardown
APIs whose NULL handling differs, rather than these vfree calls.
Coccinelle api/err_cast.cocci reports:
scst/src/scst_lib.c:8710:8-15:
WARNING: ERR_CAST can be used with bw
blk_copy_kern_sg returns an error pointer on failure. Its caller,
blk_map_kern_sg, converts that value to an integer with PTR_ERR and
immediately back to a pointer with ERR_PTR.
Assign ERR_CAST(bw) to rq instead. It expresses propagation of the same
error pointer to the request-pointer return type without the redundant
pointer-to-integer round trip.
Coccinelle reports "parse error" at declarations using compl, including
this fragment of struct scst_lksb in scst/include/scst.h:
BAD:!!!!! struct completion compl;
The parser treats compl as the C++ complement operator even in these C
files. Rename the DLM and SRPT fields and the local target variable to
completion. Update their init, wait and signal sites, as well as the
SRPT field documentation.
This removes the conflicting identifier without adding a parser model.
Types, structure layout and completion lifetimes remain unchanged.
Other unsupported macros can still cause independent parser failures.
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.
Coccinelle iterators/use_after_iter.cocci reports the following diagnostic
in scst_event_disallow_event (message wrapped):
scst/src/scst_event.c:750:38-39:
ERROR: invalid reference to the index variable of the iterator
on line 731
After the disallow loop, e is the exhausted list iterator. The following
TRACE_MEM uses e even though the allocation about to be freed is
event_entry. Log event_entry instead so the trace names the allocation
passed to kfree immediately afterwards.
This fixes the diagnostic and the misleading trace address; the lookup,
list removal and allocation lifetime are unchanged.
Coccinelle api/atomic_as_refcounter.cocci reports:
scst/src/dev_handlers/scst_user_main.c:175:5-24:
WARNING: atomic_dec_and_test variation before object free at line 176.
ucmd_ref controls the lifetime of a user command. Replace its generic
atomic operations with kref and a last-reference release callback.
Update initialization, diagnostic reads and KUnit fixtures together.
A command can remain in the hash after its last put while the destructor
waits for cmd_list_lock. The old acquisition increments that zero count
and then undoes the increment. Use kref_get_unless_zero under the same
lock to reject a dying command without modifying its zero count.
Preserve the full ordering of successful acquisition that the old
atomic_inc_return supplied. Add KUnit cases for acquiring a live command
and rejecting the zero-count state before hash removal.
The kref API supports the old kernels in the build matrix without a new
refcount_t compatibility layer. No analyzer exclusion is needed.
Coccinelle api/atomic_as_refcounter.cocci reports:
scst/src/scst_priv.h:708:5-24:
WARNING: atomic_dec_and_test variation before object free at line 709.
cmd_ref counts ownership of struct scst_cmd, and its final put invokes
scst_free_cmd. Replace the generic atomic counter with kref, using
scst_free_cmd as the release callback. Update initialization, get/put,
diagnostic reads and the vdisk KUnit fixtures together.
Keep the existing barrier after reference acquisition and the deferred
out-of-sequence destruction path. Completion and work counters retain
their atomic types; only the command lifetime counter changes.
Use kref to retain compatibility with the supported old kernels without
introducing a separate refcount_t backport. This addresses the ownership
counter diagnostic without changing the Coccinelle rule.
Coccinelle api/atomic_as_refcounter.cocci reports:
scst/src/scst_mem.c:1642:5-24:
WARNING: atomic_dec_and_test variation before object free at line 1643.
Auditing this ownership counter exposes two bugs in shared pool lookup.
A lookup with a different owner mm returns the existing pool without
taking a reference. Its caller can then release another device's
ownership. Reject that lookup with NULL.
The last put can also reach zero before the destructor acquires
sgv_pools_mutex to unlink the pool. A concurrent lookup can still find
that pool. Use kref_get_unless_zero under the lookup mutex to reject a
pool whose destruction has already started.
Convert sgv_pool_ref to kref, retaining the existing nullable cleanup for
initialization failures separately from the last-reference callback.
The kref API also works on supported kernels predating refcount_t.
Add KUnit cases for same-mm acquisition, different-mm rejection and the
zero-reference state before unlinking. Export the private test header
in generated in-tree patches as well as the out-of-tree build.
The full SCS scan reports three instances of the same diagnostic:
scst/src/scst_lib.c:3007:1:
scst/src/scst_sysfs.c:1762:1:
scst/src/scst_targ.c:5666:1:
error: SCS.1: place SCST_KUNIT_VISIBLE on a line of its own
Move the annotations above the declarations of scst_check_reassign_sess,
__scst_acg_black_hole_store and scst_mgmt_cmd_init. This satisfies the
existing SCS.1 rule without changing visibility or function behavior.
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.
KUNIT_EXPECT_PTR_EQ stores both operands in ordinary const void pointer
fields when reporting a failure. Passing the event fixture's __user
tokens directly produces two Sparse address-space warnings per assertion.
Explicitly cast the operands with __force at the three assertion sites.
Keep the __user annotations on the stub parameters and shared token, and
retain pointer equality checks and address reporting without dereferencing
the token.
TRACE_EXIT_HRES converts its argument to unsigned long for logging. Passing
an __poll_t directly triggers Sparse's restricted-type warning in both
scst_event_poll() and dev_user_poll().
Explicitly remove the bitwise restriction with __force at those two trace
calls. Keep the typed mask for event operations and the return value,
without weakening the shared trace macro for other callers.
The IB/CM and RDMA/CM callers of srpt_cm_req_recv() provide exactly one
connection identifier. Context setup branches on ib_cm_id, while error
cleanup branches on rdma_cm_id. Smatch loses the relationship between
the pointers and reports a possible NULL ib_cm_id dereference.
Select the connection identifier by ib_cm_id in both places and document
the caller contract. This preserves cleanup for both supported paths
and makes the control flow consistent without suppressing diagnostics.
SYNCHRONIZE CACHE with IMMED completes the command before calling
vdisk_fsync() with cmd=NULL and async=true. The write-through,
nonvolatile-cache and nullio bypasses dereference that pointer while
trying to complete the command again.
Only complete an asynchronous bypass when a command was supplied.
Enable the three commandless KUnit cases previously skipped for this
bug, removing their unreachable return after kunit_skip(). Also cover
SYNCHRONIZE CACHE(10/16) with IMMED in all three modes, checking that
completion occurs once, references are balanced and no flush is issued.
Fixes: 4cefa1b830 ("vdisk_fsync() fixes and cleanups")
WRITE SAME emulation chooses WRITE(16) or WRITE(32) from the required DIF
handling, independently of the input CDB length. The builder nevertheless
copies the flags from the output format's offset in the input command.
For WRITE SAME(16) converted to WRITE(32), that is byte 10 of the transfer
length; for a long command converted to WRITE(16), it is the control byte.
An unrelated value can therefore change WRPROTECT and other output flags.
Select the input flags once using the original CDB length and use that
value in both output formats. Preserve the existing WRPROTECT defaulting.
Add KUnit cases with different flag and unrelated-byte values for both
conversions and for commands that retain their format.
Fixes: 906f4dfefb ("Merge t10-pi branch back")
The internal WRITE(32) generated for DIF type 2 has three defects:
put_unaligned() infers a one-byte store from its destination pointer,
reserved/control bytes retain stack data, and the application-tag mask
is copied from bytes 26 and 27 even for a 16-byte input CDB. This can
corrupt the protection information or make the backend reject the write.
Clear the complete output buffer, use a sized big-endian application-tag
store and obtain the mask through scst_cmd_get_dif_app_tag_mask(). That
helper reads an explicit WRITE SAME(32) mask or derives the short-command
mask from the application's tag-check setting.
Extract the existing CDB builder behind the private KUnit interface. Add
regression cases for distinct tag bytes, full output images, short and
long commands, both short-command mask modes and buffer boundaries.
Nonzero poison patterns expose missing stores and reads beyond the short
CDB. Include the private header in generated in-tree kernel patches.
Fixes: 906f4dfefb ("Merge t10-pi branch back")
A progress timeout can occur before the child consumes all supplied
input. On Python 3.12, retrying communicate() without input stops
scheduling writes to stdin. The child waits for EOF while the driver
keeps waiting for output, which can hang the analyzer script tests
until the CI job timeout.
Run communicate() once in an I/O worker while the main thread waits
for its result and updates progress. On interruption, stop the owned
process group before joining the worker so blocked pipes cannot
prevent cleanup.
Make the regression wait for two progress updates before reading stdin
to force the pending-input case. Bound the entire child with an alarm
so a regression reports a failure instead of hanging the suite.
Validation: reproduce both check modes failing on Python 3.12.14
before the fix; all 75 lint-test fixtures pass on chesnokov-vm2
afterwards. All 11 process/log fixtures also pass locally on Python
3.14.8.
Fixes: c8648f9410 ("scripts: Share analyzer process and raw-log handling")
Add a parallel make checkpatch job for pushes to master and manual
runs. Fail analyzer jobs on remaining source findings with status 1;
keep tool, build and coverage failures separate with status 2.
Validate the current report, Make status and native coverage before
classifying results. Checkpatch needs its script and data, not a
prepared KUnit kernel. Update the isolated CI report fixtures.
Run SCST style fixtures and make scs_lint in a separate job without a
kernel build, alongside the native analyzers. Pin uv and its setup
action; the linter supplies its pinned parser dependencies.
Retain the pull-request Checkpatch workflow for patch and commit
checks. Remove the standalone push Checkpatch workflow, whose role
is now covered by the common static-analyzer workflow.
Rename Tooling to Analyzer script tests and clarify that analyzer
jobs include KUnit sources without executing KUnit. Update Checkout
and Upload Artifact to v7 to replace their deprecated Node.js 20
runtimes, and align the CI documentation with the actual jobs.
Document SCS.1 and expose make scs_lint for the common C source/header
scope, including userspace and KUnit. Adapt engine's OCS.7.5 check to
SCST_KUNIT_VISIBLE, SCST_KUNIT_VISIBLE_INLINE and VISIBLE_IF_KUNIT.
Require each annotation to occupy its own line; leave other function
attributes and source warnings unchanged.
Use Tree-sitter to distinguish code from comments, literals and
preprocessor directive contents while checking every conditional
branch. Pin the parser dependencies and run them through uv.
Support whole-tree, DIFFBR and explicit-file selection without new
exclusions or diagnostic exceptions. Retain findings and checked-file
records even when a later input fails. Add isolated rule, diff-mode,
coverage and read-failure fixtures with a separate test target.
Expose make checkpatch through the common C source/header policy,
including userspace and KUnit sources. DIFFBR selects whole changed
files. Use the kernel's Checkpatch and companion data with --strict,
without adding diagnostic exceptions or loading local configuration.
Retain raw output and require native totals and a completion record
for every selected file. Distinguish source findings from unreadable
inputs, inconsistent reports and tool failures, with isolated fixtures.
Keep native caches in a temporary directory and serialize only this
analyzer's reports so it can run alongside the Kbuild analyzers.
Make checkpatch_scan use this target's implementation and exit status
instead of suppressing errors. Preserve the separate existing policy
of patch/commit helpers and the kernel regression runner.
Reserve the legacy-kernel fork checks for manual runs instead of adding
two VM jobs and extra kernel builds to the regular CI selection. Remove
the Linux 5.8/5.9 Ginkgo jobs and regression matrix entries, retaining the
kernel pins and local-only VM runner for reproduction.
Label the fork spec manual and exclude that label from both ci-pr and
ci-push. Move smoke to the READ CAPACITY entries so their existing CI
coverage remains intact. Keep full manual runs inclusive, update the
selection checks, and document focused manual execution.
dev_user_map_buf() now switches at Linux 5.9: get_user_pages() with
mmap_read_lock() on 5.8, pin_user_pages() from 5.9. The GitHub regression
subset goes from 5.10 to 5.4, so neither side of that boundary is compiled
on pull requests. Add the 5.9 and 5.8 selectors, which resolve to the
5.9.16 and 5.8.18 entries already in ABT_KERNELS.
scst_user pins its data buffers from Linux 5.9, the first release in
which fork() gives the child a copy of a pinned page (commit
70e806e4e645, "mm: Do early cow for pinned pages during fork() for
ptes"). The Ginkgo "handler forks" spec checks this, but CI boots only
the current Fedora kernel, so the boundary itself is never tested.
Add a Ginkgo job matrix that runs the local "user" specs on Fedora 33's
Linux 5.8.18 and 5.9.16, on both sides of the boundary. On 5.9.16 the
fork spec relies on the pin alone; on 5.8.18 the handler protects its
buffers with MADV_DONTFORK, as required there. Both must keep correct
geometry and data. The jobs are named after their kernels, like the
regression jobs.
- kernel-5.8.env, kernel-5.9.env and their SHA256SUMS pin those kernels'
rpms from Fedora's koji, like kernel.env and kernel.SHA256SUMS.
Fedora's only 5.9.0 build is a rawhide kernel that defines RHEL_MAJOR 8
and RHEL_MINOR 99, so SCST's backports treat it as RHEL and the build
fails; 5.9.16 has the same fork() behavior.
- build-image takes an optional pin name, such as "kernel-5.9", for
Ginkgo images. The image is otherwise built the same way.
- The Dockerfile downloads exactly the rpms listed in the selected
checksum file, since 5.9 has no kernel-modules-core package.
- vm.py --local-only boots only the target guest, without the iSCSI
initiator or the private network, and expects only the local report.
The guest then writes a config without an iscsi section. The label
filter is restricted to local specs: "local && (<filter>)", or "local"
without one.
Extend the scst_user fault hook with a fork action: once armed, the
handler fork()s before its next scst_user ioctl, and the child only
sleeps, as a crash reporter, system() or popen() would. The new spec
reads LBA 0 first, so the handler's cached buffer holds known data,
then makes the handler fork. Afterwards READ CAPACITY(10) and (16),
reads and writes must return the right geometry and data.
The hook logs its success line only after the fault was applied, and
logs a failed fork() with its errno. The specs fail on that line and
require each fault's success line, so a failed fork() cannot pass as a
handler that forked.
Before Linux 5.9, the same boundary as dev_user_map_buf(), scst_user
keeps get_user_pages() and a handler that forks must use MADV_DONTFORK,
which fileio_tgt does not. There the hook applies MADV_DONTFORK to every
active data buffer before it forks and logs how many it protected. It
tracks the buffers from the handler's ALLOC_MEM and EXEC replies, and
restores MADV_DOFORK before the handler frees one. The spec requires
that line below 5.9 and its absence from 5.9, and the same correct
geometry and data on every kernel. So 5.9 and later test the pin
alone, and older kernels test the documented MADV_DONTFORK requirement.
Without pinned buffers SCST keeps reading the pre-fork page, and the
next READ CAPACITY reports the stale data block as its block length.
dev_user_map_buf() takes the handler's buffer pages with
get_user_pages(FOLL_WRITE). With SCST_USER_MEM_REUSE_* the buffer stays
cached in the SGV pool and is reused by later commands, so SCST keeps
these page references for a long time.
A page reference does not keep the handler and SCST on the same page
across fork(). After the handler forks, its private pages are shared
copy-on-write, and the handler's next write to a cached buffer moves the
handler to a new copy while SCST keeps using the old page. SCST then
returns stale data to the initiator. For READ CAPACITY it also takes the
stale bytes as the block length and changes the device geometry, for
example:
1 << 30 != 1145132097
scst_user: ...: Changed block shift from 12 into 30 / 30
Any fork() in the handler triggers this, including a crash reporter that
clones a minidump writer without CLONE_VM, system() and popen().
Pin the pages with FOLL_LONGTERM instead. Since Linux 5.9 (commit
70e806e4e645, "mm: Do early cow for pinned pages during fork() for
ptes"), fork() gives the child a copy of a pinned anonymous page, so the
handler and SCST keep sharing the same page. pin_user_pages() itself is
older, but before 5.9 fork() write-protects pinned pages like any other,
so pinning alone does not help there. Release them with
unpin_user_pages(), marking them dirty with SetPageDirty() as before.
unpin_user_pages_dirty_lock() is not usable here: it takes the page
lock, and on_free_cmd() can release an uncached buffer in atomic
context. Use the pin from Linux 5.9; older kernels, including
distribution kernels based on them, keep the existing code.
doc/scst_user_spec.sgml so far required handlers that fork() to mark
their data buffers MADV_DONTFORK. With the pin that is only needed on
kernels older than 5.9, and for private HugeTLB buffers on kernels older
than 5.12: fork() copies pinned HugeTLB pages only since commit
4eae4efa2c29 ("hugetlb: do early cow when page pinned on src mm"), which
5.10.y and 5.11.y do not have. Update that note and the source comment
accordingly. Also document that data
buffers should be anonymous or shared memory: since Linux 6.5 the kernel
refuses a long term writable pin of a MAP_SHARED mapping of a regular
file on a file system with writeback, and a command using such a buffer
then fails with INTERNAL TARGET FAILURE sense. Such a buffer was not
safe with get_user_pages() either, because SCST could write to it while
the file system wrote it back.
Add scst-USER-buffers, which maps a KUnit-owned user buffer with
dev_user_map_buf() and releases it with dev_user_unmap_buf() with
interrupts disabled, checking that the pages are unpinned and marked
dirty only after a write.
Run isolated tooling checks and separate Sparse and Smatch Make targets
on pushes to master and manual dispatch. Use the pinned kernel with
KUnit enabled and preserve full logs, policy and tool provenance.
During the source-fix rollout, completed source findings are advisory.
Tool, build, parser and coverage failures remain blocking, including an
empty scan or missing KUnit inputs. Local Make targets stay strict.
Validate the current run's report together with Make's exit status,
since Make maps both findings and incomplete analysis to status 2.
Prepare the runner and container for Coccinelle and Cppcheck as well,
but leave their matrix entries commented out. Cover all four report
formats and GNU Make exit mapping with isolated tooling fixtures.
Expose make cppcheck using freshly captured kernel CHECK commands and
real userspace compiler invocations. Keep their include paths and
definitions unchanged and analyze kernel and userspace contexts
separately.
Use the shared source scope without additional input exclusions, project
models or diagnostic suppressions. Relevant diffs trigger full
configured analysis; KUnit coverage follows the actual build
configuration.
Retain build logs, compilation databases, native XML and checker
coverage. Separate source findings from parser failures, missing inputs
and incomplete builds, with isolated report and capture fixtures.
Expose make coccicheck using report rules from the selected kernel and
the shared source policy, including userspace and KUnit sources. Diff
mode selects whole changed sources and headers.
Preserve full native output and record rule initialization, per-input
handling and finalization. Treat parser errors and missing coverage as
incomplete analysis even if spatch returns zero; report source findings
separately.
Add isolated log and policy fixtures. Do not introduce project macro
models, diagnostic exceptions or CI execution.
Build standalone Sparse and Smatch from recorded revisions, applying the
reviewed SCST Smatch frontend correction. Install matching native Smatch
data without replacing standalone Sparse or adding project models.
Reuse the existing checksum-pinned kernel headers. Record source
commits, patch and binary hashes, and installed model contents for CI
evidence. Fail preparation instead of silently falling back to different
tools; no SCST modules are built or loaded while preparing this image.
Use the dedicated native analyzer targets instead of regression wrappers
that return success after tool failures and summarize filtered output.
Remove their suffix switches and the analyzer-only IBM VIO option.
Update the regression caller and kernel-option parser together; the
maintained kernel list and compilation matrix are unchanged. Keep legacy
Checkpatch until its own integration series.
Gleb Chesnokov
wenlxieandGleb Chesnokov