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

[Pal/Linux-SGX] Detect signal stack overflow in "enclave_entry.S" #108

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

boryspoplawski
Copy link
Contributor

@boryspoplawski boryspoplawski commented Sep 30, 2021

Description of the changes

Creating a frame with CPU context on signal stack when handling an exception was done without any bound checks. Malicious host could generate any amount of signals and cause these frames to overflow the stack. This commit fixes it by adding bound checks, effectively disallowing more than ~9 nested exceptions (which should be more than enough in normal case).

How to test this PR?

Try running in Gramine with SGX:

#define _GNU_SOURCE
#include <err.h>
#include <sched.h>

int main(void) {
     while (1) {
         if (sched_yield() < 0) {
             err(1, "sched_yield");
         }
     }
}

and concurrently timeout 4 bash -c 'while true; do kill -SIGCONT pid_of_above_program; done'. This will cause weird behavior on the current master (most likely a crash) and with this PR the process will hang in newly added FAIL_LOOP


This change is Reviewable

Creating a frame with CPU context on signal stack when handling an
exception was done without any bound checks. Malicious host could
generate any amount of signals and cause these frames to overflow the
stack. This commit fixes it by adding bound checks, effectively
disallowing more than ~9 nested exceptions (which should be more than
enough in normal case).

Signed-off-by: Borys Popławski <[email protected]>
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL)

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@dimakuv dimakuv merged commit baee61e into master Oct 1, 2021
@dimakuv dimakuv deleted the borys/lasciate_ogne_speranza_voi_chintrate branch October 1, 2021 08:59
@mkow mkow mentioned this pull request Oct 8, 2021
11 tasks
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