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

WINGFC doesn't boot in master (likely H7 regression) #6382

Closed
alteman opened this issue Dec 6, 2020 · 6 comments · Fixed by #6384
Closed

WINGFC doesn't boot in master (likely H7 regression) #6382

alteman opened this issue Dec 6, 2020 · 6 comments · Fixed by #6384
Assignees
Milestone

Comments

@alteman
Copy link

alteman commented Dec 6, 2020

Current Behavior

WINGFC target doesn't boot after commit e72e702 (according to git bisect)
USB not detected, LEDs don't flash

Steps to Reproduce

  1. git clone
  2. ./build.sh WINGFC
  3. Flash the firmware
  4. Reboot the board

Expected behavior

The board boots, USB is detected, LEDs flash

Suggested solution(s)

??

  • FC Board name and vendor:
    Wing FC 10DOF (Furious FPV F-35 Lightning clone)
  • INAV version string:
    e72e702

ATTN @digitalentity

PS. This board doesn't have SWD pads, I might consider soldering to pins at some point if necessary..

@stronnag
Copy link
Collaborator

stronnag commented Dec 6, 2020

This is not entirely unexpected; #5827 states "Broken stuff and missing features will be addressed in future PRs". H7 support was merged early to avoid complex future merges, not because everything was working.

I have two F405 boards, one of which boots, one doesn't. The one that boots passes the "hover in the garden" test though I'm not convinced I'd fly it further than that.

As you noted, reverting the iniital H7 commit facilitates booting again.

@digitalentity digitalentity self-assigned this Dec 6, 2020
@digitalentity digitalentity added this to the 2.7 milestone Dec 6, 2020
@digitalentity
Copy link
Member

My debug rig with OMNIBUSF4PRO doesn't boot either. Looking into this

@digitalentity
Copy link
Member

Hardfaulting in readEEPROM(). Weird.

@digitalentity
Copy link
Member

Ok, not readEEPROM() but findEEPROM(). Now let's see why it's reading beyond valid flash area.

@digitalentity
Copy link
Member

This is unrelated to H7 code itself, but to the way we read config and was merely revealed by a H7 debugging leftover code.

https://github.com/iNavFlight/inav/blob/master/src/main/config/config_eeprom.c#L155-L158:

        if (record->size == 0
            || p + record->size >= &__config_end
            || record->size < sizeof(*record))
            break;

Here we check that p + record->size doesn't span beyond config memory area, but what happens if record->size is beyond the end of config memory area? Nothing harmful, unless the config memory area is the last page of the flash and there is nothing beyond that. In that case reading record->size will cause a hardfault, which is exactly what happened here.

In pre-H7 code we did a call to ensureEEPROMContainsValidData() to make sure EEPROM makes sense, but this was accidentally left out when debugging config read/write on H7. This caused readEEPROM() to be executed on a potentially invalid config memory contents which revealed this bug.

Interestingly, ensureEEPROMContainsValidData() may suffer from the same bug as it also doesn't check that record->size is within valid config memory area.

Note that F7, H7 and some beefy F4 chips don't suffer from this bug as there is valid memory beyond __config_end.

#6384 fixes this

@alteman
Copy link
Author

alteman commented Dec 6, 2020

Thanks, working now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants