Skip to content

Fix issues found using default CodeQL settings. #2923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix issues found using default CodeQL settings. #2923

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions src/device/usbd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,12 @@
tu_static uint8_t _app_driver_count = 0;

#define TOTAL_DRIVER_COUNT (_app_driver_count + BUILTIN_DRIVER_COUNT)
// Maximum total driver count is limited to `255u` / `0xFFu`.
// Although unlikely, should ever need >`255u` drivers:
// * Change `get_driver()` function to take `uint16_t` parameter.
// * Update callers of `get_driver()` to use `uint16_t` index variable.
// * Review all uses of index for potential overflow / underflow / etc.
#define MAXIMUM_TOTAL_DRIVER_COUNT (UINT8_MAX)

// virtually joins built-in and application drivers together.
// Application is positioned first to allow overwriting built-in ones.
Expand Down Expand Up @@ -401,7 +407,8 @@

// for usbd_control to print the name of control complete driver
void usbd_driver_print_control_complete_name(usbd_control_xfer_cb_t callback) {
for (uint8_t i = 0; i < TOTAL_DRIVER_COUNT; i++) {
for (int q = 0; (q < TOTAL_DRIVER_COUNT) && (q <= MAXIMUM_TOTAL_DRIVER_COUNT); q++) {
uint8_t i = (uint8_t)q;
usbd_class_driver_t const* driver = get_driver(i);
if (driver && driver->control_xfer_cb == callback) {
TU_LOG_USBD("%s control complete\r\n", driver->name);
Expand Down Expand Up @@ -488,10 +495,14 @@
// Get application driver if available
if (usbd_app_driver_get_cb) {
_app_driver = usbd_app_driver_get_cb(&_app_driver_count);
// See MAXIMUM_TOTAL_DRIVER_COUNT definition for details.
// Do not proceed if this invalid configuration is detected.
TU_ASSERT(TOTAL_DRIVER_COUNT <= MAXIMUM_TOTAL_DRIVER_COUNT);
}

// Init class drivers
for (uint8_t i = 0; i < TOTAL_DRIVER_COUNT; i++) {
for (int q = 0; (q < TOTAL_DRIVER_COUNT) && (q <= MAXIMUM_TOTAL_DRIVER_COUNT); q++) {
uint8_t i = (uint8_t)q;
usbd_class_driver_t const* driver = get_driver(i);
TU_ASSERT(driver && driver->init);
TU_LOG_USBD("%s init\r\n", driver->name);
Expand Down Expand Up @@ -520,7 +531,8 @@
dcd_deinit(rhport);

// Deinit class drivers
for (uint8_t i = 0; i < TOTAL_DRIVER_COUNT; i++) {
for (int q = 0; (q < TOTAL_DRIVER_COUNT) && (q <= MAXIMUM_TOTAL_DRIVER_COUNT); q++) {
uint8_t i = (uint8_t)q;
usbd_class_driver_t const* driver = get_driver(i);
if(driver && driver->deinit) {
TU_LOG_USBD("%s deinit\r\n", driver->name);
Expand All @@ -544,7 +556,8 @@
}

static void configuration_reset(uint8_t rhport) {
for (uint8_t i = 0; i < TOTAL_DRIVER_COUNT; i++) {
for (int q = 0; (q < TOTAL_DRIVER_COUNT) && (q <= MAXIMUM_TOTAL_DRIVER_COUNT); q++) {

Check warning

Code scanning / CodeQL

Comparison result is always the same Warning

Comparison is always true because q <= 255.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Belt and suspenders: Leave in the check for (q <= MAXIMUM_TOTAL_DRIVER_COUNT) ... this is not a bug.

uint8_t i = (uint8_t)q;
usbd_class_driver_t const* driver = get_driver(i);
TU_ASSERT(driver,);
driver->reset(rhport);
Expand Down Expand Up @@ -1011,9 +1024,10 @@

// Find driver for this interface
uint16_t const remaining_len = (uint16_t) (desc_end-p_desc);
uint8_t drv_id;
for (drv_id = 0; drv_id < TOTAL_DRIVER_COUNT; drv_id++)
int q;
for (q = 0; (q < TOTAL_DRIVER_COUNT) && (q <= MAXIMUM_TOTAL_DRIVER_COUNT); q++)
{
uint8_t drv_id = (uint8_t)q;
usbd_class_driver_t const *driver = get_driver(drv_id);
TU_ASSERT(driver);
uint16_t const drv_len = driver->open(rhport, desc_itf, remaining_len);
Expand Down Expand Up @@ -1061,7 +1075,7 @@
}

// Failed if there is no supported drivers
TU_ASSERT(drv_id < TOTAL_DRIVER_COUNT);
TU_ASSERT(q < TOTAL_DRIVER_COUNT);
}

return true;
Expand Down Expand Up @@ -1193,7 +1207,8 @@

case DCD_EVENT_SOF:
// SOF driver handler in ISR context
for (uint8_t i = 0; i < TOTAL_DRIVER_COUNT; i++) {
for (int q = 0; (q < TOTAL_DRIVER_COUNT) && (q <= MAXIMUM_TOTAL_DRIVER_COUNT); q++) {

Check warning

Code scanning / CodeQL

Comparison result is always the same Warning

Comparison is always true because q <= 255.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Belt and suspenders: Leave in the check for (q <= MAXIMUM_TOTAL_DRIVER_COUNT) ... this is not a bug.

uint8_t i = (uint8_t)q;
usbd_class_driver_t const* driver = get_driver(i);
if (driver && driver->sof) {
driver->sof(event->rhport, event->sof.frame_count);
Expand Down Expand Up @@ -1248,7 +1263,7 @@
// Parse consecutive endpoint descriptors (IN & OUT)
bool usbd_open_edpt_pair(uint8_t rhport, uint8_t const* p_desc, uint8_t ep_count, uint8_t xfer_type, uint8_t* ep_out, uint8_t* ep_in)
{
for(int i=0; i<ep_count; i++)
for(uint8_t i=0; i<ep_count; i++)
{
tusb_desc_endpoint_t const * desc_ep = (tusb_desc_endpoint_t const *) p_desc;

Expand Down
Loading