lkml.org 
[lkml]   [2007]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
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))
d_flag(TIF_SIGPENDING);
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.

Linus

---
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:
do_group_exit(signr);
/* NOTREACHED */
}
+ if (!recalc_sigpending_tsk(current))
+ clear_thread_flag(TIF_SIGPENDING);
spin_unlock_irq(&current->sighand->siglock);
return signr;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2007-06-06 17:39    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site