lkml.org 
[lkml]   [2009]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: Q: SEGSEGV && uc_mcontext->ip (Was: Signal delivery order)
Date
On Martes 17 Marzo 2009, Oleg Nesterov wrote:
> (see http://marc.info/?t=123704955800002)
>
> First of all, perhaps I missed somethings and this is solvable
> without kernel changes, but I can't see how.
>
> To simplify, suppose that the application wants to log the faulting
> instruction before exit, so it installs the handler for SIGSEGV
>
> void sigsegv_handler(int sig, siginfo_t *info, struct ucontext
> *context) {
> fprintf(stderr, "bug at %p\n", context->uc_mcontext->ip);
> exit(1);
> }
>
> But this doesn't work. It is possible that, before the task dequeues
> SIGSEGV, another private signal, say, SIGHUP (note that SIGHUP <
> SIGSEGV) is sent to this app.
>
> In this case the task dequeues both signals, SIGHUP and then SIGSEGV
> before return to user-space. This is correct, but now uc_mcontext->ip
> points to sighup_handler, not to the faulted instruction.
>
>
> Can/should we change force_sig_info_fault() to help?
>
> We can add siginfo_t->_sigfault.ip to solve this problem. This
> shouldn't enlarge the size of siginfo_t. With this change
> sigsegv_handler() always knows the address of the instruction which
> caused the fault.
>
>
> But this doesn't allow to change uc_mcontext->ip to, say, skip the
> offending instruction or call another function which does a fixup.
> Actually, ->si_ip helps, we can do
>
> void sigsegv_handler(int sig, siginfo_t *info, struct ucontext
> *context) {
> if (info->si_ip != context->uc_mcontext->ip)
> /*
> * The offending instruction will be repeated, and
> * we will have the "same" SIGSEGV again.
> */
> return;
>
> context->uc_mcontext->ip = fixup;
> ...
> }
>
> But this doesn't look very nice. So, perhaps we can do another
> change?
>
> --- arch/x86/mm/fault.c
> +++ arch/x86/mm/fault.c
> @@ -177,6 +177,13 @@ static void force_sig_info_fault(int si_
> {
> siginfo_t info;
>
> + current->saved_sigmask = current->blocked;
> + spin_lock_irq(&current->sighand->siglock);
> + siginitsetinv(&current->blocked, sigmask(si_signo) |
> + sigmask(SIGKILL) | sigmask(SIGSTOP));
> + spin_unlock_irq(&current->sighand->siglock);
> + set_restore_sigmask();
> +
> info.si_signo = si_signo;
> info.si_errno = 0;
> info.si_code = si_code;
>
> But this is a user-visible change, all signals will be blocked until
> sigsegv_handler() returns. But with this change sigsegv_handler()
> always has the "correct" rt_sigframe.
>
>
> Comments?
>
> And once again, I have a nasty feeling I missed something and we
> don't need to change the kernel.
>
> Oleg.

As an application developer what I'd like to have is this: synchronously
generated signals are delivered before asynchronously generated ones.
That is, if a number of signals are generated but not yet delivered
then the synchronously generated ones are delivered first. I guess, in
the kernel this would mean that the private/non-private distinction is
not enough.

I think Chris is right in that standard allows the current behaviour. I
would have quoted it already if I had thought otherwise ... He's also
right about quality of implementation. The standard doesn't say much
about synchronously and asynchronously generated signals and it even
goes further:

"The order in which multiple, simultaneously pending signals outside the
range SIGRTMIN to SIGRTMAX are delivered to or accepted by a process is
unspecified."

Bleh. It also says that the context argument represents the context at
the time of delivery. For a handler for sigsegv, sigtrap (to name just
the most likely suspects) the context - in order for it to be useful
at all - must be from the time of generation. There is an obvious
contradiction here and the only way I see to resolve this is to deliver
syncrhronously generated signals while the two contexts are the same
which is exactly what allowing other signals to 'overtake' violates. Of
course, if sigsegv is blocked then this is impossible to do, but that's
fine.

For the application I'm working on this whole issue is kind of moot
since kernels with the current behaviour will be around for ages to
come and I have the workaround of not pthread_kill()ing with a signal
whose signum is lower than the signum of any of the syncrhonously
generated signals. In practice, it only means that I'll use SIGUSR2
instead of SIGUSR1 because that's greater than SIGSEGV and pay
attention not to pthread_kill() with SIGINT. The only thing that
worries me is this remark from Oleg
(http://marc.info/?l=linux-kernel&m=123711058421913&w=2):

"But please note that it is still possible to hit is_signal_blocked()
even with test_with_kill(), but the probability is very low."

I still can't see how that's possible, but if it is, then it defeats the
workaround and I have even bigger problems than I thought. Not only me,
I guess, most applications with a sigtrap, sigsegv handler that use the
context will be broken by a C-c.

Gabor


\
 
 \ /
  Last update: 2009-03-17 09:27    [W:0.493 / U:0.296 seconds]
©2003-2014 Jasper Spaans. Advertise on this site