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

Stack guard #104

Closed
atiselsts opened this issue Oct 20, 2017 · 8 comments
Closed

Stack guard #104

atiselsts opened this issue Oct 20, 2017 · 8 comments
Assignees

Comments

@atiselsts
Copy link
Member

atiselsts commented Oct 20, 2017

See the discussion here: contiki-os/contiki#2089

I can adapt it for Contiki-NG (some work required) and resubmit here.

The one thing remaining to discuss here: do you have objections against it being enabled by default?
As I stated in the "old" Contiki PR discussion, I believe it would be much more useful if enabled. "Release" builds can always explicitly remove it, by simply adding a compilation flag.

@simonduq
Copy link
Member

Yeah that sounds very useful. Is it ported to all platforms?
Why enabling by default? Is it just to make it more visible? Then I simply add a documentation and tutorial page on the wiki https://github.com/sics-iot/contiki/wiki

BTW, the project name is spelled with a "-": Contiki-NG

@atiselsts
Copy link
Member Author

atiselsts commented Oct 20, 2017

Is it ported to all platforms?

No, but I could do all platforms that Contiki-NG supports - except Cooja mote and native, I suppose.

Why enabling by default? Is it just to make it more visible?

Ideally, I would like to have an additional line in George's unified main() that periodically does the stack integrity check and sends the system in a panic mode if that fails. (Similar to what AVR platforms alreayd have in Contiki.) Basically this is to save people's time ("my program was crashing for some reason and I had no idea why for a week, after which I finally decided to enable this feature and saw the problem" vs. "stack overflow has been detected, which followed by a crash")

If there is no check in the main, then I'm probably ok with having it disabled by default. There still are some small benefits, like this (quoting myself):

Just the fact itself that the stack is filled with 0xcd sometimes does make a difference - it can be helpful when looking at memory dumps in mspsim or by using gdb a big region of 0xcd will clearly indicate untouched stack

(This second part is more speculative, but...) it could also be seen as a security feature, and evidence that Contiki is a robustly designed OS. I mean - it could make the system less vulnerable to attacks which aim to corrupt the global memory in some targeted way. As example, think of a function that has a large stack buffer, which is filled with data coming from an untrusted source - e.g. a packet coming from the network. If the buffer overlaps with some static memory at the runtime, that static memory could be changed in a specific way by just sending the right packet to the node.

@simonduq
Copy link
Member

ok, we might consider enabling by default, happy to hear what others have to say.
What is the overhead, roughly?

@g-oikonomou
Copy link
Member

I do not feel strongly about either option, I can see benefits in both

@atiselsts
Copy link
Member Author

What is the overhead, roughly?

For checking the memory, one byte at a time, it takes around 0.2 milliseconds to check 1000 bytes of RAM on CC2650. Do you think that doing this once per second would be acceptable? Once 10 seconds? Minute?

@simonduq
Copy link
Member

Maybe once every 10 seconds. I don't know

@joakimeriksson
Copy link
Member

every 10 should work - and I guess default on but documentation on how to turn of should be ok!

@g-oikonomou
Copy link
Member

Closed in #192

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

No branches or pull requests

4 participants