Commit 1a00f5c0 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

bdev/nvme: Fix overflow of RB tree comparison when the NSID is very big 



If 0 - UINT32_MAX or UINT32_MAX - 0 is substituted into a int variable,
we cannot get any expected result.

Fix the bug and add unit test case to verify the fix.

Signed-off-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Change-Id: Ib045273238753e16755328805b38569909c8b83a
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11836


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
parent b9a66791

module/bdev/nvme/bdev_nvme.c

+1 −1
Original line number Diff line number Diff line
@@ -200,7 +200,7 @@ static int nvme_ctrlr_read_ana_log_page(struct nvme_ctrlr *nvme_ctrlr); 
static int

nvme_ns_cmp(struct nvme_ns *ns1, struct nvme_ns *ns2)

{

	return ns1->id - ns2->id;

	return ns1->id < ns2->id ? -1 : ns1->id > ns2->id;

}



RB_GENERATE_STATIC(nvme_ns_tree, nvme_ns, node, nvme_ns_cmp);

test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c

+13 −0
Original line number Diff line number Diff line
@@ -5801,6 +5801,18 @@ test_fail_path(void) 
	free(bdev_io);

}



static void

test_nvme_ns_cmp(void)

{

	struct nvme_ns nvme_ns1 = {}, nvme_ns2 = {};



	nvme_ns1.id = 0;

	nvme_ns2.id = UINT32_MAX;



	CU_ASSERT(nvme_ns_cmp(&nvme_ns1, &nvme_ns2) < 0);

	CU_ASSERT(nvme_ns_cmp(&nvme_ns2, &nvme_ns1) > 0);

}



int

main(int argc, const char **argv)

{
@@ -5848,6 +5860,7 @@ main(int argc, const char **argv) 
	CU_ADD_TEST(suite, test_reconnect_ctrlr);

	CU_ADD_TEST(suite, test_retry_failover_ctrlr);

	CU_ADD_TEST(suite, test_fail_path);

	CU_ADD_TEST(suite, test_nvme_ns_cmp);



	CU_basic_set_mode(CU_BRM_VERBOSE);