Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(402)

Issue 1739011: Added support for sigreturn() and rt_sigreturn(). On x86-32, this is... (Closed)

Created:
10 years, 8 months ago by Markus (顧孟勤)
Modified:
9 years, 7 months ago
Reviewers:
agl
CC:
chromium-reviews, Evan Martin, willchan no longer on Chromium
Visibility:
Public.

Description

Added support for sigreturn() and rt_sigreturn(). On x86-32, this is complicated by the fact that in Seccomp mode, we can only ever call sigreturn(). But in order to eventually support sigaction(), we want to be able to also call rt_sigreturn(). We solve this problem by rewriting the signal stack frame from an RT signal frame to a legacy frame. Fortunately, this part of the signal frame is stable between kernel versions. The unstable part (i.e. extended registers such as FP, MMX, SSE, ...) is always identical in both in both types of signal frames. None of these complications exist on x86-64 and it is relatively straight-forward to enable support for the system call. The only difficulty lies in the fact that its calling conventions are somewhat different from "normal" system calls. So, we have to handle rt_sigreturn() from within the syscallWrapper() and the segv() handler and cannot write it in C code. TEST=ad hoc testing until we have support for sigaction(). Then we can add a unittest BUG=37728 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=45774

Patch Set 1 #

Total comments: 6

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : '' #

Patch Set 8 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+239 lines, -127 lines) Patch
M sandbox/linux/seccomp/ioctl.cc View 1 1 chunk +5 lines, -2 lines 0 comments Download
M sandbox/linux/seccomp/library.cc View 1 1 chunk +12 lines, -14 lines 0 comments Download
M sandbox/linux/seccomp/sandbox.cc View 1 9 chunks +140 lines, -79 lines 0 comments Download
M sandbox/linux/seccomp/sandbox_impl.h View 1 1 chunk +1 line, -1 line 0 comments Download
M sandbox/linux/seccomp/syscall.cc View 1 15 chunks +81 lines, -31 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Markus (顧孟勤)
More crazy code. Sorry about that :-/ I know why I was so resistant to ...
10 years, 8 months ago (2010-04-27 02:13:45 UTC) #1
agl
10 years, 8 months ago (2010-04-27 15:27:34 UTC) #2
LGTM

http://codereview.chromium.org/1739011/diff/1/4
File sandbox/linux/seccomp/sandbox.cc (right):

http://codereview.chromium.org/1739011/diff/1/4#newcode248
sandbox/linux/seccomp/sandbox.cc:248: "mov  $1, %%edi\n"           //
rt_sigreturn() should never return
Maybe a more magic number here? One is a bit common :)

http://codereview.chromium.org/1739011/diff/1/4#newcode325
sandbox/linux/seccomp/sandbox.cc:325: "cmpw $0x010F, (%%ebp)\n"    // RDTSC
Shouldn't the comment be "RDTSCP"?

http://codereview.chromium.org/1739011/diff/1/4#newcode333
sandbox/linux/seccomp/sandbox.cc:333: "mov  $0x16, %%ecx\n"
I would tend to write this as $22 rather than use hex. The 16 and the 0x cause a
mental hiccup.

http://codereview.chromium.org/1739011/diff/1/4#newcode338
sandbox/linux/seccomp/sandbox.cc:338: "lea  6f, %%esi\n"
I must admit that I'm not sure what's going on here. "6f"? You're copying from
the label 6: (below) into the structure?

http://codereview.chromium.org/1739011/diff/1/6
File sandbox/linux/seccomp/syscall.cc (right):

http://codereview.chromium.org/1739011/diff/1/6#newcode54
sandbox/linux/seccomp/syscall.cc:54: "mov  $1, %edi\n"              //
rt_sigreturn() should never return
Again, maybe more magic in the number?

http://codereview.chromium.org/1739011/diff/1/6#newcode147
sandbox/linux/seccomp/syscall.cc:147: "mov  $1, %ebx\n"              //
sigreturn() should never return
ditto

Powered by Google App Engine
This is Rietveld 408576698