From e4a191c10995c674b372bea7989782334eb069ce Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Wed, 21 Oct 2026 09:48:04 +0300 Subject: [PATCH] isert-scst: Stop polling before indexing a failed CQ result 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. --- doc/KUNIT.md | 7 + iscsi-scst/Makefile | 11 +- iscsi-scst/kernel/isert-scst/Kbuild | 3 + iscsi-scst/kernel/isert-scst/iser_rdma.c | 41 ++++-- iscsi-scst/kernel/isert-scst/iser_rdma_test.h | 21 +++ .../isert-scst/kunit/suites/iser_cq_test.c | 135 ++++++++++++++++++ scripts/kunit/test_clean.py | 2 + scripts/kunit/test_generator.py | 1 + scripts/kunit/test_headers.py | 1 + scripts/kunit/test_make.py | 19 ++- scst/src/kunit/modules.mk | 3 +- scst/src/kunit/objects.mk | 3 + 12 files changed, 230 insertions(+), 17 deletions(-) create mode 100644 iscsi-scst/kernel/isert-scst/iser_rdma_test.h create mode 100644 iscsi-scst/kernel/isert-scst/kunit/suites/iser_cq_test.c diff --git a/doc/KUNIT.md b/doc/KUNIT.md index 0f1af46e7..6403da9e1 100644 --- a/doc/KUNIT.md +++ b/doc/KUNIT.md @@ -9,6 +9,7 @@ For GitHub Actions, see [CI](CI.md). ## Requirements Build against a configured and prepared kernel tree with `CONFIG_KUNIT=y/m`. +The iSER test module also requires `CONFIG_INFINIBAND=y/m`. Run tests in a disposable VM: ordinary SCST initialization discovers SCSI devices. The runner requires Python 3.11+, kmod utilities, native KUnit autorun control, `CONFIG_DEBUG_FS=y`, `CONFIG_KUNIT_DEBUGFS=y`, and debugfs mounted at @@ -68,8 +69,14 @@ files. Base labels come directly from suite registrations: | `scst_vdisk.ko` | `VDISK` | | `scst_user.ko` | `USER` | | `iscsi-scst.ko` | `ISCSI` | +| `isert-scst.ko` | `ISER` | | `qla2x00tgt.ko` | `QLA` | +`ISER` selects `scst-ISER-cq`, which tests CQ poll errors, batch sizes and +completion ordering inside `isert-scst.ko` using private RDMA fixtures. +No RDMA device is needed. Detecting out-of-bounds pointer formation on a +negative poll result additionally requires kernel `CONFIG_UBSAN_BOUNDS=y`. + `QLA` selects `scst-QLA-pool`, which tests the SCST-owned command/tag helpers inside `qla2x00tgt.ko`. The build and runner include its matching `qla2xxx_scst.ko` dependency. Use a disposable VM without a passed-through QLogic HBA: diff --git a/iscsi-scst/Makefile b/iscsi-scst/Makefile index 70723b1b0..6126987c5 100644 --- a/iscsi-scst/Makefile +++ b/iscsi-scst/Makefile @@ -144,16 +144,23 @@ mods: include/iscsi_scst_itf_ver.h $(CONFTEST_OUTPUTS) echo "$@: INFINIBAND_ENABLED = $(INFINIBAND_ENABLED)" if $(INFINIBAND_ENABLED); then \ echo " Building against $(OFED_FLAVOR) RDMA kernel headers.";\ - $(MAKE) -C $(KDIR) M=$(ISERTMOD) ISER_CFLAGS="$(ISER_CFLAGS)" \ + $(MAKE) -C $(KDIR) M=$(ISERTMOD) $(SCST_KUNIT_FLAGS) \ + ISER_CFLAGS="$(ISER_CFLAGS)" \ $(shell [ -n "$(PASS_CC_TO_MAKE)" ] && echo CC="$(CC)") \ $(OFED_CONFIG) modules; \ fi .PHONY: kunit-build -kunit-build: include/iscsi_scst_itf_ver.h conftest/sock_recvmsg/result-$(KVER).txt +kunit-build: include/iscsi_scst_itf_ver.h $(CONFTEST_OUTPUTS) + @if ! $(INFINIBAND_ENABLED); then \ + echo 'iSER KUnit tests require CONFIG_INFINIBAND=y or m' >&2; exit 1; \ + fi $(MAKE) -C $(KDIR) M=$(KMOD) $(SCST_KUNIT_FLAGS) \ CONFTEST_CFLAGS="$(CONFTEST_CFLAGS)" \ $(shell [ -n "$(PASS_CC_TO_MAKE)" ] && echo CC="$(CC)") iscsi-scst.ko + $(MAKE) -C $(KDIR) M=$(ISERTMOD) $(SCST_KUNIT_FLAGS) \ + ISER_CFLAGS="$(ISER_CFLAGS)" $(OFED_CONFIG) \ + $(shell [ -n "$(PASS_CC_TO_MAKE)" ] && echo CC="$(CC)") isert-scst.ko progs: include/iscsi_scst_itf_ver.h $(MAKE) -C usr diff --git a/iscsi-scst/kernel/isert-scst/Kbuild b/iscsi-scst/kernel/isert-scst/Kbuild index 94bb3e836..120afddee 100644 --- a/iscsi-scst/kernel/isert-scst/Kbuild +++ b/iscsi-scst/kernel/isert-scst/Kbuild @@ -40,3 +40,6 @@ ccflags-y := -I$(src)/../../../scst/include \ obj-m += isert-scst.o isert-scst-objs := isert.o isert_login.o \ iser_datamover.o iser_rdma.o iser_buf.o iser_pdu.o iser_global.o + +scst-kunit-module := isert-scst +include $(src)/../../../scst/src/kunit/Makefile diff --git a/iscsi-scst/kernel/isert-scst/iser_rdma.c b/iscsi-scst/kernel/isert-scst/iser_rdma.c index 53c0311c4..6caf5c991 100644 --- a/iscsi-scst/kernel/isert-scst/iser_rdma.c +++ b/iscsi-scst/kernel/isert-scst/iser_rdma.c @@ -47,6 +47,7 @@ #include "iser.h" #include "isert.h" #include "iser_datamover.h" +#include "iser_rdma_test.h" #define ISER_CQ_ENTRIES (128 * 1024) #define ISER_LISTEN_BACKLOG 8 @@ -466,11 +467,14 @@ static void isert_rdma_wr_completion_handler(struct isert_wr *wr) isert_data_in_sent(&wr->pdu->iscsi); } -static void isert_handle_wc(struct ib_wc *wc) +SCST_KUNIT_VISIBLE +void isert_handle_wc(struct ib_wc *wc) { struct isert_wr *wr = _u64_to_ptr(wc->wr_id); struct isert_conn *isert_conn; + SCST_KUNIT_REDIRECT(isert_handle_wc, wc); + TRACE_ENTRY(); switch (wr->wr_op) { @@ -639,16 +643,26 @@ void isert_sched_conn_free(struct isert_conn *isert_conn) isert_conn_queue_work(&isert_conn->release_work); } -static void isert_handle_wc_error(struct ib_wc *wc) +SCST_KUNIT_VISIBLE +void isert_handle_wc_error(struct ib_wc *wc) { - struct isert_wr *wr = _u64_to_ptr(wc->wr_id); - struct isert_cmnd *isert_pdu = wr->pdu; - struct isert_conn *isert_conn = wr->conn; - struct isert_buf *isert_buf = wr->buf; - struct isert_device *isert_dev = wr->isert_dev; - struct ib_device *ib_dev = isert_dev->ib_dev; + struct isert_wr *wr; + struct isert_cmnd *isert_pdu; + struct isert_conn *isert_conn; + struct isert_buf *isert_buf; + struct isert_device *isert_dev; + struct ib_device *ib_dev; u32 num_sge; + SCST_KUNIT_REDIRECT(isert_handle_wc_error, wc); + + wr = _u64_to_ptr(wc->wr_id); + isert_pdu = wr->pdu; + isert_conn = wr->conn; + isert_buf = wr->buf; + isert_dev = wr->isert_dev; + ib_dev = isert_dev->ib_dev; + TRACE_ENTRY(); if (wc->status != IB_WC_WR_FLUSH_ERR) @@ -714,15 +728,19 @@ static void isert_handle_wc_error(struct ib_wc *wc) TRACE_EXIT(); } -static int isert_poll_cq(struct isert_cq *cq) +SCST_KUNIT_VISIBLE +int isert_poll_cq(struct isert_cq *cq) { int err; struct ib_wc *wc, *last_wc; TRACE_ENTRY(); - do { + for (;;) { err = ib_poll_cq(cq->cq, ARRAY_SIZE(cq->wc), cq->wc); + if (err <= 0) + break; + last_wc = &cq->wc[err]; for (wc = cq->wc; wc < last_wc; ++wc) { if (likely(wc->status == IB_WC_SUCCESS)) @@ -730,8 +748,7 @@ static int isert_poll_cq(struct isert_cq *cq) else isert_handle_wc_error(wc); } - - } while (err > 0); + } TRACE_EXIT_RES(err); return err; diff --git a/iscsi-scst/kernel/isert-scst/iser_rdma_test.h b/iscsi-scst/kernel/isert-scst/iser_rdma_test.h new file mode 100644 index 000000000..4ae2e28d9 --- /dev/null +++ b/iscsi-scst/kernel/isert-scst/iser_rdma_test.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef SCST_ISER_RDMA_TEST_H +#define SCST_ISER_RDMA_TEST_H + +#ifdef INSIDE_KERNEL_TREE +#include "../../scst_test.h" +#else +#include "../../../scst/src/scst_test.h" +#endif + +#ifdef SCST_KUNIT_ENABLED + +struct ib_wc; +struct isert_cq; + +int isert_poll_cq(struct isert_cq *cq); +void isert_handle_wc(struct ib_wc *wc); +void isert_handle_wc_error(struct ib_wc *wc); + +#endif /* SCST_KUNIT_ENABLED */ +#endif /* SCST_ISER_RDMA_TEST_H */ diff --git a/iscsi-scst/kernel/isert-scst/kunit/suites/iser_cq_test.c b/iscsi-scst/kernel/isert-scst/kunit/suites/iser_cq_test.c new file mode 100644 index 000000000..0e1301030 --- /dev/null +++ b/iscsi-scst/kernel/isert-scst/kunit/suites/iser_cq_test.c @@ -0,0 +1,135 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include "../../../../../scst/src/kunit/scst_kunit.h" +#include "../../isert_dbg.h" +#include "../../iser.h" +#include "../../iser_rdma_test.h" + +struct iser_cq_case { + const char *name; + int results[3]; + unsigned int polls; + unsigned int completions; +}; + +static const struct iser_cq_case iser_cq_cases[] = { + { "initial-io-error", { -EIO }, 1, 0 }, + { "initial-invalid-error", { -EINVAL }, 1, 0 }, + { "empty", { 0 }, 1, 0 }, + { "full-then-empty", { ISER_SQ_SIZE, 0 }, 2, ISER_SQ_SIZE }, + { "full-short-empty", { ISER_SQ_SIZE, 2, 0 }, 3, ISER_SQ_SIZE + 2 }, + { "short-then-error", { 2, -EIO }, 2, 2 }, +}; + +struct iser_cq_fixture { + struct isert_cq cq; + struct ib_cq ib_cq; + struct ib_device device; + unsigned int polls; + unsigned int produced; + unsigned int completed; + unsigned int batch_start; + unsigned int successes; + unsigned int errors; +}; + +static int iser_cq_poll_stub(struct ib_cq *cq, int capacity, struct ib_wc *wc) +{ + struct kunit *test = kunit_get_current_test(); + const struct iser_cq_case *param = test->param_value; + struct iser_cq_fixture *f = test->priv; + int count, i; + + KUNIT_EXPECT_PTR_EQ(test, cq, &f->ib_cq); + KUNIT_EXPECT_PTR_EQ(test, wc, &f->cq.wc[0]); + KUNIT_EXPECT_EQ(test, capacity, (int)ARRAY_SIZE(f->cq.wc)); + KUNIT_EXPECT_EQ(test, f->completed, f->produced); + if (f->polls >= param->polls) { + KUNIT_FAIL(test, "polled after the terminal CQ result"); + return -EINVAL; + } + count = param->results[f->polls++]; + if (count > capacity) { + KUNIT_FAIL(test, "poll buffer is too small for the fixture"); + return -EINVAL; + } + f->batch_start = f->produced; + for (i = 0; i < count; i++) { + wc[i].wr_id = f->produced++; + wc[i].status = wc[i].wr_id & 1 ? IB_WC_WR_FLUSH_ERR : IB_WC_SUCCESS; + } + return count; +} + +static void iser_cq_complete(struct ib_wc *wc, bool success) +{ + struct kunit *test = kunit_get_current_test(); + struct iser_cq_fixture *f = test->priv; + + KUNIT_ASSERT_LT(test, f->completed, f->produced); + KUNIT_EXPECT_PTR_EQ(test, wc, &f->cq.wc[f->completed - f->batch_start]); + KUNIT_EXPECT_EQ(test, wc->wr_id, (u64)f->completed); + KUNIT_EXPECT_EQ(test, wc->status, success ? IB_WC_SUCCESS : IB_WC_WR_FLUSH_ERR); + KUNIT_EXPECT_EQ(test, success, !(f->completed & 1)); + f->completed++; + if (success) + f->successes++; + else + f->errors++; +} + +static void iser_cq_success_stub(struct ib_wc *wc) +{ + iser_cq_complete(wc, true); +} + +static void iser_cq_error_stub(struct ib_wc *wc) +{ + iser_cq_complete(wc, false); +} + +static int iser_cq_test_init(struct kunit *test) +{ + struct iser_cq_fixture *f; + + f = kunit_kzalloc(test, sizeof(*f), GFP_KERNEL); + if (!f) + return -ENOMEM; + test->priv = f; + f->cq.cq = &f->ib_cq; + f->ib_cq.device = &f->device; + f->device.ops.poll_cq = iser_cq_poll_stub; + kunit_activate_static_stub(test, isert_handle_wc, iser_cq_success_stub); + kunit_activate_static_stub(test, isert_handle_wc_error, iser_cq_error_stub); + return 0; +} + +static void iser_cq_case_name(const struct iser_cq_case *param, char *desc) +{ + strscpy(desc, param->name, KUNIT_PARAM_DESC_SIZE); +} + +KUNIT_ARRAY_PARAM(iser_cq, iser_cq_cases, iser_cq_case_name); + +static void iser_cq_poll_test(struct kunit *test) +{ + const struct iser_cq_case *param = test->param_value; + struct iser_cq_fixture *f = test->priv; + int ret; + + ret = isert_poll_cq(&f->cq); + KUNIT_EXPECT_EQ(test, ret, param->results[param->polls - 1]); + KUNIT_EXPECT_EQ(test, f->polls, param->polls); + KUNIT_EXPECT_EQ(test, f->completed, param->completions); + KUNIT_EXPECT_EQ(test, f->successes, (param->completions + 1) / 2); + KUNIT_EXPECT_EQ(test, f->errors, param->completions / 2); +} + +static struct kunit_case iser_cq_test_cases[] = { + KUNIT_CASE_PARAM(iser_cq_poll_test, iser_cq_gen_params), + {} +}; + +scst_kunit_suite(iser_cq_suite, "ISER", "-cq", + .init = iser_cq_test_init, + .test_cases = iser_cq_test_cases, +); diff --git a/scripts/kunit/test_clean.py b/scripts/kunit/test_clean.py index e3ead5901..cdc60b6bc 100644 --- a/scripts/kunit/test_clean.py +++ b/scripts/kunit/test_clean.py @@ -44,6 +44,8 @@ class CleanTests(unittest.TestCase): 'scst/src/dev_handlers/kunit/vdisk/suites/test.o', 'iscsi-scst/kernel/iscsi-scst.ko', 'iscsi-scst/kernel/kunit/suites/test.o', + 'iscsi-scst/kernel/isert-scst/isert-scst.ko', + 'iscsi-scst/kernel/isert-scst/kunit/suites/test.o', 'qla2x00t-32gbit/qla2xxx_scst.ko', 'qla2x00t-32gbit/qla_os.o', 'qla2x00t-32gbit/qla2x00-target/qla2x00tgt.ko', diff --git a/scripts/kunit/test_generator.py b/scripts/kunit/test_generator.py index c0e5d77be..eab4d7121 100644 --- a/scripts/kunit/test_generator.py +++ b/scripts/kunit/test_generator.py @@ -80,6 +80,7 @@ download_and_extract_kernel_tree() { owners = {'scst/src': 'drivers/scst', 'scst/src/dev_handlers': 'drivers/scst/dev_handlers', 'iscsi-scst/kernel': 'drivers/scst/iscsi-scst', + 'iscsi-scst/kernel/isert-scst': 'drivers/scst/iscsi-scst/isert-scst', 'qla2x00t-32gbit/qla2x00-target': 'drivers/scsi/qla2xxx'} for source_dir, target_dir in owners.items(): for header in (ROOT / source_dir).glob('*_test.h'): diff --git a/scripts/kunit/test_headers.py b/scripts/kunit/test_headers.py index e9264de06..0dfcfb1ce 100644 --- a/scripts/kunit/test_headers.py +++ b/scripts/kunit/test_headers.py @@ -11,6 +11,7 @@ ROOT = Path(__file__).resolve().parents[2] class HeaderTests(unittest.TestCase): def test_normal_headers_do_not_require_kernel_or_test_types(self): directories = ('scst/src', 'scst/src/dev_handlers', 'iscsi-scst/kernel', + 'iscsi-scst/kernel/isert-scst', 'qla2x00t-32gbit/qla2x00-target') headers = sorted(header for directory in directories for header in (ROOT / directory).glob('*_test.h')) diff --git a/scripts/kunit/test_make.py b/scripts/kunit/test_make.py index de0ac3250..b2eaa91ba 100644 --- a/scripts/kunit/test_make.py +++ b/scripts/kunit/test_make.py @@ -28,6 +28,8 @@ OWNERS = { 'scst_vdisk': ('scst/src/dev_handlers', 'scst/kernel/in-tree/Makefile.dev_handlers'), 'scst_user': ('scst/src/dev_handlers', 'scst/kernel/in-tree/Makefile.dev_handlers'), 'iscsi-scst': ('iscsi-scst/kernel', 'iscsi-scst/kernel/Makefile.in-kernel'), + 'isert-scst': ('iscsi-scst/kernel/isert-scst', + 'iscsi-scst/kernel/isert-scst/Makefile.in-kernel'), 'qla2x00tgt': ('qla2x00t-32gbit/qla2x00-target', 'qla2x00t-32gbit/Makefile_in-tree'), } @@ -239,6 +241,7 @@ class SelectionTests(unittest.TestCase): kdir = root / 'kernel' (kdir / 'include/config').mkdir(parents=True) (kdir / 'include/config/kernel.release').write_text('kunit-build-test\n') + (kdir / '.config').write_text('CONFIG_INFINIBAND=m\n') fields = ['$(patsubst $(KUNIT_TEST_ROOT)/%,%,$(M))', '$(KUNIT_LABEL)', '$(KDIR)', '$(KVER)', '$(ARCH)', '$(CROSS_COMPILE)', '$(CC)', '$(PASS_CC_TO_MAKE)', '$(WERROR)'] @@ -249,10 +252,12 @@ class SelectionTests(unittest.TestCase): 'built_module = $(if $(filter all,$@),' '$(if $(filter $(qla_root),$(M)),qla2xxx_scst.ko,' '$(if $(filter $(qla_root)/qla2x00-target,$(M)),qla2x00tgt.ko,all)),$@)\n' - '.PHONY: all scst.ko scst_vdisk.ko scst_user.ko iscsi-scst.ko\n' - 'all scst.ko scst_vdisk.ko scst_user.ko iscsi-scst.ko:\n' + '.PHONY: all scst.ko scst_vdisk.ko scst_user.ko iscsi-scst.ko isert-scst.ko\n' + 'all scst.ko scst_vdisk.ko scst_user.ko iscsi-scst.ko isert-scst.ko:\n' '\t@test "$(M)" = "$(KUNIT_TEST_ROOT)/scst/src" || ' 'test -f "$(KUNIT_TEST_ROOT)/scst/src/Module.symvers"\n' + '\t@test "$(M)" != "$(KUNIT_TEST_ROOT)/iscsi-scst/kernel/isert-scst" || ' + 'test -f "$(KUNIT_TEST_ROOT)/iscsi-scst/kernel/Module.symvers"\n' '\t@test "$(M)" != "$(qla_root)/qla2x00-target" || ' 'test -f "$(qla_root)/Module.symvers"\n' '\t@test "$(M)" != "$(qla_root)" || ' @@ -331,6 +336,16 @@ class SelectionTests(unittest.TestCase): result, rows, modules, _, _ = self.build_components('KUNIT_LABEL=' + selection) self.assert_full_build(result, rows, modules) + def test_iser_requires_rdma_and_propagates_build_failures(self): + for options in ({'options': ('INFINIBAND_ENABLED=false',)}, + {'fail': 'iscsi-scst/kernel/isert-scst'}): + with self.subTest(options=options): + result, _, modules, events, _ = self.build_components( + *options.get('options', ()), fail=options.get('fail', '')) + self.assertNotEqual(result.returncode, 0) + self.assertIsNone(modules) + self.assertNotIn('runner', events) + def test_normal_build_keeps_tests_disabled(self): for selection in (None, 'LIB'): result, rows, _, events, _ = self.build_components(goals=('scst',), selection=selection) diff --git a/scst/src/kunit/modules.mk b/scst/src/kunit/modules.mk index b5ec31452..2a8e94b9a 100644 --- a/scst/src/kunit/modules.mk +++ b/scst/src/kunit/modules.mk @@ -8,7 +8,8 @@ SCST_KUNIT_MODULES_scst := \ scst/src/dev_handlers/scst_user.ko SCST_KUNIT_MODULES_iscsi-scst := \ - iscsi-scst/kernel/iscsi-scst.ko + iscsi-scst/kernel/iscsi-scst.ko \ + iscsi-scst/kernel/isert-scst/isert-scst.ko SCST_KUNIT_MODULES_qla2x00t-32gbit/qla2x00-target := \ qla2x00t-32gbit/qla2xxx_scst.ko \ diff --git a/scst/src/kunit/objects.mk b/scst/src/kunit/objects.mk index 2fcafc721..ceef218ab 100644 --- a/scst/src/kunit/objects.mk +++ b/scst/src/kunit/objects.mk @@ -23,6 +23,9 @@ scst-kunit-iscsi-scst := \ kunit/suites/iscsi_close_test.o \ kunit/suites/iscsi_tx_test.o +scst-kunit-isert-scst := \ + kunit/suites/iser_cq_test.o + scst-kunit-qla2x00tgt := \ kunit/suites/scst_qla_pool_test.o