From ea17dc18f5d2f24f3b7bfae18a262bab3a9a7ffe Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Tue, 20 Oct 2026 18:17:45 +0300 Subject: [PATCH] scst_lib: Start explicit buffer iteration at the supplied scatterlist 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. --- scst/include/scst.h | 2 +- scst/src/kunit/lib/suites/scst_kunit_lib.c | 60 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/scst/include/scst.h b/scst/include/scst.h index 6219c288e..93ee9329f 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -5016,7 +5016,7 @@ static inline int scst_get_sg_buf_first(struct scst_cmd *cmd, uint8_t **buf, return 0; } cmd->get_sg_buf_entry_num = 0; - cmd->get_sg_buf_cur_sg_entry = cmd->sg; + cmd->get_sg_buf_cur_sg_entry = sg; cmd->may_need_dma_sync = 1; return __scst_get_buf(cmd, sg_cnt, buf); } diff --git a/scst/src/kunit/lib/suites/scst_kunit_lib.c b/scst/src/kunit/lib/suites/scst_kunit_lib.c index 2dbb67da1..0747cc9f3 100644 --- a/scst/src/kunit/lib/suites/scst_kunit_lib.c +++ b/scst/src/kunit/lib/suites/scst_kunit_lib.c @@ -79,9 +79,69 @@ static void scst_lun_flat_encoding(struct kunit *test) (u64)0x1234); } +static void scst_explicit_sg_buffers(struct kunit *test) +{ + struct scatterlist command_sg, selected_sg[2]; + struct scst_cmd *cmd; + u8 *data, *buf; + unsigned int i; + + cmd = kunit_kzalloc(test, sizeof(*cmd), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, cmd); + data = kunit_kzalloc(test, 64, GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, data); + sg_init_one(&command_sg, data, 8); + sg_init_table(selected_sg, ARRAY_SIZE(selected_sg)); + sg_set_buf(&selected_sg[0], data + 8, 16); + sg_set_buf(&selected_sg[1], data + 24, 32); + cmd->sg_cnt = 1; + + /* An explicit list works with either another command list or none. */ + for (i = 0; i < 2; i++) { + cmd->sg = i ? NULL : &command_sg; + cmd->may_need_dma_sync = 0; + KUNIT_ASSERT_EQ(test, scst_get_sg_buf_first(cmd, &buf, selected_sg, 2), 16); + KUNIT_ASSERT_PTR_EQ(test, buf, data + 8); + KUNIT_EXPECT_EQ(test, cmd->get_sg_buf_entry_num, 1); + KUNIT_EXPECT_TRUE(test, cmd->may_need_dma_sync); + scst_put_sg_buf(cmd, buf, selected_sg, 2); + + KUNIT_ASSERT_EQ(test, scst_get_sg_buf_next(cmd, &buf, selected_sg, 2), 32); + KUNIT_ASSERT_PTR_EQ(test, buf, data + 24); + scst_put_sg_buf(cmd, buf, selected_sg, 2); + KUNIT_EXPECT_EQ(test, scst_get_sg_buf_next(cmd, &buf, selected_sg, 2), 0); + KUNIT_EXPECT_NULL(test, buf); + KUNIT_EXPECT_EQ(test, cmd->get_sg_buf_entry_num, 2); + } +} + +static void scst_empty_explicit_sg_buffers(struct kunit *test) +{ + struct scatterlist sg; + struct scst_cmd *cmd; + u8 *data, *buf; + + cmd = kunit_kzalloc(test, sizeof(*cmd), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, cmd); + data = kunit_kzalloc(test, 16, GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, data); + sg_init_one(&sg, data, 16); + cmd->sg = &sg; + cmd->sg_cnt = 1; + + buf = data; + KUNIT_EXPECT_EQ(test, scst_get_sg_buf_first(cmd, &buf, NULL, 0), 0); + KUNIT_EXPECT_NULL(test, buf); + buf = data; + KUNIT_EXPECT_EQ(test, scst_get_sg_buf_first(cmd, &buf, &sg, 0), 0); + KUNIT_EXPECT_NULL(test, buf); +} + static struct kunit_case scst_lib_cases[] = { KUNIT_CASE(scst_port_id_boundaries), KUNIT_CASE(scst_lun_flat_encoding), + KUNIT_CASE(scst_explicit_sg_buffers), + KUNIT_CASE(scst_empty_explicit_sg_buffers), {} };