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

[Photon/P1] Fixes handling of invalid WiFi access point config entries in DCT #1976

Merged
merged 3 commits into from
Dec 2, 2019

Conversation

avtolstoy
Copy link
Member

Problem

  1. See #1968, WiFi AP list gets initialized to 0xff instead of expected 0x00, which causes invalid (filled with 0xff) entries to be treated as valid credentials resulting in crashes
  2. There aren't any sanity checks on the WiFi AP entries retrieved from DCT

Solution

  1. Fill the stored AP list with 0x00 in factory DCT image file: dct_prep.bin. Previously this list should have been initialized with NVMEM_SPARK_Reset_SysFlag system flag set, however (unconfirmed, most likely) due to restructuring of the factory image in Changes to support building combined Photon and P1 images for the recent releases #1887 and NVMEM_SPARK_Reset_SysFlag being used to perform a factory reset in the bootloader (after which it's reset), the system no longer runs wlan_reset_credentials_store().
  2. Add sanity checks to wlan_hal.

Steps to Test

Sanity checks

  1. Flash https://github.com/particle-iot/device-os/files/3877387/dct_dump_full.bin.zip @ 0x08004000 using the debugger
  2. Flash stock DeviceOS
  3. The device should crash after failing to connect to the WiFi AP
  4. Flash DeviceOS out of this branch
  5. The device should not crash

Factory image

Follow the steps in #1887, get the device to boot into DeviceOS, put into DFU mode, get a full two-page DCT dump (dfu-util -d 2b04:c006 -s 0x08004000:0x8000 -a 0 -U dct_dump_full.bin), make sure that 0x4a30 - 0x4a90 and 0x8a30 - 0x8a90 are zero-initialized.

Example App

N/A

References

  • #1968
  • [CH42251]

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)

@sergeuz
Copy link
Member

sergeuz commented Nov 29, 2019

There's a check in wlan_hal which seems to be there to prevent this kind of issues, however, it doesn't work correctly after this change (device_configured evaluates to true if DCT is not initialized). Should we fix it in this PR as well?

@avtolstoy
Copy link
Member Author

Nice catch, @sergeuz. Yeah, we definitely need to fix it.

@avtolstoy
Copy link
Member Author

Fixed.

@avtolstoy avtolstoy added ready to merge PR has been reviewed and tested and removed needs review labels Dec 2, 2019
@avtolstoy avtolstoy merged commit b0e3f5a into develop Dec 2, 2019
@avtolstoy avtolstoy deleted the fix/ch42251-stored-ap-invalid-entries branch December 2, 2019 12:40
@avtolstoy avtolstoy added this to the 1.4.4 milestone Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ready to merge PR has been reviewed and tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants