Skip to content

Commit 24b2abb

Browse files
authored
Merge pull request #2982 from hathach/fix-stringop-overread-warning
fix stringop-overread warning for msc device with memmove
2 parents 35e9f03 + 3ffe8db commit 24b2abb

File tree

1 file changed

+10
-11
lines changed

1 file changed

+10
-11
lines changed

src/class/msc/msc_device.c

+10-11
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ bool mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t
365365
memcpy(p_cbw, _mscd_epbuf.buf, sizeof(msc_cbw_t));
366366

367367
TU_LOG_DRV(" SCSI Command [Lun%u]: %s\r\n", p_cbw->lun, tu_lookup_find(&_msc_scsi_cmd_table, p_cbw->command[0]));
368-
//TU_LOG_MEM(MSC_DEBUG, p_cbw, xferred_bytes, 2);
368+
// TU_LOG_MEM(CFG_TUD_MSC_LOG_LEVEL, p_cbw, xferred_bytes, 2);
369369

370370
p_csw->signature = MSC_CSW_SIGNATURE;
371371
p_csw->tag = p_cbw->tag;
@@ -422,7 +422,7 @@ bool mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t
422422
} else if (resplen == 0) {
423423
if (p_cbw->total_bytes) {
424424
// 6.7 The 13 Cases: case 4 (Hi > Dn)
425-
// TU_LOG(MSC_DEBUG, " SCSI case 4 (Hi > Dn): %lu\r\n", p_cbw->total_bytes);
425+
// TU_LOG_DRV(" SCSI case 4 (Hi > Dn): %lu\r\n", p_cbw->total_bytes);
426426
fail_scsi_op(rhport, p_msc, MSC_CSW_STATUS_FAILED);
427427
} else {
428428
// case 1 Hn = Dn: all good
@@ -431,7 +431,7 @@ bool mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t
431431
} else {
432432
if (p_cbw->total_bytes == 0) {
433433
// 6.7 The 13 Cases: case 2 (Hn < Di)
434-
// TU_LOG(MSC_DEBUG, " SCSI case 2 (Hn < Di): %lu\r\n", p_cbw->total_bytes);
434+
// TU_LOG_DRV(" SCSI case 2 (Hn < Di): %lu\r\n", p_cbw->total_bytes);
435435
fail_scsi_op(rhport, p_msc, MSC_CSW_STATUS_FAILED);
436436
} else {
437437
// cannot return more than host expect
@@ -445,7 +445,8 @@ bool mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t
445445

446446
case MSC_STAGE_DATA:
447447
TU_LOG_DRV(" SCSI Data [Lun%u]\r\n", p_cbw->lun);
448-
//TU_LOG_MEM(MSC_DEBUG, _mscd_epbuf.buf, xferred_bytes, 2);
448+
TU_ASSERT(xferred_bytes <= CFG_TUD_MSC_EP_BUFSIZE); // sanity check to avoid buffer overflow
449+
// TU_LOG_MEM(CFG_TUD_MSC_LOG_LEVEL, _mscd_epbuf.buf, xferred_bytes, 2);
449450

450451
if (SCSI_CMD_READ_10 == p_cbw->command[0]) {
451452
p_msc->xferred_len += xferred_bytes;
@@ -492,7 +493,7 @@ bool mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t
492493
// Wait for the Status phase to complete
493494
if ((ep_addr == p_msc->ep_in) && (xferred_bytes == sizeof(msc_csw_t))) {
494495
TU_LOG_DRV(" SCSI Status [Lun%u] = %u\r\n", p_cbw->lun, p_csw->status);
495-
// TU_LOG_MEM(MSC_DEBUG, p_csw, xferred_bytes, 2);
496+
// TU_LOG_MEM(CFG_TUD_MSC_LOG_LEVEL, p_csw, xferred_bytes, 2);
496497

497498
// Invoke complete callback if defined
498499
// Note: There is racing issue with samd51 + qspi flash testing with arduino
@@ -532,7 +533,7 @@ bool mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t
532533
if (!usbd_edpt_stalled(rhport, p_msc->ep_in)) {
533534
if ((p_cbw->total_bytes > p_msc->xferred_len) && is_data_in(p_cbw->dir)) {
534535
// 6.7 The 13 Cases: case 5 (Hi > Di): STALL before status
535-
// TU_LOG(MSC_DEBUG, " SCSI case 5 (Hi > Di): %lu > %lu\r\n", p_cbw->total_bytes, p_msc->xferred_len);
536+
// TU_LOG_DRV(" SCSI case 5 (Hi > Di): %lu > %lu\r\n", p_cbw->total_bytes, p_msc->xferred_len);
536537
usbd_edpt_stall(rhport, p_msc->ep_in);
537538
} else {
538539
TU_ASSERT(send_csw(rhport, p_msc));
@@ -827,20 +828,18 @@ static void proc_write10_new_data(uint8_t rhport, mscd_interface_t* p_msc, uint3
827828
// update actual byte before failed
828829
p_msc->xferred_len += xferred_bytes;
829830

830-
// Set sense
831831
set_sense_medium_not_present(p_cbw->lun);
832-
833832
fail_scsi_op(rhport, p_msc, MSC_CSW_STATUS_FAILED);
834833
} else {
835-
// Application consume less than what we got (including zero)
836834
if ((uint32_t)nbytes < xferred_bytes) {
837-
uint32_t const left_over = xferred_bytes - (uint32_t)nbytes;
835+
// Application consume less than what we got (including zero)
836+
const uint32_t left_over = xferred_bytes - (uint32_t)nbytes;
838837
if (nbytes > 0) {
839838
p_msc->xferred_len += (uint16_t)nbytes;
840839
memmove(_mscd_epbuf.buf, _mscd_epbuf.buf + nbytes, left_over);
841840
}
842841

843-
// simulate an transfer complete with adjusted parameters --> callback will be invoked with adjusted parameter
842+
// simulate a transfer complete with adjusted parameters --> callback will be invoked with adjusted parameter
844843
dcd_event_xfer_complete(rhport, p_msc->ep_out, left_over, XFER_RESULT_SUCCESS, false);
845844
} else {
846845
// Application consume all bytes in our buffer

0 commit comments

Comments
 (0)