Skip to content

Commit

Permalink
test: Cast values to u32 if shift overflows int
Browse files Browse the repository at this point in the history 
Bit shifts that overflow the resulting type are undefined behavior in C.
C arithmetic promotes to ints all smaller integer types.
There are several places where a 32-bit unsigned value
is constructed by shifting a u8 or u16 to the most significant bits.
Since this overflows a signed 32-bit integer,
explicitly cast to u32 to avoid the UB.
Technically, an int is allowed to only be 16 bits,
so any shift that could set bit 15 or higher is UB.
But platforms where int is s16 are not very common,
so it's likely not worth the effort to fix the code.

Signed-off-by: Caleb Sander <[email protected]>
  • Loading branch information
@calebsander @igaw
calebsander authored and igaw committed Nov 2, 2023
1 parent 3ca94c0 commit d5962d8
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 19 deletions.
34 changes: 17 additions & 17 deletions test/ioctl/features.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static void test_set_features(void)
.nsid = TEST_NSID,
.in_data = data,
.data_len = sizeof(data),
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| TEST_FID,
.cdw11 = TEST_CDW11,
.cdw12 = TEST_CDW12,
Expand Down Expand Up @@ -163,7 +163,7 @@ static void test_set_features_simple(void)
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.nsid = TEST_NSID,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| TEST_FID,
.cdw11 = TEST_CDW11,
.result = TEST_RESULT,
Expand Down Expand Up @@ -205,7 +205,7 @@ static void test_set_arbitration(void)
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = NVME_FEAT_FID_ARBITRATION,
.cdw11 = HPW << 24 | MPW << 16 | LPW << 8 | AB,
.cdw11 = (uint32_t)HPW << 24 | MPW << 16 | LPW << 8 | AB,
.result = TEST_RESULT,
};
uint32_t result = 0;
Expand Down Expand Up @@ -243,7 +243,7 @@ static void test_set_power_mgmt(void)
uint8_t PS = 0b10101, WH = 0b101;
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_POWER_MGMT,
.cdw11 = WH << 5 | PS,
.result = TEST_RESULT,
Expand Down Expand Up @@ -337,7 +337,7 @@ static void test_set_temp_thresh(void)
NVME_FEATURE_TEMPTHRESH_THSEL_UNDER;
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_TEMP_THRESH,
.cdw11 = THSEL << 20 | TMPSEL << 16 | TMPTH,
.result = TEST_RESULT,
Expand Down Expand Up @@ -424,7 +424,7 @@ static void test_set_volatile_wc(void)
{
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_VOLATILE_WC,
.cdw11 = 1 << 0, /* WCE */
.result = TEST_RESULT,
Expand Down Expand Up @@ -521,7 +521,7 @@ static void test_set_irq_config(void)
uint16_t IV = 0x1234;
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_IRQ_CONFIG,
.cdw11 = 1 << 16 /* CD */
| IV,
Expand Down Expand Up @@ -600,7 +600,7 @@ static void test_set_async_event(void)
uint32_t EVENTS = 0x87654321;
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_ASYNC_EVENT,
.cdw11 = EVENTS,
.result = TEST_RESULT,
Expand Down Expand Up @@ -717,7 +717,7 @@ static void test_set_timestamp(void)
.opcode = nvme_admin_set_features,
.in_data = &ts,
.data_len = sizeof(ts),
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_TIMESTAMP,
};
int err;
Expand Down Expand Up @@ -771,7 +771,7 @@ static void test_set_hctm(void)
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = NVME_FEAT_FID_HCTM,
.cdw11 = TMT1 << 16 | TMT2,
.cdw11 = (uint32_t)TMT1 << 16 | TMT2,
.result = TEST_RESULT,
};
uint32_t result = 0;
Expand Down Expand Up @@ -807,7 +807,7 @@ static void test_set_nopsc(void)
{
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_NOPSC,
.cdw11 = 1 << 0 /* NOPPME */,
.result = TEST_RESULT,
Expand Down Expand Up @@ -890,7 +890,7 @@ static void test_set_plm_config(void)
.opcode = nvme_admin_set_features,
.in_data = &config,
.data_len = sizeof(config),
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_PLM_CONFIG,
.cdw11 = NVMSETID,
.cdw12 = 1 << 0 /* Predictable Latency Enable */,
Expand Down Expand Up @@ -984,7 +984,7 @@ static void test_set_lba_sts_interval(void)
uint16_t LSIRI = 0x1234, LSIPI = 0x5678;
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_LBA_STS_INTERVAL,
.cdw11 = LSIPI << 16 | LSIRI,
.result = TEST_RESULT,
Expand Down Expand Up @@ -1105,7 +1105,7 @@ static void test_set_endurance_evt_cfg(void)
uint8_t EGWARN = 0xCD;
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_ENDURANCE_EVT_CFG,
.cdw11 = EGWARN << 16 | ENDGID,
.result = TEST_RESULT,
Expand Down Expand Up @@ -1182,7 +1182,7 @@ static void test_set_sw_progress(void)
uint8_t PBSLC = 0xBA;
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_SW_PROGRESS,
.cdw11 = PBSLC,
.result = TEST_RESULT,
Expand Down Expand Up @@ -1223,7 +1223,7 @@ static void test_set_host_id(void)
.opcode = nvme_admin_set_features,
.in_data = hostid,
.data_len = sizeof(hostid),
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_HOST_ID,
.result = TEST_RESULT,
};
Expand Down Expand Up @@ -1305,7 +1305,7 @@ static void test_set_resv_mask(void)
struct mock_cmd mock_admin_cmd = {
.opcode = nvme_admin_set_features,
.nsid = TEST_NSID,
.cdw10 = 1 << 31 /* SAVE */
.cdw10 = (uint32_t)1 << 31 /* SAVE */
| NVME_FEAT_FID_RESV_MASK,
.cdw11 = MASK,
.result = TEST_RESULT,
Expand Down
7 changes: 5 additions & 2 deletions test/mi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1647,7 +1647,10 @@ static int test_admin_format_nvm_cb(struct nvme_mi_ep *ep,

assert(rq_hdr[4] == nvme_admin_format_nvm);

nsid = rq_hdr[11] << 24 | rq_hdr[10] << 16 | rq_hdr[9] << 8 | rq_hdr[8];
nsid = (__u32)rq_hdr[11] << 24
| rq_hdr[10] << 16
| rq_hdr[9] << 8
| rq_hdr[8];
assert(nsid == args->nsid);

assert(((rq_hdr[44] >> 0) & 0xf) == args->lbaf);
Expand Down Expand Up @@ -1722,7 +1725,7 @@ static int test_admin_sanitize_nvm_cb(struct nvme_mi_ep *ep,
assert(((rq_hdr[45] >> 0) & 0x1) == args->oipbp);
assert(((rq_hdr[45] >> 1) & 0x1) == args->nodas);

ovrpat = rq_hdr[51] << 24 | rq_hdr[50] << 16 |
ovrpat = (__u32)rq_hdr[51] << 24 | rq_hdr[50] << 16 |
rq_hdr[49] << 8 | rq_hdr[48];
assert(ovrpat == args->ovrpat);

Expand Down

0 comments on commit d5962d8

Please sign in to comment.