-
Notifications
You must be signed in to change notification settings - Fork 151
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.
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
fix(modem): handle nullptr in DTE constructors to prevent invalid access (IDFGH-14688) #768
fix(modem): handle nullptr in DTE constructors to prevent invalid access (IDFGH-14688) #768
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix, LGTM!
Could you please just update the commit message prefix -> fix(modem): ...
?
(or install the pre-commit hook)
@deodatomatheus Please check also the CI failures, above, |
0f0a4cb
to
1a43f17
Compare
1a43f17
to
95b5660
Compare
@david-cermak i changed the code to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, LGTM!
Description
The issue can be reproduced by setting a timeout for waiting for a USB connection (esp_modem_usb_term_config.timeou_ms). When the timeout occurs, the value of
primary_term
becomes null.If either primary_term or secondary_term is
nullptr
, the set_command_callbacks function attempts to access invalid values. For example, in the first line of the function,primary_term->set_read_cb([this](uint8_t *data, size_t len)
, the program tries to dereference primary_term, leading to aLoadProhibited
exception and crashing the application.To address this, I added a validation check in the DTE constructor to ensure the values are valid. Additionally, I handled the exception in the standard exception handler.
Related
Testing
I test the changes in a ESP32S3.
Checklist
Before submitting a Pull Request, please ensure the following: