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

loader fix for global symbols lookup #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gianlu33
Copy link
Member

There is a weird bug that occurs when an ELF binary is loaded dynamically. Basically, when we load an ELF, some global symbols are added in the global symbol table, and everything seems correct here. However, if we want to retrieve the symbol value afterwards, the lookup fails because the symbol name in the table contains weird spacing characters at the end, making symbol_matches return false.

It is difficult to reproduce this bug because we need to dynamically load an ELF, but I took a screenshot that shows the anomaly:
Capture

In this debug logs, I printed the name of the symbol and its length with the format <name>:<length>.

The second line (Adding symbol...) is printed when the ELF is loaded dynamically: here the name of the symbol is correct. The following lines are printed when we want to retrieve the value of this symbol: the name printed out is the one stored in the symbol table. You can see that now this symbol for some strange reason has an additional character at the end (a newline).

I inspected the code quite carefully but I couldn't find the reason for this bug. However, I came up with a (temporary) fix to circumvent the bug, by ignoring spacing characters at the end of the symbol name in the comparison performed in symbol_matches

@fritzalder
Copy link
Member

Thanks for the detached PR! I also briefly looked into this after our chat and I also could not find any obvious reason for this bug. Maybe Jo or Job know more

@jovanbulck
Copy link
Member

Thanks Gianluca for reporting this issue! Im thinking let's try to understand what's going on here and fix the core issue instead of applying this workaround if possible; that way we won't unintended break other things as well 🤔

Ideally would be to first add a small sancus-examples program that directly uses sm_id sm_load(void* file, const char* name, vendor_id vid) in the simulator to see if the issue pops up there as well? It should be possible to get a character buffer for the file parameter using the simulator support for fileio as follows:

https://github.com/sancus-tee/sancus-examples/tree/master/fileio

@gianlu33 do you think you can try to construct such a sancus-examples program to see if the issues arises there as well?

once we have a small example, maybe @mtvec could also try to help to understand what's going on

@gianlu33
Copy link
Member Author

Hi @jovanbulck, I made a simple example as you suggested and I found out that the problem seems related to Riot. Indeed, by running the example without Riot everything looks correct; instead, using Riot I experience the bug.

I will check the Riot code to see if I can find the root cause of the problem and if it can be fixed.

@jovanbulck
Copy link
Member

Great, many thanks @gianlu33 ! I suggest the following:

Let's leave this PR open for now. We can close this later, once we figure out the root cause of the problem, and if it it would indeed not be related to sancus-support and doesn't need a fix here.

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 this pull request may close these issues.

3 participants