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

Signals not delivered to correct thread #644

Closed
prp opened this issue Jul 18, 2020 · 5 comments
Closed

Signals not delivered to correct thread #644

prp opened this issue Jul 18, 2020 · 5 comments
Labels
area: sgx-lkl Core SGX-LKL functionality enhancement p2 Important but non-urgent priority
Milestone

Comments

@prp
Copy link
Member

prp commented Jul 18, 2020

The following test uses rt_sigqueueinfo to send a signal to a thread: vtikoo@7e539ee

When running the test directly, it succeeds:

$ ./rt_sigqueueinfo01
pid=1204
tid within main thread: 1204
errno: 0, Success
tid of child thread: 1205
tst_checkpoint_wake(0)
errno: 0, Success
ret: 0, errno: 0, Success
tid within signal handler: 1205
Received correct signal and data!
ret: 0, errno: 0, Success
rt_sigqueueinfo() was successful

When run with SGX-LKL, it fails as follows:

[[  SGX-LKL ]] libc_start_main_stage2(): Calling app main: rt_sigqueueinfo01
pid=36
tid within main thread: 1
errno: 0, No error information
tid of child thread: 47
tst_checkpoint_wake(0)
errno: 0, No error information
ret: 0, errno: 0, No error information
rt_sigqueueinfo() failed (ret=-1 errno=3 (No such process))

The problem seems to be that the assigned tid for the main thread is inconsistent with the pid.

(cc: @vtikoo @davidchisnall)

@prp prp added bug area: sgx-lkl Core SGX-LKL functionality p3 Non-blocking priority labels Jul 18, 2020
@prp prp added this to the Milestone 1 milestone Jul 18, 2020
@prp prp changed the title rt_sigqueueinfo fails to send signal to thread Signals not delivered to correct thread Jul 18, 2020
@prp
Copy link
Member Author

prp commented Jul 18, 2020

Ok, the above problem goes away when we remove the syscall bypass for gettid(). (The pid/tid assignment is then still not consistent, but this may be ok.)

The problem then is that the signal is always delivered to the thread calling rt_sigqueueinfo and not the thread whose tid is specified.

The reason is that the code in LKL's handle_signal() does not create a proper stack frame for the signal handler, but rather just calls the handler directly.

For the pending signal to be associated with the correct thread, I suspect that we need to follow the x86 signal handling code, which first creates a stack frame and then invokes the kernel signal delivery.

This may pull in more x86 architecture code into LKL.

@davidchisnall
Copy link
Contributor

I don't think that we need a proper stack frame to have the correct behaviour, we just need to wake up the correct thread and allow it to pull the pending signal off the stack on syscall return. Currently, we are handling all signals for any task on syscall return.

@prp
Copy link
Member Author

prp commented Jul 24, 2020

@davidchisnall I'm not sure. I think that the correct code path for signal handling is to create a stack frame and associate the pending signal with the correct thread (which can be done on the syscall return path -- for x86 it's done during a user/kernel space transition). The pending signal will then be picked up by the scheduler. I think that the problem with our current implementation is that it just calls the signal handler directly.

@SeanTAllen
Copy link
Contributor

The tgkill LTP should be enabled when this is fixed.

@KenGordon
Copy link
Collaborator

This is fixed as of #830 with the test re-enabled in 539b65a

/datadrive/kegordo/sept17/sgx-lkl/build/sgx-lkl-run-oe --hw-debug sgxlkl-miniroot-fs.img /ltp/testcases/kernel/syscalls/rt_sigqueueinfo/rt_sigqueueinfo01 tst_test.c:1117: INFO: Timeout per run is 0h 00m 20s tst_test.c:1136: INFO: No fork support rt_sigqueueinfo01.c:37: PASS: Received correct signal and data! rt_sigqueueinfo01.c:78: PASS: rt_sigqueueinfo() was successful! [ 0.117755] reboot: Restarting system

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: sgx-lkl Core SGX-LKL functionality enhancement p2 Important but non-urgent priority
Projects
None yet
Development

No branches or pull requests

4 participants