Skip to content
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

[gen3] hal: zero out invalid user modules #2374

Merged
merged 3 commits into from
Nov 5, 2021

Conversation

avtolstoy
Copy link
Member

Problem

Devices may present invalid user modules in system describe which can appear as some other type of module with potentially a set of invalid dependencies which cannot be satisfied preventing OTA from working correctly.

This happens because we ignore integrity or any other checks when making a decision on whether to include read-out module data in system describe. This is more problematic with Gen 3 devices running 3.1.0+ with 256KB applications as the compat (128KB) module header location would be in the middle of the larger binary. If by chance bytes at that location contain something seemingly valid (e.g. correct module type), this erroneous module would be reported in the describe message. If there is additionally a seemingly valid dependency, this is going to prevent product updates from happening, as DS will not be able to satisfy it.

Solution

Zero out module data if we've fetched an invalid module from module_user or module_user_compat locations.

Steps to Test

wiring/gen3_invalid_compat_user_app

References

N/A


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@avtolstoy avtolstoy added this to the 3.2.0 milestone Nov 3, 2021
@avtolstoy avtolstoy added the bug label Nov 3, 2021
@keeramis
Copy link
Contributor

keeramis commented Nov 4, 2021

Do we also need to modify these tests accordingly? Perhaps the info collected from HAL_System_Info is now different

Assertion failed: (factory == nullptr=1) == (false=0), file tests/integration/wiring/no_fixture/system.cpp, line 168.
        3) SYSTEM_06_system_describe_is_not_overflowed_when_factory_module_present
        Assertion failed: (factory == nullptr=1) == (false=0), file tests/integration/wiring/no_fixture/system.cpp, line 261.
        4) SYSTEM_07_system_describe_is_not_overflowed_when_factory_module_present_but_invalid

@avtolstoy
Copy link
Member Author

@keeramis Good catch, we should actually also check that we are checking against MODULE_STORE_MAIN. Thanks

@avtolstoy
Copy link
Member Author

Fixed

@avtolstoy avtolstoy merged commit 6294043 into develop Nov 5, 2021
@avtolstoy avtolstoy deleted the fix/gen3-invalid-user-module branch November 5, 2021 23:22
@avtolstoy avtolstoy restored the fix/gen3-invalid-user-module branch November 5, 2021 23:29
avtolstoy added a commit that referenced this pull request Nov 5, 2021
@avtolstoy avtolstoy deleted the fix/gen3-invalid-user-module branch November 5, 2021 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants