mirror of
https://github.com/SCST-project/scst.git
synced 2026-10-22 06:02:33 +00:00
scst_lib: Keep relative target port IDs within the valid range
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.
This commit is contained in:
@@ -7,6 +7,66 @@
|
||||
#include "scst.h"
|
||||
#endif
|
||||
|
||||
#include "../../../scst_lib_test.h"
|
||||
|
||||
struct scst_port_id_fixture {
|
||||
unsigned int free_id;
|
||||
unsigned int probes;
|
||||
bool invalid_id;
|
||||
};
|
||||
|
||||
static bool scst_port_id_available(u16 id, const struct scst_tgt *t)
|
||||
{
|
||||
struct kunit *test = kunit_get_current_test();
|
||||
struct scst_port_id_fixture *fixture = test->priv;
|
||||
|
||||
fixture->probes++;
|
||||
fixture->invalid_id |= id < SCST_MIN_REL_TGT_ID;
|
||||
return id == fixture->free_id;
|
||||
}
|
||||
|
||||
static void scst_port_id_boundaries(struct kunit *test)
|
||||
{
|
||||
static const struct {
|
||||
unsigned int start;
|
||||
unsigned int free_id;
|
||||
unsigned int next;
|
||||
unsigned int probes;
|
||||
int status;
|
||||
} cases[] = {
|
||||
{ 1, 1, 2, 1, 0 },
|
||||
{ 1, 3, 4, 3, 0 },
|
||||
{ SCST_MAX_REL_TGT_ID, SCST_MAX_REL_TGT_ID, 1, 1, 0 },
|
||||
{ SCST_MAX_REL_TGT_ID, 1, 2, 2, 0 },
|
||||
{ 42, 0, 42, SCST_MAX_REL_TGT_ID, -EOVERFLOW },
|
||||
};
|
||||
struct scst_port_id_fixture fixture;
|
||||
unsigned int i;
|
||||
|
||||
test->priv = &fixture;
|
||||
kunit_activate_static_stub(test, __scst_is_relative_target_port_id_unique,
|
||||
scst_port_id_available);
|
||||
for (i = 0; i < ARRAY_SIZE(cases); i++) {
|
||||
unsigned int next = cases[i].start;
|
||||
u16 id = 123;
|
||||
u16 expected_id = cases[i].free_id;
|
||||
|
||||
if (cases[i].status)
|
||||
expected_id = id;
|
||||
|
||||
memset(&fixture, 0, sizeof(fixture));
|
||||
fixture.free_id = cases[i].free_id;
|
||||
KUNIT_EXPECT_EQ(test, scst_alloc_relative_target_port_id(&id, &next),
|
||||
cases[i].status);
|
||||
KUNIT_EXPECT_EQ(test, next, cases[i].next);
|
||||
KUNIT_EXPECT_EQ(test, id, expected_id);
|
||||
KUNIT_EXPECT_EQ(test, fixture.probes, cases[i].probes);
|
||||
KUNIT_EXPECT_FALSE(test, fixture.invalid_id);
|
||||
}
|
||||
kunit_deactivate_static_stub(test, __scst_is_relative_target_port_id_unique);
|
||||
test->priv = NULL;
|
||||
}
|
||||
|
||||
static void scst_lun_flat_encoding(struct kunit *test)
|
||||
{
|
||||
/* Flat addressing: 01 in the top bits, followed by the 14-bit LUN. */
|
||||
@@ -20,6 +80,7 @@ static void scst_lun_flat_encoding(struct kunit *test)
|
||||
}
|
||||
|
||||
static struct kunit_case scst_lib_cases[] = {
|
||||
KUNIT_CASE(scst_port_id_boundaries),
|
||||
KUNIT_CASE(scst_lun_flat_encoding),
|
||||
{}
|
||||
};
|
||||
|
||||
+31
-18
@@ -4115,12 +4115,14 @@ restart:
|
||||
TRACE_EXIT();
|
||||
}
|
||||
|
||||
static bool __scst_is_relative_target_port_id_unique(uint16_t id,
|
||||
const struct scst_tgt *t)
|
||||
SCST_KUNIT_VISIBLE
|
||||
bool __scst_is_relative_target_port_id_unique(u16 id, const struct scst_tgt *t)
|
||||
{
|
||||
bool res = true;
|
||||
struct scst_tgt_template *tgtt;
|
||||
|
||||
SCST_KUNIT_REDIRECT(__scst_is_relative_target_port_id_unique, id, t);
|
||||
|
||||
TRACE_ENTRY();
|
||||
|
||||
list_for_each_entry(tgtt, &scst_template_list,
|
||||
@@ -4160,10 +4162,32 @@ bool scst_is_relative_target_port_id_unique(uint16_t id,
|
||||
return res;
|
||||
}
|
||||
|
||||
/* Caller serializes the cursor and target list with scst_mutex. */
|
||||
SCST_KUNIT_VISIBLE
|
||||
int scst_alloc_relative_target_port_id(u16 *id, unsigned int *next)
|
||||
{
|
||||
unsigned int candidate = *next;
|
||||
|
||||
do {
|
||||
bool available = __scst_is_relative_target_port_id_unique(candidate, NULL);
|
||||
unsigned int selected = candidate;
|
||||
|
||||
if (++candidate > SCST_MAX_REL_TGT_ID)
|
||||
candidate = SCST_MIN_REL_TGT_ID;
|
||||
if (available) {
|
||||
*id = selected;
|
||||
*next = candidate;
|
||||
return 0;
|
||||
}
|
||||
} while (candidate != *next);
|
||||
|
||||
return -EOVERFLOW;
|
||||
}
|
||||
|
||||
int gen_relative_target_port_id(uint16_t *id)
|
||||
{
|
||||
int res = -EOVERFLOW;
|
||||
static unsigned long rti = SCST_MIN_REL_TGT_ID, rti_prev;
|
||||
static unsigned int next = SCST_MIN_REL_TGT_ID;
|
||||
int res;
|
||||
|
||||
TRACE_ENTRY();
|
||||
|
||||
@@ -4171,21 +4195,10 @@ int gen_relative_target_port_id(uint16_t *id)
|
||||
if (res != 0)
|
||||
goto out;
|
||||
|
||||
rti_prev = rti;
|
||||
do {
|
||||
if (__scst_is_relative_target_port_id_unique(rti, NULL)) {
|
||||
*id = (uint16_t)rti++;
|
||||
res = 0;
|
||||
goto out_unlock;
|
||||
}
|
||||
rti++;
|
||||
if (rti > SCST_MAX_REL_TGT_ID)
|
||||
rti = SCST_MIN_REL_TGT_ID;
|
||||
} while (rti != rti_prev);
|
||||
res = scst_alloc_relative_target_port_id(id, &next);
|
||||
if (res)
|
||||
PRINT_ERROR("%s", "Unable to create unique relative target port id");
|
||||
|
||||
PRINT_ERROR("%s", "Unable to create unique relative target port id");
|
||||
|
||||
out_unlock:
|
||||
mutex_unlock(&scst_mutex);
|
||||
|
||||
out:
|
||||
|
||||
@@ -8,6 +8,8 @@
|
||||
|
||||
struct scst_write_same_priv;
|
||||
|
||||
bool __scst_is_relative_target_port_id_unique(u16 id, const struct scst_tgt *t);
|
||||
int scst_alloc_relative_target_port_id(u16 *id, unsigned int *next);
|
||||
int scst_ws_build_write_cdb(const struct scst_write_same_priv *wsp,
|
||||
u8 *write_cdb, s64 lba, int blocks);
|
||||
void scst_del_free_acg_dev(struct scst_acg_dev *acg_dev, bool del_sysfs);
|
||||
|
||||
Reference in New Issue
Block a user