[lkml]   [2007]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: [BUG] ptraced process waiting on syscall may return kernel internal errnos

    On Wed, 6 Jun 2007, Roland McGrath wrote:
    > [PATCH] Restrict clearing TIF_SIGPENDING
    > This patch should get a few birds. It prevents sigaction calls from
    > clearing TIF_SIGPENDING in other threads, which could leak -ERESTART*.
    > It fixes ptrace_stop not to clear it, which done at the syscall exit
    > stop could leak -ERESTART*. It probably removes the harm from
    > signalfd, at least assuming it never calls dequeue_signal on kernel
    > threads that might have used block_all_signals.

    Ok, this one is more complex than my suggested one-liner, but seems to fix
    another bug. And it's logic in dequeue_signal() is a bit prettier than
    Ben's (otherwise somewhat similar) patch.

    HOWEVER: I'm still a bit worried about the fact that we have about ~180
    calls to "recalc_sigpending()" around the kernel, and I'm not AT ALL sure
    that they are all supposed to possibly clear the TIF_SIGPENDING flag. The
    fact that we had basically two independent bugs in this area really makes
    me more convinced that we probably should clear TIF_SIGPENDING in the only
    path that really matters, namely the one that _depends_ on it being right
    (the "do_signal()" path).

    So I think that the *right* place to clear TIF_SIGPENDING is actually in
    "get_signal_to_deliver()", because that function is called _only_ by the
    actual per-architecture "I'm going to deliver a signal now".

    (Oh, and we do need to handle it in the "notifier()" case too - ugly hack
    for DRM).

    So how does *this* patch look? It:

    - totally removes the "clear_tsk_thread_flag()" from the signal pending
    recalculations (ie now both "recalc_sigpending()" _and_
    "recalc_sigpending_tsk()" will ever only _set_ that bit)

    - in get_signal_to_deliver(), it adds a

    if (!recalc_sigpending_tsk(current))

    at the end.

    - it removes the "recalc_sigpending_tsk()" from dequeue_signal(), since
    that can never clear the signal any more.

    So now we should do "recalc_sigpending()" only when signals may be *added*
    (where messing with the "blocked" mask obviously is a form of adding
    signals, and possibly the most common reason for having to recalculate the
    sigpending mask).

    Comments? This patch is _entirely_ and utterly untested, so I'm only
    saying that this "feels" safer and more correct to me.


    kernel/signal.c | 4 ++--
    1 files changed, 2 insertions(+), 2 deletions(-)

    diff --git a/kernel/signal.c b/kernel/signal.c
    index acdfc05..82b3c1a 100644
    --- a/kernel/signal.c
    +++ b/kernel/signal.c
    @@ -105,7 +105,6 @@ static int recalc_sigpending_tsk(struct task_struct *t)
    set_tsk_thread_flag(t, TIF_SIGPENDING);
    return 1;
    - clear_tsk_thread_flag(t, TIF_SIGPENDING);
    return 0;

    @@ -385,7 +384,6 @@ int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
    - recalc_sigpending_tsk(tsk);
    if (signr && unlikely(sig_kernel_stop(signr))) {
    * Set a marker that we have dequeued a stop signal. Our
    @@ -1859,6 +1857,8 @@ relock:
    /* NOTREACHED */
    + if (!recalc_sigpending_tsk(current))
    + clear_thread_flag(TIF_SIGPENDING);
    return signr;
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at
     \ /
      Last update: 2007-06-06 17:39    [W:0.027 / U:12.116 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site