-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Secure stack check #9024
Secure stack check #9024
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9024 +/- ##
=======================================
Coverage 52.15% 52.15%
=======================================
Files 212 212
Lines 25916 25916
Branches 5582 5582
=======================================
Hits 13517 13517
Misses 10149 10149
Partials 2250 2250 Continue to review full report at Codecov.
|
Looks ok, few discussion topics:
|
Hi Ruud
| I don't see corresponding stack check enables in most cases where checking is disabled (exceptions, interrupts), assume that is handled implicitly by restore of the original STAT/SEC_STAT on interrupt return?
Yes, it is handled implicitly by restore of the original STAT/SEC_STAT on interrupt return.
| is it necessary to always disable stack checking for interrupt processing? Would it be possible to keep stack checking enabled and only disable on reprogramming the stack boundary registers on a context switch?
In interrupt processing, the stack will be switched to isr stack, if stack check is not disabled, the stack check fail exception will be raised, as the new sp will not be in the range from STACK_TOP to STACK_BASE.
Of course, we can reprogram the stack check reg to match the change of sp, but it means we need to save the old one and restore it later. More instructions are required, it will affect the interrupt latency.
One more, if the interrupt stack is overflow, there will be no way to recover from this cause, the system has to stop.
In zephyr, it seems the stack protection is mainly for thread stack. For isr stack, it needs more discussion.
From: ruuddw [mailto:notifications@github.com]
Sent: 2018年8月2日 21:27
To: zephyrproject-rtos/zephyr <zephyr@noreply.github.com>
Cc: Wayne Ren <Wei.Ren@synopsys.com>; Author <author@noreply.github.com>
Subject: Re: [zephyrproject-rtos/zephyr] Secure stack check (#9024)
Looks ok, few discussion topics:
* I don't see corresponding stack check enables in most cases where checking is disabled (exceptions, interrupts), assume that is handled implicitly by restore of the original STAT/SEC_STAT on interrupt return?
* is it necessary to always disable stack checking for interrupt processing? Would it be possible to keep stack checking enabled and only disable on reprogramming the stack boundary registers on a context switch?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_zephyrproject-2Drtos_zephyr_pull_9024-23issuecomment-2D409925389&d=DwMFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=uIRv5OnfaYG5_34mbrNlSdByDrbpAfi15TvQ5YKDOG8&m=ZMUHGvxja3uOHWHlHtAew-HgvPq0sv6ZHDiv3sFlKng&s=Trnf-ptyQczDg9yuNWttZw-OzgHz58m7uaCMzbJi8Kw&e=>, or mute the thread<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AFB9O4P3TZ-5FzOe9QhL3eP31gA3-2DxhlDYks5uMv4VgaJpZM4VWBoF&d=DwMFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=uIRv5OnfaYG5_34mbrNlSdByDrbpAfi15TvQ5YKDOG8&m=ZMUHGvxja3uOHWHlHtAew-HgvPq0sv6ZHDiv3sFlKng&s=hM7BV1_ZSvWie-wkVqvHgpKmhrSe7DVQdUc9UBSiSyA&e=>.
|
64cf09f
to
6bceee1
Compare
b8c0bac
to
201213c
Compare
9c1e8f4
to
10bce89
Compare
stack check bit of status32/sec_stat will be cleared automically in exception entry. so remove the redundent codes Signed-off-by: Wayne Ren <wei.ren@synopsys.com>
when arc is in secure mode, SSC bit of sec_stat, not SC bit of status32,is used to enable stack check. Signed-off-by: Wayne Ren <wei.ren@synopsys.com>
The fake exception return is used to jump to user mode. So the init status of user thread is in exception mode. Signed-off-by: Wayne Ren <wei.ren@synopsys.com>
re-orginize the code in _new_thread to make it easier to understand and maintain Signed-off-by: Wayne Ren <wei.ren@synopsys.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok, and sanitycheck is (almost) clean now for nsim_em and nsim_sem. Few remaining failures, but these are understood and expected.
@nashif I believe this PR is ready for merging now.
as the thread is created in privileged mode, the init context should also be in privileged stack. Signed-off-by: Wayne Ren <wei.ren@synopsys.com>
to avoid _get_num_regions to access the aux reg each time in the for loop Signed-off-by: Wayne Ren <wei.ren@synopsys.com>
10bce89
to
201b260
Compare
this PR is used to enable the stack check when arc is in secure mode.
If arc is in secure mode, SSC bit of sec_stat, not SC bit of status32, is used to enable/disable stack check.
Fixes #8313