The basic-or-smoke PR filter selects 67 specs. Recent successful PR
runs spent more than 41 minutes executing their VM tests, in addition
to image preparation. Broad functional labels keep growing this set
whenever a new basic or smoke regression is added.
Select explicit pr-labelled specs instead: 11 local and 8 iSCSI cases.
Keep representative device, target, LUN and group management, repeated
configuration application, failed fileio creation, discovery and login,
MD5/SHA256 CHAP, portal validation, READ CAPACITY, read-only rejection,
and verified fileio/blockio I/O. Preserve each test's workload, assertions
and cleanup. Exclude manual, race and stress scenarios from the PR set.
The master and manual selections remain 103 and 106 specs respectively.
Limit the complete PR job to 30 minutes, its image build to 10 minutes
and VM execution to 18 minutes. Preserve the longer push/manual limits.
The intended end-to-end PR duration is 15-20 minutes; confirm it on CI
rather than treating dry-run counts as runtime evidence.
Validate selection in the Go runner and Python VM supervisor tests.
Framework checks, all 59 CI tests, actionlint and selection dry runs pass.
The saved Cppcheck report flags driver_major:
iscsi-scst/usr/misc.c:44: warning: sscanf() without field width limits
can crash with huge input data. [invalidscanf]
The preceding fgets currently bounds input to a 256-byte buffer, so
this report does not demonstrate an overflow in the present path.
Still, state the 255-character limit in the %s conversion to bind it
directly to devname's capacity, including the terminating NUL.
The saved Cppcheck report shows that config_isns_load cannot fail:
iscsi-scst/usr/config.c:1162: style: Condition 'err==0' is always true
[knownConditionTrueFalse]
Manual review of that path found an unchecked strdup. An allocation
failure discards the existing server name and silently accepts the
configuration without its iSNS server. The diagnostic above describes
the constant result, not the unchecked allocation itself.
Duplicate the replacement before freeing the old name and return
-ENOMEM on failure. Propagate that error through config_load before
initializing iSNS or applying the remaining configuration.
The saved Cppcheck report flags the zero-length final send:
iscsi-scst/usr/af_alg.c:76: error: Null pointer dereference
[nullPointer]
Linux accepts a NULL buffer for this zero-length AF_ALG operation; this
report does not establish a runtime dereference. Pass an empty string
instead so the buffer contract is explicit without a model or waiver.
The length remains zero and MSG_MORE remains clear, preserving hash
finalization behavior.
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 alignment-dependent loads in scst_unpack_lun;
no saved analyzer diagnostic directly reports this defect. The helper
accepts a byte pointer without an alignment contract, then casts it to
16-, 32- or 64-bit pointers. In particular, a six-byte LUN reads a 32-bit
tail at offset two even when the start is aligned.
Use unaligned big-endian loads for each supported length while retaining
flat-address decoding and rejection of nonzero tail bytes. This preserves
the public interface and avoids faults on strict-alignment architectures.
Add KUnit coverage for all eight alignments and supported lengths,
nonzero bytes outside the supplied length, and every rejected tail byte.
The saved Cppcheck report warns about the formatter output:
scst/src/scst_main.c:637: warning: Uninitialized variable: buf
[uninitvar]
scst_dump_cdb does not write anything when cdb_len is zero, but
scst_trace_cmds prints its stack buffer as a string. Unknown opcode
groups can produce an empty CDB, so termination must not depend on
valid command contents.
Initialize nonempty output buffers and leave zero-capacity buffers
untouched. Add KUnit coverage for empty, ordinary, truncated, one-byte
and zero-size output. Include the new private test header in generated
kernel patches.
Manual review found a use-after-free in internal_portal validation; no
saved analyzer diagnostic directly reports this aliasing case. portal
points inside the allocation owned by portals, but the error branch
frees portals before passing portal to the logging function.
Keep the allocation alive until the invalid address has been logged.
Preserve the existing EINVAL response and cleanup path.
Add a Ginkgo management regression through scstadmin. Reject a list
starting with an invalid token, check that the daemon log contains the
intact token, and verify daemon liveness and the unchanged setting.
Manual review found division-by-zero and truncation bugs in REQUEST
SENSE during FORMAT UNIT. No saved analyzer diagnostic directly reports
these failures. do_div truncates the shifted 64-bit total to 32 bits:
small totals and totals such as 1ULL << 48 produce a zero divisor.
Completion can also wrap the 16-bit progress field back to zero.
Use bounded 64-bit scaling and saturate completion at 0xffff. Publish
initial counters with format_active and read/update them under
flags_lock, avoiding inconsistent or torn 64-bit counter snapshots.
Include the progress descriptor in the returned sense length.
Add twelve KUnit cases for both sense formats, unknown/small totals,
truncated divisors, large counters and completion. Exercise the actual
REQUEST SENSE handler and validate the returned descriptor length.
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.
Gleb Chesnokov