Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Thu, Jun 07, 2007 at 01:29:23PM +1000, Benjamin Herrenschmidt wrote: > But you use ptrace and don't steal signals with dequeue_signal() on a > live other task, which is ok. True. However, with an SMP UML running a threaded app (which is also threads on the host), if the thread on one CPU gets a signal which will be nullified (SIGTRAP, SIG*ALRM, later SIGSEGV) and the other thread continues running, does that count as stealing a signal from another live task? Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Thu, Jun 07, 2007 at 01:29:23PM +1000, Benjamin Herrenschmidt wrote: But you use ptrace and don't steal signals with dequeue_signal() on a live other task, which is ok. True. However, with an SMP UML running a threaded app (which is also threads on the host), if the thread on one CPU gets a signal which will be nullified (SIGTRAP, SIG*ALRM, later SIGSEGV) and the other thread continues running, does that count as stealing a signal from another live task? Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, 2007-06-06 at 22:20 -0400, Jeff Dike wrote: > On Thu, Jun 07, 2007 at 08:43:42AM +1000, Paul Mackerras wrote: > > What Ben was talking about was stealing a synchronous SEGV from a task > > without stopping it, and as Ben says that makes no sense. > > Intercepting a signal and stopping the task is reasonable, and that is > > what ptrace does, and I assume also UML. > > It is, but I can also see UML stealing the SEGV from the child. The > UML skas does this - a ptrace extension, PTRACE_FAULTINFO, is used to > extract page fault information from the child, and other pieces of the > patch are used to fix the fault without the child continuing until > it's fixed. So, in this case, the child never sees the SEGV. But you use ptrace and don't steal signals with dequeue_signal() on a live other task, which is ok. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, 2007-06-06 at 08:52 -0400, Jeff Dike wrote: > On Wed, Jun 06, 2007 at 12:50:04PM +1000, Benjamin Herrenschmidt wrote: > > Yeah, synchronous signals should probably never be delivered to another > > process, even via signalfd. There's no point delivering a SEGV to > > somebody else :-) > > Sure there is. UML does exactly that - intercepting child signals > (including SEGV) with wait. UML is definitely what I call a special case :-) Now the question is how do you get them ? Are you via some code path I haven't figured out calling dequeue_signal() from another context ? Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Thu, Jun 07, 2007 at 08:43:42AM +1000, Paul Mackerras wrote: > What Ben was talking about was stealing a synchronous SEGV from a task > without stopping it, and as Ben says that makes no sense. > Intercepting a signal and stopping the task is reasonable, and that is > what ptrace does, and I assume also UML. It is, but I can also see UML stealing the SEGV from the child. The UML skas does this - a ptrace extension, PTRACE_FAULTINFO, is used to extract page fault information from the child, and other pieces of the patch are used to fix the fault without the child continuing until it's fixed. So, in this case, the child never sees the SEGV. Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
Jeff Dike writes: > On Wed, Jun 06, 2007 at 12:50:04PM +1000, Benjamin Herrenschmidt wrote: > > Yeah, synchronous signals should probably never be delivered to another > > process, even via signalfd. There's no point delivering a SEGV to > > somebody else :-) > > Sure there is. UML does exactly that - intercepting child signals > (including SEGV) with wait. What Ben was talking about was stealing a synchronous SEGV from a task without stopping it, and as Ben says that makes no sense. Intercepting a signal and stopping the task is reasonable, and that is what ptrace does, and I assume also UML. Paul. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: > On Tue, 2007-06-05 at 20:37 -0700, Linus Torvalds wrote: > > > I agree that it would be a limitation, but it would be a sane one. > > > > How about we try to live with that limitation, if only to avoid the issue > > of having the private signals being stolen by anybody else. If we actually > > find a real-live use-case where that is bad in the future, we can re-visit > > the issue - it's always easier to _expand_ semantics later than it is to > > restrict them, so I think this thread is a good argument for starting it > > out in a more restricted form before people start depending on semantics > > that can be nasty.. > > Here's a patch. Let me know if I missed something. > > Fix races with signalfd and TIF_SIGPENDING > > We must never clear TIF_SIGPENDING for another task. This patch > ensures that by preventing recalc_sigpending_tsk() from clearing > that bit if the target task is not current. > > In addition we also prevent __dequeue_signal() from calling the > DRM notifier thingy when stealing signals from another task via > signalfd. > > Finally, we only dequeue shared signals when called from another > task (via signalfd), we leave private signals alone. Makes sense... > Signed-off-by: Benjamin Herrenschmidt <[EMAIL PROTECTED]> Acked-by: Davide Libenzi <[EMAIL PROTECTED]> - Davide - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, Jun 06, 2007 at 12:50:04PM +1000, Benjamin Herrenschmidt wrote: > Yeah, synchronous signals should probably never be delivered to another > process, even via signalfd. There's no point delivering a SEGV to > somebody else :-) Sure there is. UML does exactly that - intercepting child signals (including SEGV) with wait. Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 2007-06-05 at 20:37 -0700, Linus Torvalds wrote:
> I agree that it would be a limitation, but it would be a sane one.
>
> How about we try to live with that limitation, if only to avoid the issue
> of having the private signals being stolen by anybody else. If we actually
> find a real-live use-case where that is bad in the future, we can re-visit
> the issue - it's always easier to _expand_ semantics later than it is to
> restrict them, so I think this thread is a good argument for starting it
> out in a more restricted form before people start depending on semantics
> that can be nasty..
Here's a patch. Let me know if I missed something.
Fix races with signalfd and TIF_SIGPENDING
We must never clear TIF_SIGPENDING for another task. This patch
ensures that by preventing recalc_sigpending_tsk() from clearing
that bit if the target task is not current.
In addition we also prevent __dequeue_signal() from calling the
DRM notifier thingy when stealing signals from another task via
signalfd.
Finally, we only dequeue shared signals when called from another
task (via signalfd), we leave private signals alone.
Signed-off-by: Benjamin Herrenschmidt <[EMAIL PROTECTED]>
---
Index: linux-work/kernel/signal.c
===
--- linux-work.orig/kernel/signal.c 2007-06-06 16:38:05.0 +1000
+++ linux-work/kernel/signal.c 2007-06-06 16:42:43.0 +1000
@@ -105,7 +105,12 @@ static int recalc_sigpending_tsk(struct
set_tsk_thread_flag(t, TIF_SIGPENDING);
return 1;
}
- clear_tsk_thread_flag(t, TIF_SIGPENDING);
+ /* Only clear the flag when this is issued by the target task to
+* clearing TIF_SIGPENDING after the target task decided to return
+* -ERESTARTSYS from a syscall
+*/
+ if (t == current)
+ clear_tsk_thread_flag(t, TIF_SIGPENDING);
return 0;
}
@@ -328,12 +333,12 @@ static int collect_signal(int sig, struc
}
static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
- siginfo_t *info)
+ siginfo_t *info, int stealing)
{
int sig = next_signal(pending, mask);
if (sig) {
- if (current->notifier) {
+ if (current->notifier && !stealing) {
if (sigismember(current->notifier_mask, sig)) {
if
(!(current->notifier)(current->notifier_data)) {
clear_thread_flag(TIF_SIGPENDING);
@@ -357,10 +362,19 @@ static int __dequeue_signal(struct sigpe
*/
int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
{
- int signr = __dequeue_signal(>pending, mask, info);
+ int stealing = tsk != current;
+ int signr = 0;
+
+ /* Only dequeue private signals if we are the owner, not when signals
+* are being stolen by another task via signalfd
+*/
+ if (!stealing)
+ signr = __dequeue_signal(>pending, mask, info, 0);
+
+ /* No private signal, look for shared ones */
if (!signr) {
signr = __dequeue_signal(>signal->shared_pending,
-mask, info);
+mask, info, stealing);
/*
* itimer signal ?
*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 2007-06-05 at 20:37 -0700, Linus Torvalds wrote:
I agree that it would be a limitation, but it would be a sane one.
How about we try to live with that limitation, if only to avoid the issue
of having the private signals being stolen by anybody else. If we actually
find a real-live use-case where that is bad in the future, we can re-visit
the issue - it's always easier to _expand_ semantics later than it is to
restrict them, so I think this thread is a good argument for starting it
out in a more restricted form before people start depending on semantics
that can be nasty..
Here's a patch. Let me know if I missed something.
Fix races with signalfd and TIF_SIGPENDING
We must never clear TIF_SIGPENDING for another task. This patch
ensures that by preventing recalc_sigpending_tsk() from clearing
that bit if the target task is not current.
In addition we also prevent __dequeue_signal() from calling the
DRM notifier thingy when stealing signals from another task via
signalfd.
Finally, we only dequeue shared signals when called from another
task (via signalfd), we leave private signals alone.
Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED]
---
Index: linux-work/kernel/signal.c
===
--- linux-work.orig/kernel/signal.c 2007-06-06 16:38:05.0 +1000
+++ linux-work/kernel/signal.c 2007-06-06 16:42:43.0 +1000
@@ -105,7 +105,12 @@ static int recalc_sigpending_tsk(struct
set_tsk_thread_flag(t, TIF_SIGPENDING);
return 1;
}
- clear_tsk_thread_flag(t, TIF_SIGPENDING);
+ /* Only clear the flag when this is issued by the target task to
+* clearing TIF_SIGPENDING after the target task decided to return
+* -ERESTARTSYS from a syscall
+*/
+ if (t == current)
+ clear_tsk_thread_flag(t, TIF_SIGPENDING);
return 0;
}
@@ -328,12 +333,12 @@ static int collect_signal(int sig, struc
}
static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
- siginfo_t *info)
+ siginfo_t *info, int stealing)
{
int sig = next_signal(pending, mask);
if (sig) {
- if (current-notifier) {
+ if (current-notifier !stealing) {
if (sigismember(current-notifier_mask, sig)) {
if
(!(current-notifier)(current-notifier_data)) {
clear_thread_flag(TIF_SIGPENDING);
@@ -357,10 +362,19 @@ static int __dequeue_signal(struct sigpe
*/
int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
{
- int signr = __dequeue_signal(tsk-pending, mask, info);
+ int stealing = tsk != current;
+ int signr = 0;
+
+ /* Only dequeue private signals if we are the owner, not when signals
+* are being stolen by another task via signalfd
+*/
+ if (!stealing)
+ signr = __dequeue_signal(tsk-pending, mask, info, 0);
+
+ /* No private signal, look for shared ones */
if (!signr) {
signr = __dequeue_signal(tsk-signal-shared_pending,
-mask, info);
+mask, info, stealing);
/*
* itimer signal ?
*
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, Jun 06, 2007 at 12:50:04PM +1000, Benjamin Herrenschmidt wrote: Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) Sure there is. UML does exactly that - intercepting child signals (including SEGV) with wait. Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: On Tue, 2007-06-05 at 20:37 -0700, Linus Torvalds wrote: I agree that it would be a limitation, but it would be a sane one. How about we try to live with that limitation, if only to avoid the issue of having the private signals being stolen by anybody else. If we actually find a real-live use-case where that is bad in the future, we can re-visit the issue - it's always easier to _expand_ semantics later than it is to restrict them, so I think this thread is a good argument for starting it out in a more restricted form before people start depending on semantics that can be nasty.. Here's a patch. Let me know if I missed something. Fix races with signalfd and TIF_SIGPENDING We must never clear TIF_SIGPENDING for another task. This patch ensures that by preventing recalc_sigpending_tsk() from clearing that bit if the target task is not current. In addition we also prevent __dequeue_signal() from calling the DRM notifier thingy when stealing signals from another task via signalfd. Finally, we only dequeue shared signals when called from another task (via signalfd), we leave private signals alone. Makes sense... Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED] Acked-by: Davide Libenzi [EMAIL PROTECTED] - Davide - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
Jeff Dike writes: On Wed, Jun 06, 2007 at 12:50:04PM +1000, Benjamin Herrenschmidt wrote: Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) Sure there is. UML does exactly that - intercepting child signals (including SEGV) with wait. What Ben was talking about was stealing a synchronous SEGV from a task without stopping it, and as Ben says that makes no sense. Intercepting a signal and stopping the task is reasonable, and that is what ptrace does, and I assume also UML. Paul. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Thu, Jun 07, 2007 at 08:43:42AM +1000, Paul Mackerras wrote: What Ben was talking about was stealing a synchronous SEGV from a task without stopping it, and as Ben says that makes no sense. Intercepting a signal and stopping the task is reasonable, and that is what ptrace does, and I assume also UML. It is, but I can also see UML stealing the SEGV from the child. The UML skas does this - a ptrace extension, PTRACE_FAULTINFO, is used to extract page fault information from the child, and other pieces of the patch are used to fix the fault without the child continuing until it's fixed. So, in this case, the child never sees the SEGV. Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, 2007-06-06 at 08:52 -0400, Jeff Dike wrote: On Wed, Jun 06, 2007 at 12:50:04PM +1000, Benjamin Herrenschmidt wrote: Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) Sure there is. UML does exactly that - intercepting child signals (including SEGV) with wait. UML is definitely what I call a special case :-) Now the question is how do you get them ? Are you via some code path I haven't figured out calling dequeue_signal() from another context ? Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, 2007-06-06 at 22:20 -0400, Jeff Dike wrote: On Thu, Jun 07, 2007 at 08:43:42AM +1000, Paul Mackerras wrote: What Ben was talking about was stealing a synchronous SEGV from a task without stopping it, and as Ben says that makes no sense. Intercepting a signal and stopping the task is reasonable, and that is what ptrace does, and I assume also UML. It is, but I can also see UML stealing the SEGV from the child. The UML skas does this - a ptrace extension, PTRACE_FAULTINFO, is used to extract page fault information from the child, and other pieces of the patch are used to fix the fault without the child continuing until it's fixed. So, in this case, the child never sees the SEGV. But you use ptrace and don't steal signals with dequeue_signal() on a live other task, which is ok. Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 5 Jun 2007, Linus Torvalds wrote: > On Tue, 5 Jun 2007, Davide Libenzi wrote: > > On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: > > > > > > Yeah, synchronous signals should probably never be delivered to another > > > process, even via signalfd. There's no point delivering a SEGV to > > > somebody else :-) > > > > That'd be a limitation. Like you can choose to not handle SEGV, you can > > choose to have a signalfd listening to it. Of course, not with the > > intention to *handle* the signal, but with a notification intent. > > I agree that it would be a limitation, but it would be a sane one. > > How about we try to live with that limitation, if only to avoid the issue > of having the private signals being stolen by anybody else. If we actually > find a real-live use-case where that is bad in the future, we can re-visit > the issue - it's always easier to _expand_ semantics later than it is to > restrict them, so I think this thread is a good argument for starting it > out in a more restricted form before people start depending on semantics > that can be nasty.. Yeah, that's easy. We can exclude them at signalfd creation time. - Davide - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
> a) Process-global signals can be read by any thread (inside or outside
> of the process receiving the signal).
>
> Rationale:
> This should always work, so there's no reason to limit it.
I agree, with an appropriate fix to recalc_sigpending_tsk() to only
clear TIF_SIGPENDING if tsk == current (the patch Linus posted
basically) _along_ with a fix to avoid the notifier thingy if stealing
from another task, that would work.
> b) Thread-specific signals can only be read by their target thread.
>
> Rationale:
> This behavior is required by POSIX, and if an application is using
> pthread_kill()/tkill()/tgkill()/etc. to specifically direct a signal, it
> damn well better get to where the app wants it to go.
I agree there too. I don't see the point of the 'feature' of allowing
those to be stolen and can only lead into all sort of new headaches
nobody needs.
> c) Synchronous signals ("Naturally" generated SIGILL, SIGFPE, SIGSEGV,
> SIGBUS, and SIGTRAP. Did I miss any?) are not delivered via signalfd()
> at all. (And by "naturally" generated, I mean signals that would have
> the SI_KERNEL flag set.)
Heh, well, as you say later, it can't be delivered anyway... I don't
think we need to do anything explicit to prevent them from being read()
in signalfd, it will just not happen.
> Rationale:
> These are a subset of thread-specific signals, so they can only be read
> from a signalfd by their target thread.
>
> However, there's no way for the target thread to get the signal because
> it is either:
>
> a) not blocked in a syscall waiting for signal delivery and thus further
> execution beyond the instruction causing the signal is impossible
> OR
> b) it is blocked in a syscall waiting for signal delivery and the error
> is caused by the signal delivery mechanism itself (i.e. a bad pointer
> passed to read/select/poll/epoll_wait/etc.) and thus the signal can't be
> delivered
Ben.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 2007-06-05 at 20:37 -0700, Linus Torvalds wrote:
>
> On Tue, 5 Jun 2007, Davide Libenzi wrote:
> > On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote:
> > >
> > > Yeah, synchronous signals should probably never be delivered to another
> > > process, even via signalfd. There's no point delivering a SEGV to
> > > somebody else :-)
> >
> > That'd be a limitation. Like you can choose to not handle SEGV, you can
> > choose to have a signalfd listening to it. Of course, not with the
> > intention to *handle* the signal, but with a notification intent.
>
> I agree that it would be a limitation, but it would be a sane one.
>
> How about we try to live with that limitation, if only to avoid the issue
> of having the private signals being stolen by anybody else. If we actually
> find a real-live use-case where that is bad in the future, we can re-visit
> the issue - it's always easier to _expand_ semantics later than it is to
> restrict them, so I think this thread is a good argument for starting it
> out in a more restricted form before people start depending on semantics
> that can be nasty..
>
> Linus
Proposed semantics:
a) Process-global signals can be read by any thread (inside or outside
of the process receiving the signal).
Rationale:
This should always work, so there's no reason to limit it.
b) Thread-specific signals can only be read by their target thread.
Rationale:
This behavior is required by POSIX, and if an application is using
pthread_kill()/tkill()/tgkill()/etc. to specifically direct a signal, it
damn well better get to where the app wants it to go.
c) Synchronous signals ("Naturally" generated SIGILL, SIGFPE, SIGSEGV,
SIGBUS, and SIGTRAP. Did I miss any?) are not delivered via signalfd()
at all. (And by "naturally" generated, I mean signals that would have
the SI_KERNEL flag set.)
Rationale:
These are a subset of thread-specific signals, so they can only be read
from a signalfd by their target thread.
However, there's no way for the target thread to get the signal because
it is either:
a) not blocked in a syscall waiting for signal delivery and thus further
execution beyond the instruction causing the signal is impossible
OR
b) it is blocked in a syscall waiting for signal delivery and the error
is caused by the signal delivery mechanism itself (i.e. a bad pointer
passed to read/select/poll/epoll_wait/etc.) and thus the signal can't be
delivered
--
Nicholas Miell <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
> That'd be a limitation. Like you can choose to not handle SEGV, you can > choose to have a signalfd listening to it. Of course, not with the > intention to *handle* the signal, but with a notification intent. Hrm.. either you handle it or you are dead ... I fail to see how signalfd can sneak in to catch it just at the right time... > > I'm actually thinking we shoud -also- only handle shared signals in > > dequeue_signal() when called from a different task. > > Why do you want to impose this? signalfd is a "sniffer", and the user > controls what it can dequeue/sniff or what not. I don't see a reason of > imposing such limits, unless there're clear technical issues. Well, a synchronous signal such a SIGSEGV, SIGILL or SIGFPE generally means that execution cannot continue unless the signal handler does something about it... I think you are opening a whole can of worms here. > > Well.. you certainly need to instanciate a signalfd for every thread in > > the process if you want to get shared signals for sure. > > Why? Or better, what do you mean for "instanciate"? Well, because the kernel makes the decision of which thread to target the signal for a shared signal at emission time (though it -can- be caught by another thread). /me reads more code to be sure.. Oh well, a read from signalfd created on one thread -will- catch any shared signal that was already pending, whatever thread the kernel decided to target it at, it seems (that is, whatever thread actually got TIF_SIGPENDING set), but will only catch private signals for _that_ thread (and I still think catching private signals is a wrong thing). However, I'm not sure about the wakeup condition. signalfd_deliver will wakeup anybody in the signalfd_list, which is -not- whoever is blocked in signalfd_read() unless I'm missing something. For your scheme to work, signalfd_read() you probably need to keep a separate list of people to be nofified of signals and add current to it from signalfd_list(). Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 5 Jun 2007, Davide Libenzi wrote: > On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: > > > > Yeah, synchronous signals should probably never be delivered to another > > process, even via signalfd. There's no point delivering a SEGV to > > somebody else :-) > > That'd be a limitation. Like you can choose to not handle SEGV, you can > choose to have a signalfd listening to it. Of course, not with the > intention to *handle* the signal, but with a notification intent. I agree that it would be a limitation, but it would be a sane one. How about we try to live with that limitation, if only to avoid the issue of having the private signals being stolen by anybody else. If we actually find a real-live use-case where that is bad in the future, we can re-visit the issue - it's always easier to _expand_ semantics later than it is to restrict them, so I think this thread is a good argument for starting it out in a more restricted form before people start depending on semantics that can be nasty.. Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: > On Tue, 2007-06-05 at 17:58 -0700, Nicholas Miell wrote: > > > > "At the time of generation, a determination shall be made whether the > > signal has been generated for the process or for a specific thread > > within the process. Signals which are generated by some action > > attributable to a particular thread, such as a hardware fault, shall > > be generated for the thread that caused the signal to be generated. > > Yeah, synchronous signals should probably never be delivered to another > process, even via signalfd. There's no point delivering a SEGV to > somebody else :-) That'd be a limitation. Like you can choose to not handle SEGV, you can choose to have a signalfd listening to it. Of course, not with the intention to *handle* the signal, but with a notification intent. > I'm actually thinking we shoud -also- only handle shared signals in > dequeue_signal() when called from a different task. Why do you want to impose this? signalfd is a "sniffer", and the user controls what it can dequeue/sniff or what not. I don't see a reason of imposing such limits, unless there're clear technical issues. > > dequeue_signal(tsk, ...) looks for signals first in tsk->pending and > > then in tsk->signal->shared_pending. > > > > sys_signalfd() stores current in signalfd_ctx. signalfd_read() passes > > that context to signalfd_dequeue, which passes that that saved > > task_struct pointer to dequeue_signal. > > > > This means that a signalfd will deliver signals targeted towards > > either the original thread that created that signalfd, or signals > > targeted towards the process as a whole. > > > > This means that a single signalfd is not adequate to handle signal > > delivery for all threads in a process, because signals targeted > > towards threads other than the thread that originally created the > > signalfd will never be queued to that signalfd. > > Well.. you certainly need to instanciate a signalfd for every thread in > the process if you want to get shared signals for sure. Why? Or better, what do you mean for "instanciate"? - Davide - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 2007-06-05 at 17:58 -0700, Nicholas Miell wrote: > > "At the time of generation, a determination shall be made whether the > signal has been generated for the process or for a specific thread > within the process. Signals which are generated by some action > attributable to a particular thread, such as a hardware fault, shall > be generated for the thread that caused the signal to be generated. Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) I'm actually thinking we shoud -also- only handle shared signals in dequeue_signal() when called from a different task. > dequeue_signal(tsk, ...) looks for signals first in tsk->pending and > then in tsk->signal->shared_pending. > > sys_signalfd() stores current in signalfd_ctx. signalfd_read() passes > that context to signalfd_dequeue, which passes that that saved > task_struct pointer to dequeue_signal. > > This means that a signalfd will deliver signals targeted towards > either the original thread that created that signalfd, or signals > targeted towards the process as a whole. > > This means that a single signalfd is not adequate to handle signal > delivery for all threads in a process, because signals targeted > towards threads other than the thread that originally created the > signalfd will never be queued to that signalfd. Well.. you certainly need to instanciate a signalfd for every thread in the process if you want to get shared signals for sure. BTW. Not directly related, but that notifier thing ... it looks really really dodgy. It's also only ever used by the DRM. Somebody around knows why that's in and why the DRM cannot just use normal signal blocking techniques ? Cheers, Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 2007-06-05 at 17:58 -0700, Nicholas Miell wrote: At the time of generation, a determination shall be made whether the signal has been generated for the process or for a specific thread within the process. Signals which are generated by some action attributable to a particular thread, such as a hardware fault, shall be generated for the thread that caused the signal to be generated. Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) I'm actually thinking we shoud -also- only handle shared signals in dequeue_signal() when called from a different task. dequeue_signal(tsk, ...) looks for signals first in tsk-pending and then in tsk-signal-shared_pending. sys_signalfd() stores current in signalfd_ctx. signalfd_read() passes that context to signalfd_dequeue, which passes that that saved task_struct pointer to dequeue_signal. This means that a signalfd will deliver signals targeted towards either the original thread that created that signalfd, or signals targeted towards the process as a whole. This means that a single signalfd is not adequate to handle signal delivery for all threads in a process, because signals targeted towards threads other than the thread that originally created the signalfd will never be queued to that signalfd. Well.. you certainly need to instanciate a signalfd for every thread in the process if you want to get shared signals for sure. BTW. Not directly related, but that notifier thing ... it looks really really dodgy. It's also only ever used by the DRM. Somebody around knows why that's in and why the DRM cannot just use normal signal blocking techniques ? Cheers, Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: On Tue, 2007-06-05 at 17:58 -0700, Nicholas Miell wrote: At the time of generation, a determination shall be made whether the signal has been generated for the process or for a specific thread within the process. Signals which are generated by some action attributable to a particular thread, such as a hardware fault, shall be generated for the thread that caused the signal to be generated. Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) That'd be a limitation. Like you can choose to not handle SEGV, you can choose to have a signalfd listening to it. Of course, not with the intention to *handle* the signal, but with a notification intent. I'm actually thinking we shoud -also- only handle shared signals in dequeue_signal() when called from a different task. Why do you want to impose this? signalfd is a sniffer, and the user controls what it can dequeue/sniff or what not. I don't see a reason of imposing such limits, unless there're clear technical issues. dequeue_signal(tsk, ...) looks for signals first in tsk-pending and then in tsk-signal-shared_pending. sys_signalfd() stores current in signalfd_ctx. signalfd_read() passes that context to signalfd_dequeue, which passes that that saved task_struct pointer to dequeue_signal. This means that a signalfd will deliver signals targeted towards either the original thread that created that signalfd, or signals targeted towards the process as a whole. This means that a single signalfd is not adequate to handle signal delivery for all threads in a process, because signals targeted towards threads other than the thread that originally created the signalfd will never be queued to that signalfd. Well.. you certainly need to instanciate a signalfd for every thread in the process if you want to get shared signals for sure. Why? Or better, what do you mean for instanciate? - Davide - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 5 Jun 2007, Davide Libenzi wrote: On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) That'd be a limitation. Like you can choose to not handle SEGV, you can choose to have a signalfd listening to it. Of course, not with the intention to *handle* the signal, but with a notification intent. I agree that it would be a limitation, but it would be a sane one. How about we try to live with that limitation, if only to avoid the issue of having the private signals being stolen by anybody else. If we actually find a real-live use-case where that is bad in the future, we can re-visit the issue - it's always easier to _expand_ semantics later than it is to restrict them, so I think this thread is a good argument for starting it out in a more restricted form before people start depending on semantics that can be nasty.. Linus - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
That'd be a limitation. Like you can choose to not handle SEGV, you can choose to have a signalfd listening to it. Of course, not with the intention to *handle* the signal, but with a notification intent. Hrm.. either you handle it or you are dead ... I fail to see how signalfd can sneak in to catch it just at the right time... I'm actually thinking we shoud -also- only handle shared signals in dequeue_signal() when called from a different task. Why do you want to impose this? signalfd is a sniffer, and the user controls what it can dequeue/sniff or what not. I don't see a reason of imposing such limits, unless there're clear technical issues. Well, a synchronous signal such a SIGSEGV, SIGILL or SIGFPE generally means that execution cannot continue unless the signal handler does something about it... I think you are opening a whole can of worms here. Well.. you certainly need to instanciate a signalfd for every thread in the process if you want to get shared signals for sure. Why? Or better, what do you mean for instanciate? Well, because the kernel makes the decision of which thread to target the signal for a shared signal at emission time (though it -can- be caught by another thread). /me reads more code to be sure.. Oh well, a read from signalfd created on one thread -will- catch any shared signal that was already pending, whatever thread the kernel decided to target it at, it seems (that is, whatever thread actually got TIF_SIGPENDING set), but will only catch private signals for _that_ thread (and I still think catching private signals is a wrong thing). However, I'm not sure about the wakeup condition. signalfd_deliver will wakeup anybody in the signalfd_list, which is -not- whoever is blocked in signalfd_read() unless I'm missing something. For your scheme to work, signalfd_read() you probably need to keep a separate list of people to be nofified of signals and add current to it from signalfd_list(). Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 2007-06-05 at 20:37 -0700, Linus Torvalds wrote: On Tue, 5 Jun 2007, Davide Libenzi wrote: On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) That'd be a limitation. Like you can choose to not handle SEGV, you can choose to have a signalfd listening to it. Of course, not with the intention to *handle* the signal, but with a notification intent. I agree that it would be a limitation, but it would be a sane one. How about we try to live with that limitation, if only to avoid the issue of having the private signals being stolen by anybody else. If we actually find a real-live use-case where that is bad in the future, we can re-visit the issue - it's always easier to _expand_ semantics later than it is to restrict them, so I think this thread is a good argument for starting it out in a more restricted form before people start depending on semantics that can be nasty.. Linus Proposed semantics: a) Process-global signals can be read by any thread (inside or outside of the process receiving the signal). Rationale: This should always work, so there's no reason to limit it. b) Thread-specific signals can only be read by their target thread. Rationale: This behavior is required by POSIX, and if an application is using pthread_kill()/tkill()/tgkill()/etc. to specifically direct a signal, it damn well better get to where the app wants it to go. c) Synchronous signals (Naturally generated SIGILL, SIGFPE, SIGSEGV, SIGBUS, and SIGTRAP. Did I miss any?) are not delivered via signalfd() at all. (And by naturally generated, I mean signals that would have the SI_KERNEL flag set.) Rationale: These are a subset of thread-specific signals, so they can only be read from a signalfd by their target thread. However, there's no way for the target thread to get the signal because it is either: a) not blocked in a syscall waiting for signal delivery and thus further execution beyond the instruction causing the signal is impossible OR b) it is blocked in a syscall waiting for signal delivery and the error is caused by the signal delivery mechanism itself (i.e. a bad pointer passed to read/select/poll/epoll_wait/etc.) and thus the signal can't be delivered -- Nicholas Miell [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
a) Process-global signals can be read by any thread (inside or outside of the process receiving the signal). Rationale: This should always work, so there's no reason to limit it. I agree, with an appropriate fix to recalc_sigpending_tsk() to only clear TIF_SIGPENDING if tsk == current (the patch Linus posted basically) _along_ with a fix to avoid the notifier thingy if stealing from another task, that would work. b) Thread-specific signals can only be read by their target thread. Rationale: This behavior is required by POSIX, and if an application is using pthread_kill()/tkill()/tgkill()/etc. to specifically direct a signal, it damn well better get to where the app wants it to go. I agree there too. I don't see the point of the 'feature' of allowing those to be stolen and can only lead into all sort of new headaches nobody needs. c) Synchronous signals (Naturally generated SIGILL, SIGFPE, SIGSEGV, SIGBUS, and SIGTRAP. Did I miss any?) are not delivered via signalfd() at all. (And by naturally generated, I mean signals that would have the SI_KERNEL flag set.) Heh, well, as you say later, it can't be delivered anyway... I don't think we need to do anything explicit to prevent them from being read() in signalfd, it will just not happen. Rationale: These are a subset of thread-specific signals, so they can only be read from a signalfd by their target thread. However, there's no way for the target thread to get the signal because it is either: a) not blocked in a syscall waiting for signal delivery and thus further execution beyond the instruction causing the signal is impossible OR b) it is blocked in a syscall waiting for signal delivery and the error is caused by the signal delivery mechanism itself (i.e. a bad pointer passed to read/select/poll/epoll_wait/etc.) and thus the signal can't be delivered Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)
On Tue, 5 Jun 2007, Linus Torvalds wrote: On Tue, 5 Jun 2007, Davide Libenzi wrote: On Wed, 6 Jun 2007, Benjamin Herrenschmidt wrote: Yeah, synchronous signals should probably never be delivered to another process, even via signalfd. There's no point delivering a SEGV to somebody else :-) That'd be a limitation. Like you can choose to not handle SEGV, you can choose to have a signalfd listening to it. Of course, not with the intention to *handle* the signal, but with a notification intent. I agree that it would be a limitation, but it would be a sane one. How about we try to live with that limitation, if only to avoid the issue of having the private signals being stolen by anybody else. If we actually find a real-live use-case where that is bad in the future, we can re-visit the issue - it's always easier to _expand_ semantics later than it is to restrict them, so I think this thread is a good argument for starting it out in a more restricted form before people start depending on semantics that can be nasty.. Yeah, that's easy. We can exclude them at signalfd creation time. - Davide - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
