lkml.org 
[lkml]   [2019]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFC PATCH 1/5] signal: Teach sigsuspend to use set_user_sigmask
    Oleg Nesterov <oleg@redhat.com> writes:

    > On 06/07, Eric W. Biederman wrote:
    >>
    >> +static int set_sigmask(sigset_t *kmask)
    >> +{
    >> + set_restore_sigmask();
    >> + current->saved_sigmask = current->blocked;
    >> + set_current_blocked(kmask);
    >> +
    >> + return 0;
    >> +}
    >
    > I was going to do the same change except my version returns void ;)
    >
    > So ACK.
    >
    >
    > As for 2-5, sorry I can't read them today, will do tomorrow.
    >
    > But at first glance... yes, we can remove TIF_RESTORE_SIGMASK.
    >
    > As for "remove saved_sigmask" I have some concerns... At least this
    > means a user-visible change iiuc. Say, pselect unblocks a fatal signal.
    > Say, SIGINT without a handler. Suppose SIGINT comes after set_sigmask().
    >
    > Before this change the process will be killed.
    >
    > After this change it will be killed or not. It won't be killed if
    > do_select() finds an already ready fd without blocking, or it finds a
    > ready fd right after SIGINT interrupts poll_schedule_timeout().

    Yes. Because having the signal set in real_blocked disables the
    immediate kill optimization, and the signal has to be delivered before
    we decide to kill the process. Which matters because as you say if
    nothing checks signal_pending() when the signals are unblocked we might
    not attempt to deliver the signal.

    So it is a matter of timing.

    If we have both a signal and a file descriptor become ready
    at the same time I would call that a race. Either could
    wake up the process and depending on the exact time we could
    return either one.

    So it is possible that today if the signal came just after the file
    descriptor ,the code might have made it to restore_saved_sigmask_unless,
    before __send_signal runs.

    I see the concern. I think in a matter like this we try it. Make
    the patches clean so people can bisect the problem. Then if someone
    runs into this problem we revert the offending patches.

    If it looks like bisection won't cleanly reveal the potential problem
    please let me know.

    Personally I don't think anyone sane would intentionally depend on this
    and I don't think there is a sufficiently reliable way to depend on this
    by accident that people would actually be depending on it.

    > And _to me_ the new behaviour makes more sense. But when it comes to
    > user-visible changes you can never know if it breaks something or not.

    True.

    The set of applications is larger than any developer can reasonably test.

    Eric

    \
     
     \ /
      Last update: 2019-06-10 23:22    [W:5.647 / U:0.988 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site