From 9671b117cbe9dc4cbee92d67c27a4c4a2b0a0095 Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Tue, 20 Oct 2026 18:07:35 +0300 Subject: [PATCH] qla2x00t-32gbit: Allocate and validate the full tag bitmap 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. --- .../kunit/suites/scst_qla_pool_test.c | 46 +++++++++++++++++++ qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c | 21 ++++++++- .../qla2x00-target/scst_qla2xxx_test.h | 2 + 3 files changed, 67 insertions(+), 2 deletions(-) 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 5ed1a37ed..718e349ec 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 @@ -8,6 +8,7 @@ struct qla_pool_fixture { struct scst_tgt target; struct scst_tgt_template tmplate; struct sqa_scst_tgt *allocated; + unsigned long *bitmap; unsigned int maps_freed; unsigned int pools_freed; unsigned int registrations; @@ -77,6 +78,7 @@ static void qla_pool_exit(struct kunit *test) if (f->allocated) sqa_free_tgt(f->allocated); + kfree(f->bitmap); } enum qla_pool_failure { @@ -139,8 +141,52 @@ static void qla_pool_lifetime_test(struct kunit *test) KUNIT_EXPECT_EQ(test, f->registrations, p->registrations); } +static const unsigned int qla_bitmap_sizes[] = { + 1, BITS_PER_LONG, BITS_PER_LONG + 1, SQA_DEFAULT_TAGS, +}; + +static void qla_bitmap_desc(const unsigned int *size, char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%u_tags", *size); +} + +KUNIT_ARRAY_PARAM(qla_bitmap, qla_bitmap_sizes, qla_bitmap_desc); + +static void qla_bitmap_allocation_test(struct kunit *test) +{ + unsigned int tags = *(const unsigned int *)test->param_value; + struct qla_pool_fixture *f = test->priv; + size_t bytes = BITS_TO_LONGS(tags) * sizeof(*f->bitmap); + unsigned int i; + + KUNIT_ASSERT_EQ(test, sqa_init_tag_bitmap(&f->bitmap, tags), 0); + KUNIT_ASSERT_NOT_NULL(test, f->bitmap); + /* Check the allocation before touching the high bits, even without KASAN. */ + KUNIT_ASSERT_GE(test, ksize(f->bitmap), bytes); + for (i = 0; i < BITS_TO_LONGS(tags); i++) + KUNIT_EXPECT_EQ(test, f->bitmap[i], 0UL); + set_bit(tags - 1, f->bitmap); + KUNIT_EXPECT_TRUE(test, test_bit(tags - 1, f->bitmap)); +} + +static unsigned long *qla_bitmap_no_memory(unsigned int tag_num) +{ + return NULL; +} + +static void qla_bitmap_allocation_failure_test(struct kunit *test) +{ + struct qla_pool_fixture *f = test->priv; + + kunit_activate_static_stub(test, sqa_alloc_tag_bitmap, qla_bitmap_no_memory); + KUNIT_EXPECT_EQ(test, sqa_init_tag_bitmap(&f->bitmap, SQA_DEFAULT_TAGS), -ENOMEM); + KUNIT_EXPECT_PTR_EQ(test, f->bitmap, NULL); +} + 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), {} }; diff --git a/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c b/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c index c0b9c496d..d569a2d1c 100644 --- a/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c +++ b/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c @@ -223,6 +223,24 @@ void sqa_free_cmd_map(void *map) kvfree(map); } +#if (!QLT_USE_PERCPU_IDA && !QLT_USE_SBITMAP) || defined(SCST_KUNIT_ENABLED) +SCST_KUNIT_VISIBLE +unsigned long *sqa_alloc_tag_bitmap(unsigned int tag_num) +{ + SCST_KUNIT_REDIRECT(sqa_alloc_tag_bitmap, tag_num); + return kcalloc(BITS_TO_LONGS(tag_num), sizeof(unsigned long), GFP_KERNEL); +} + +SCST_KUNIT_VISIBLE +int sqa_init_tag_bitmap(unsigned long **bitmap, unsigned int tag_num) +{ + *bitmap = sqa_alloc_tag_bitmap(tag_num); + if (!*bitmap) + return -ENOMEM; + return 0; +} +#endif + SCST_KUNIT_VISIBLE int sqa_init_tag_pool(struct sqa_scst_tgt *sqa_tgt, unsigned int tag_num) { @@ -234,8 +252,7 @@ int sqa_init_tag_pool(struct sqa_scst_tgt *sqa_tgt, unsigned int tag_num) false, GFP_KERNEL, NUMA_NO_NODE); #else sqa_tgt->tag_num = tag_num; - sqa_tgt->tgt_tag_pool = kzalloc(BITS_TO_LONGS(tag_num), GFP_KERNEL); - return PTR_ERR_OR_ZERO(sqa_tgt->tgt_tag_pool); + return sqa_init_tag_bitmap(&sqa_tgt->tgt_tag_pool, tag_num); #endif } diff --git a/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx_test.h b/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx_test.h index 738116cee..308b4e12c 100644 --- a/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx_test.h +++ b/qla2x00t-32gbit/qla2x00-target/scst_qla2xxx_test.h @@ -15,6 +15,8 @@ int sqa_init_tag_pool(struct sqa_scst_tgt *sqa_tgt, unsigned int tag_num); void sqa_destroy_tag_pool(struct sqa_scst_tgt *sqa_tgt); struct scst_tgt *sqa_register_target(struct scst_tgt_template *tmplate, const char *name); +unsigned long *sqa_alloc_tag_bitmap(unsigned int tag_num); +int sqa_init_tag_bitmap(unsigned long **bitmap, unsigned int tag_num); struct sqa_scst_tgt *sqa_alloc_tgt(struct scst_tgt_template *tmplate, const char *name, unsigned int tag_num, size_t tag_size);