From 2d7183ce8331132131ff629019e92a51615490c7 Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Tue, 20 Oct 2026 18:08:02 +0300 Subject: [PATCH] qla2x00t-32gbit: Reject exhausted bitmap tags before access 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. --- doc/KUNIT.md | 3 ++ .../kunit/suites/scst_qla_pool_test.c | 35 +++++++++++++++++++ qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c | 25 +++++++------ .../qla2x00-target/scst_qla2xxx_test.h | 2 ++ 4 files changed, 55 insertions(+), 10 deletions(-) diff --git a/doc/KUNIT.md b/doc/KUNIT.md index acf60483f..0f1af46e7 100644 --- a/doc/KUNIT.md +++ b/doc/KUNIT.md @@ -76,6 +76,9 @@ dependency. Use a disposable VM without a passed-through QLogic HBA: module initialization still runs. Fixtures stub target registration and check resource unwinding and destruction; hardware session teardown requires separate validation. +The fallback bitmap allocation and exhaustion cases also run on current kernels, +even though the normal driver uses sbitmap there. They check word boundaries, +allocation failure, a full or empty pool, and reuse of the last valid tag. The runner reads suite metadata and dependencies from the modules and loads selected owners in dependency order. Supply all required SCST modules from the same build; diff --git a/qla2x00t-32gbit/qla2x00-target/kunit/suites/scst_qla_pool_test.c b/qla2x00t-32gbit/qla2x00-target/kunit/suites/scst_qla_pool_test.c index 718e349ec..348cd9f3e 100644 --- a/qla2x00t-32gbit/qla2x00-target/kunit/suites/scst_qla_pool_test.c +++ b/qla2x00t-32gbit/qla2x00-target/kunit/suites/scst_qla_pool_test.c @@ -183,10 +183,45 @@ static void qla_bitmap_allocation_failure_test(struct kunit *test) KUNIT_EXPECT_PTR_EQ(test, f->bitmap, NULL); } +static void qla_bitmap_exhaustion_test(struct kunit *test) +{ + unsigned int tags = *(const unsigned int *)test->param_value; + unsigned int words = BITS_TO_LONGS(tags); + unsigned long *bitmap; + unsigned int i; + + /* A zero guard word makes an out-of-range claim observable without an OOB. */ + bitmap = kunit_kcalloc(test, words + 1, sizeof(*bitmap), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, bitmap); + for (i = 0; i < tags; i++) + KUNIT_EXPECT_EQ(test, sqa_get_tag_bitmap(bitmap, tags), (int)i); + + KUNIT_EXPECT_EQ(test, sqa_get_tag_bitmap(bitmap, tags), -ENOENT); + KUNIT_EXPECT_EQ(test, bitmap[words], 0UL); + if (tags % BITS_PER_LONG) + KUNIT_EXPECT_EQ(test, bitmap[words - 1], BITMAP_LAST_WORD_MASK(tags)); + + /* The last valid tag remains reusable after the pool was exhausted. */ + clear_bit(tags - 1, bitmap); + KUNIT_EXPECT_EQ(test, sqa_get_tag_bitmap(bitmap, tags), (int)tags - 1); + KUNIT_EXPECT_EQ(test, sqa_get_tag_bitmap(bitmap, tags), -ENOENT); + KUNIT_EXPECT_EQ(test, bitmap[words], 0UL); +} + +static void qla_bitmap_empty_pool_test(struct kunit *test) +{ + unsigned long guard = 0; + + KUNIT_EXPECT_EQ(test, sqa_get_tag_bitmap(&guard, 0), -ENOENT); + KUNIT_EXPECT_EQ(test, guard, 0UL); +} + static struct kunit_case qla_pool_test_cases[] = { KUNIT_CASE_PARAM(qla_pool_lifetime_test, qla_pool_gen_params), KUNIT_CASE_PARAM(qla_bitmap_allocation_test, qla_bitmap_gen_params), KUNIT_CASE(qla_bitmap_allocation_failure_test), + KUNIT_CASE_PARAM(qla_bitmap_exhaustion_test, qla_bitmap_gen_params), + KUNIT_CASE(qla_bitmap_empty_pool_test), {} }; diff --git a/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c b/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c index d569a2d1c..1f0f8bf36 100644 --- a/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c +++ b/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c @@ -239,6 +239,20 @@ int sqa_init_tag_bitmap(unsigned long **bitmap, unsigned int tag_num) return -ENOMEM; return 0; } + +SCST_KUNIT_VISIBLE +int sqa_get_tag_bitmap(unsigned long *bitmap, unsigned int tag_num) +{ + unsigned int tag; + + for (;;) { + tag = find_first_zero_bit(bitmap, tag_num); + if (tag >= tag_num) + return -ENOENT; + if (!test_and_set_bit(tag, bitmap)) + return tag; + } +} #endif SCST_KUNIT_VISIBLE @@ -482,16 +496,7 @@ static struct qla_tgt_cmd *sqa_qla2xxx_get_cmd(struct fc_port *sess) tag = sbitmap_queue_get(&sqa_tgt->tgt_tag_pool, &cpu); #else - for (;;) { - tag = find_first_zero_bit(sqa_tgt->tgt_tag_pool, - sqa_tgt->tag_num); - if (test_and_set_bit(tag, sqa_tgt->tgt_tag_pool) == 0) - break; - if (tag >= sqa_tgt->tag_num) { - tag = -ENOENT; - break; - } - } + tag = sqa_get_tag_bitmap(sqa_tgt->tgt_tag_pool, sqa_tgt->tag_num); #endif if (tag < 0) return NULL; diff --git a/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx_test.h b/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx_test.h index 308b4e12c..d272d59eb 100644 --- a/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx_test.h +++ b/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx_test.h @@ -21,6 +21,8 @@ struct sqa_scst_tgt *sqa_alloc_tgt(struct scst_tgt_template *tmplate, const char *name, unsigned int tag_num, size_t tag_size); void sqa_free_tgt(struct sqa_scst_tgt *sqa_tgt); + +int sqa_get_tag_bitmap(unsigned long *bitmap, unsigned int tag_num); #endif #endif /* SCST_QLA2XXX_TEST_H */