Skip to content

Commit

Permalink
fix: Replace device calls to memcpy with tu_memcpy_s
Browse files Browse the repository at this point in the history
Introduces a new function tu_memcpy_s, which is effectively
a backport of memcpy_s. The change also refactors calls
to memcpy over to the more secure tu_memcpy_s.
  • Loading branch information
silvergasp committed Jan 13, 2023
1 parent 8775d55 commit 326cca5
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 20 deletions.
7 changes: 2 additions & 5 deletions src/class/audio/audio_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -823,10 +823,7 @@ uint16_t tud_audio_int_ctr_n_write(uint8_t func_id, uint8_t const* buffer, uint1
// We write directly into the EP's buffer - abort if previous transfer not complete
TU_VERIFY(!usbd_edpt_busy(_audiod_fct[func_id].rhport, _audiod_fct[func_id].ep_int_ctr));

// Check length
TU_VERIFY(len <= CFG_TUD_AUDIO_INT_CTR_EP_IN_SW_BUFFER_SIZE);

memcpy(_audiod_fct[func_id].ep_int_ctr_buf, buffer, len);
TU_VERIFY(tu_memcpy_s(_audiod_fct[func_id].ep_int_ctr_buf, CFG_TUD_AUDIO_INT_CTR_EP_IN_SW_BUFFER_SIZE, buffer, len)==0);

// Schedule transmit
TU_VERIFY(usbd_edpt_xfer(_audiod_fct[func_id].rhport, _audiod_fct[func_id].ep_int_ctr, _audiod_fct[func_id].ep_int_ctr_buf, len));
Expand Down Expand Up @@ -2202,7 +2199,7 @@ bool tud_audio_buffer_and_schedule_control_xfer(uint8_t rhport, tusb_control_req
if (len > _audiod_fct[func_id].ctrl_buf_sz) len = _audiod_fct[func_id].ctrl_buf_sz;

// Copy into buffer
memcpy((void *)_audiod_fct[func_id].ctrl_buf, data, (size_t)len);
TU_VERIFY(tu_memcpy_s(_audiod_fct[func_id].ctrl_buf, sizeof(_audiod_fct[func_id].ctrl_buf), data, (size_t)len)==0);

// Schedule transmit
return tud_control_xfer(rhport, p_request, (void*)_audiod_fct[func_id].ctrl_buf, len);
Expand Down
2 changes: 1 addition & 1 deletion src/class/dfu/dfu_rt_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ bool dfu_rtd_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request
TU_LOG2(" DFU RT Request: GETSTATUS\r\n");
dfu_status_response_t resp;
// Status = OK, Poll timeout is ignored during RT, State = APP_IDLE, IString = 0
memset(&resp, 0x00, sizeof(dfu_status_response_t));
TU_VERIFY(tu_memset_s(&resp, sizeof(resp), 0x00, sizeof(resp))==0);
tud_control_xfer(rhport, request, &resp, sizeof(dfu_status_response_t));
}
break;
Expand Down
18 changes: 7 additions & 11 deletions src/class/hid/hid_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,12 @@ bool tud_hid_n_report(uint8_t instance, uint8_t report_id, void const* report, u
// prepare data
if (report_id)
{
len = tu_min16(len, CFG_TUD_HID_EP_BUFSIZE-1);

p_hid->epin_buf[0] = report_id;
memcpy(p_hid->epin_buf+1, report, len);
TU_VERIFY(tu_memcpy_s(p_hid->epin_buf+1, CFG_TUD_HID_EP_BUFSIZE-1, report, len)==0);
len++;
}else
{
// If report id = 0, skip ID field
len = tu_min16(len, CFG_TUD_HID_EP_BUFSIZE);
memcpy(p_hid->epin_buf, report, len);
TU_VERIFY(tu_memcpy_s(p_hid->epin_buf, CFG_TUD_HID_EP_BUFSIZE, report, len)==0);
}

return usbd_edpt_xfer(rhport, p_hid->ep_in, p_hid->epin_buf, len);
Expand All @@ -126,7 +122,7 @@ bool tud_hid_n_keyboard_report(uint8_t instance, uint8_t report_id, uint8_t modi

if ( keycode )
{
memcpy(report.keycode, keycode, 6);
TU_VERIFY(tu_memcpy_s(report.keycode, sizeof(report.keycode), keycode, sizeof(report.keycode))==0);
}else
{
tu_memclr(report.keycode, 6);
Expand All @@ -151,8 +147,7 @@ bool tud_hid_n_mouse_report(uint8_t instance, uint8_t report_id,
}

bool tud_hid_n_gamepad_report(uint8_t instance, uint8_t report_id,
int8_t x, int8_t y, int8_t z, int8_t rz, int8_t rx, int8_t ry, uint8_t hat, uint32_t buttons)
{
int8_t x, int8_t y, int8_t z, int8_t rz, int8_t rx, int8_t ry, uint8_t hat, uint32_t buttons) {
hid_gamepad_report_t report =
{
.x = x,
Expand Down Expand Up @@ -183,11 +178,12 @@ void hidd_reset(uint8_t rhport)
}

uint16_t hidd_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t max_len)
{
{
TU_VERIFY(TUSB_CLASS_HID == desc_itf->bInterfaceClass, 0);

// len = interface + hid + n*endpoints
uint16_t const drv_len = (uint16_t) (sizeof(tusb_desc_interface_t) + sizeof(tusb_hid_descriptor_hid_t) +
uint16_t const drv_len =
(uint16_t) (sizeof(tusb_desc_interface_t) + sizeof(tusb_hid_descriptor_hid_t) +
desc_itf->bNumEndpoints * sizeof(tusb_desc_endpoint_t));
TU_ASSERT(max_len >= drv_len, 0);

Expand Down
2 changes: 1 addition & 1 deletion src/class/midi/midi_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ uint32_t tud_midi_n_stream_read(uint8_t itf, uint8_t cable_num, void* buffer, ui
uint8_t const count = (uint8_t) tu_min32(stream->total - stream->index, bufsize);

// Skip the header (1st byte) in the buffer
memcpy(buf8, stream->buffer + 1 + stream->index, count);
TU_VERIFY(tu_memcpy_s(buf8, bufsize, stream->buffer + 1 + stream->index, count)==0);

total_read += count;
stream->index += count;
Expand Down
2 changes: 1 addition & 1 deletion src/class/msc/msc_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ static int32_t proc_builtin_scsi(uint8_t lun, uint8_t const scsi_cmd[16], uint8_
read_capa10.block_size = tu_htonl(block_size);

resplen = sizeof(read_capa10);
memcpy(buffer, &read_capa10, (size_t) resplen);
TU_VERIFY(tu_memcpy_s(buffer, bufsize, &read_capa10, (size_t) resplen));
}
}
break;
Expand Down
17 changes: 17 additions & 0 deletions src/common/tusb_common.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#include "common/tusb_common.h"

int32_t tu_memset_s(void *dest, size_t destsz, int ch, size_t count) {
if (count > destsz) {
return -1;
}
memset(dest, ch, count);
return 0;
}

int32_t tu_memcpy_s(void *dest, size_t destsz, const void *src, size_t count) {
if (count > destsz) {
return -1;
}
memcpy(dest, src, count);
return 0;
}
8 changes: 8 additions & 0 deletions src/common/tusb_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@
#define tu_memclr(buffer, size) memset((buffer), 0, (size))
#define tu_varclr(_var) tu_memclr(_var, sizeof(*(_var)))

// This is a backport of memset_s from c11
int32_t tu_memset_s(void *dest, size_t destsz, int ch, size_t count);

// This is a backport of memcpy_s from c11
int32_t tu_memcpy_s(void *dest, size_t destsz,
const void * src, size_t count );


//------------- Bytes -------------//
TU_ATTR_ALWAYS_INLINE static inline uint32_t tu_u32(uint8_t b3, uint8_t b2, uint8_t b1, uint8_t b0)
{
Expand Down
23 changes: 22 additions & 1 deletion src/tusb.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
* This file is part of the TinyUSB stack.
*/

#include "common/tusb_common.h"
#include "tusb_option.h"

#if CFG_TUH_ENABLED || CFG_TUD_ENABLED
Expand Down Expand Up @@ -460,7 +461,7 @@ void tu_print_mem(void const *buf, uint32_t count, uint8_t indent)
tu_printf("%04X: ", 16*i/item_per_line);
}

memcpy(&value, buf8, size);
tu_memcpy_s(&value, sizeof(value), buf8, size);
buf8 += size;

tu_printf(" ");
Expand All @@ -486,3 +487,23 @@ void tu_print_mem(void const *buf, uint32_t count, uint8_t indent)
#endif

#endif // host or device enabled

//--------------------------------------------------------------------+
// Common
//--------------------------------------------------------------------+

int32_t tu_memset_s(void *dest, size_t destsz, int ch, size_t count) {
if (count > destsz) {
return -1;
}
memset(dest, ch, count);
return 0;
}

int32_t tu_memcpy_s(void *dest, size_t destsz, const void *src, size_t count) {
if (count > destsz) {
return -1;
}
memcpy(dest, src, count);
return 0;
}

0 comments on commit 326cca5

Please sign in to comment.