Fix stack protector#1650
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1650 +/- ##
=======================================
Coverage 92.00% 92.00%
=======================================
Files 39 39
Lines 4653 4653
Branches 597 597
=======================================
Hits 4281 4281
Misses 258 258
Partials 114 114
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
fbeutin-ledger
left a comment
There was a problem hiding this comment.
Also needs more text in the __stack_chk_guard generation code to explain how it behaves with os_lib_call
Maybe also reroll if == 0 🤔
|
|
7a5dce8 to
80a7870
Compare
There was a problem hiding this comment.
Tested with app-bitcoin built as:
make -C . -j DEBUG=1 ENABLE_STACK_PROTECTOR=1 TARGET=flex BOLOS_SDK=/tmp/my-sdk/
against the current SDK.
it works with Speculos and with the real physical device:
echo "EF01000000" | python3 -m ledgerblue.runScript --apdu
HID => ef01000000
HID <= 4ccad0829000
<= Clear bytearray(b'L\xca\xd0\x82')
Need still to test it with swap.
|
Tested successfully with |
e150b32 to
b231399
Compare
b231399 to
fc00006
Compare
Description
There were two problems in the initial stack protector implementation :
ENTRY(main);The second problem could not be detected during tests as canary was not initialized.
The idea of this fix is to have the correct entry point whether building with SSP.
Also, moving canary out of bss with appropriate pic adaptation.
Changes include
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it.
Additional comments
Please post additional comments in this section if you have them, otherwise delete it.
Auto cherry-pick in API_LEVEL
If requested to port the commits from this PR on a dedicated API_LEVEL branch,
select the targeted one(s), or add new references if not listed:
[x] TARGET_API_LEVEL: API_LEVEL_26
This will only create the PR with cherry-picks, ready to be reviewed and merged.
Remember: