Re: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Roland McGrath ([EMAIL PROTECTED]) wrote: > > Yeah, it fixes the issue, but opens the door to larger consumption of > > pending signals. Roland, what was your final preference? I'm kind of > > leaning towards Jeremy's original patch. > > It's not a matter of preference. As I said in the first place, without my > patch we are violating POSIX, and delivering unreliable results to users. Right, and as you also mentioned, it's identical case to exhausting atomic pool, in either case we're out of resources, and in both cases the machine may recover and be functional. And sneaking around the rlimit can cost ~4k per-process, which is why I'd consider the edge case a reasonable loss. (heck, maybe 4k is fine considering task size, and mlock limits, etc). thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
> Yeah, it fixes the issue, but opens the door to larger consumption of > pending signals. Roland, what was your final preference? I'm kind of > leaning towards Jeremy's original patch. It's not a matter of preference. As I said in the first place, without my patch we are violating POSIX, and delivering unreliable results to users. Thanks, Roland - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Jeremy Fitzhardinge ([EMAIL PROTECTED]) wrote: > Roland McGrath wrote: > > >Indeed, I think your patch does not go far enough. I can read POSIX to say > >that the siginfo_t data must be available when `kill' was used, as well. > >This patch makes it allocate the siginfo_t, even when that exceeds > >{RLIMIT_SIGPENDING}, for any non-RT signal (< SIGRTMIN) not sent by > >sigqueue (actually, any signal that couldn't have been faked by a sigqueue > >call). > > > Looks OK to me. I'll give this a try soon. Yeah, it fixes the issue, but opens the door to larger consumption of pending signals. Roland, what was your final preference? I'm kind of leaning towards Jeremy's original patch. thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
Roland McGrath wrote: >Indeed, I think your patch does not go far enough. I can read POSIX to say >that the siginfo_t data must be available when `kill' was used, as well. >This patch makes it allocate the siginfo_t, even when that exceeds >{RLIMIT_SIGPENDING}, for any non-RT signal (< SIGRTMIN) not sent by >sigqueue (actually, any signal that couldn't have been faked by a sigqueue >call). > Looks OK to me. I'll give this a try soon. J - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
Roland McGrath wrote: Indeed, I think your patch does not go far enough. I can read POSIX to say that the siginfo_t data must be available when `kill' was used, as well. This patch makes it allocate the siginfo_t, even when that exceeds {RLIMIT_SIGPENDING}, for any non-RT signal ( SIGRTMIN) not sent by sigqueue (actually, any signal that couldn't have been faked by a sigqueue call). Looks OK to me. I'll give this a try soon. J - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Jeremy Fitzhardinge ([EMAIL PROTECTED]) wrote: Roland McGrath wrote: Indeed, I think your patch does not go far enough. I can read POSIX to say that the siginfo_t data must be available when `kill' was used, as well. This patch makes it allocate the siginfo_t, even when that exceeds {RLIMIT_SIGPENDING}, for any non-RT signal ( SIGRTMIN) not sent by sigqueue (actually, any signal that couldn't have been faked by a sigqueue call). Looks OK to me. I'll give this a try soon. Yeah, it fixes the issue, but opens the door to larger consumption of pending signals. Roland, what was your final preference? I'm kind of leaning towards Jeremy's original patch. thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
Yeah, it fixes the issue, but opens the door to larger consumption of pending signals. Roland, what was your final preference? I'm kind of leaning towards Jeremy's original patch. It's not a matter of preference. As I said in the first place, without my patch we are violating POSIX, and delivering unreliable results to users. Thanks, Roland - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Roland McGrath ([EMAIL PROTECTED]) wrote: Yeah, it fixes the issue, but opens the door to larger consumption of pending signals. Roland, what was your final preference? I'm kind of leaning towards Jeremy's original patch. It's not a matter of preference. As I said in the first place, without my patch we are violating POSIX, and delivering unreliable results to users. Right, and as you also mentioned, it's identical case to exhausting atomic pool, in either case we're out of resources, and in both cases the machine may recover and be functional. And sneaking around the rlimit can cost ~4k per-process, which is why I'd consider the edge case a reasonable loss. (heck, maybe 4k is fine considering task size, and mlock limits, etc). thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Roland McGrath ([EMAIL PROTECTED]) wrote: > > * Roland McGrath ([EMAIL PROTECTED]) wrote: > > > Indeed, I think your patch does not go far enough. I can read POSIX to > > > say > > > that the siginfo_t data must be available when `kill' was used, as well. > > > > How? I only see reference to filling in SI_USER for rt signals? > > Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs). > > There is stuff about a SA_SIGINFO signal handler's siginfo_t argument > "shall contain" the various specified information like si_pid/si_uid values > for a kill caller. OK, guess it's odd corner case, since they aren't queued anyway. > > Good point. Although it's RLIMIT_SIGPENDING + (31 * user_nprocs). So > > that could be 31 * 8k, for example. > > And a "good point" back to you, sir! I think the right way to think about > this in terms of resource consumption is that sizeof(struct sigqueue)*31 is > part of the potential per-process overhead that make up the consumption > units one should have in mind when choosing how to set the RLIMIT_NPROC limit. As in dynamic, and work with the patch that you sent to redo default sigpending as per nproc? thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
> * Roland McGrath ([EMAIL PROTECTED]) wrote: > > Indeed, I think your patch does not go far enough. I can read POSIX to say > > that the siginfo_t data must be available when `kill' was used, as well. > > How? I only see reference to filling in SI_USER for rt signals? > Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs). There is stuff about a SA_SIGINFO signal handler's siginfo_t argument "shall contain" the various specified information like si_pid/si_uid values for a kill caller. > Good point. Although it's RLIMIT_SIGPENDING + (31 * user_nprocs). So > that could be 31 * 8k, for example. And a "good point" back to you, sir! I think the right way to think about this in terms of resource consumption is that sizeof(struct sigqueue)*31 is part of the potential per-process overhead that make up the consumption units one should have in mind when choosing how to set the RLIMIT_NPROC limit. Thanks, Roland - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Roland McGrath ([EMAIL PROTECTED]) wrote: > Indeed, I think your patch does not go far enough. I can read POSIX to say > that the siginfo_t data must be available when `kill' was used, as well. How? I only see reference to filling in SI_USER for rt signals? Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs). > This patch makes it allocate the siginfo_t, even when that exceeds > {RLIMIT_SIGPENDING}, for any non-RT signal (< SIGRTMIN) not sent by > sigqueue (actually, any signal that couldn't have been faked by a sigqueue > call). Of course, in an extreme memory shortage situation, you are SOL and > violate POSIX a little before you die horribly from being out of memory > anyway. > The LEGACY_QUEUE logic already ensures that, for non-RT signals, at most > one is ever on the queue. So there really is no risk at all of unbounded > resource consumption; the usage can reach {RLIMIT_SIGPENDING} + 31, is all. Good point. Although it's RLIMIT_SIGPENDING + (31 * user_nprocs). So that could be 31 * 8k, for example. thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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/
[PATCH] override RLIMIT_SIGPENDING for non-RT signals
Indeed, I think your patch does not go far enough. I can read POSIX to say that the siginfo_t data must be available when `kill' was used, as well. This patch makes it allocate the siginfo_t, even when that exceeds {RLIMIT_SIGPENDING}, for any non-RT signal (< SIGRTMIN) not sent by sigqueue (actually, any signal that couldn't have been faked by a sigqueue call). Of course, in an extreme memory shortage situation, you are SOL and violate POSIX a little before you die horribly from being out of memory anyway. The LEGACY_QUEUE logic already ensures that, for non-RT signals, at most one is ever on the queue. So there really is no risk at all of unbounded resource consumption; the usage can reach {RLIMIT_SIGPENDING} + 31, is all. It's already the case that the limit can be exceeded by (in theory) up to {RLIMIT_NPROC}-1 in race conditions because the bump and the limit check are not atomic. (Obviously you can only get anywhere near that many with assloads of preemption, but exceeding it by a few is not too unlikely.) This patch also fixes that accounting so that it should not be possible to exceed {RLIMIT_SIGPENDING} + SIGRTMIN-1 queue items per user in races. Thanks, Roland Signed-off-by: Roland McGrath <[EMAIL PROTECTED]> --- linux-2.6/kernel/signal.c +++ linux-2.6/kernel/signal.c @@ -260,19 +260,23 @@ next_signal(struct sigpending *pending, return sig; } -static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags) +static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags, +int override_rlimit) { struct sigqueue *q = NULL; - if (atomic_read(>user->sigpending) < + atomic_inc(>user->sigpending); + if (override_rlimit || + atomic_read(>user->sigpending) <= t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur) q = kmem_cache_alloc(sigqueue_cachep, flags); - if (q) { + if (unlikely(q == NULL)) { + atomic_dec(>user->sigpending); + } else { INIT_LIST_HEAD(>list); q->flags = 0; q->lock = NULL; q->user = get_uid(t->user); - atomic_inc(>user->sigpending); } return(q); } @@ -793,7 +797,9 @@ static int send_signal(int sig, struct s make sure at least one signal gets delivered and don't pass on the info struct. */ - q = __sigqueue_alloc(t, GFP_ATOMIC); + q = __sigqueue_alloc(t, GFP_ATOMIC, (sig < SIGRTMIN && +((unsigned long) info < 2 || + info->si_code >= 0))); if (q) { list_add_tail(>list, >list); switch ((unsigned long) info) { @@ -1316,7 +1322,7 @@ struct sigqueue *sigqueue_alloc(void) { struct sigqueue *q; - if ((q = __sigqueue_alloc(current, GFP_KERNEL))) + if ((q = __sigqueue_alloc(current, GFP_KERNEL, 0))) q->flags |= SIGQUEUE_PREALLOC; return(q); } - 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/
[PATCH] override RLIMIT_SIGPENDING for non-RT signals
Indeed, I think your patch does not go far enough. I can read POSIX to say that the siginfo_t data must be available when `kill' was used, as well. This patch makes it allocate the siginfo_t, even when that exceeds {RLIMIT_SIGPENDING}, for any non-RT signal ( SIGRTMIN) not sent by sigqueue (actually, any signal that couldn't have been faked by a sigqueue call). Of course, in an extreme memory shortage situation, you are SOL and violate POSIX a little before you die horribly from being out of memory anyway. The LEGACY_QUEUE logic already ensures that, for non-RT signals, at most one is ever on the queue. So there really is no risk at all of unbounded resource consumption; the usage can reach {RLIMIT_SIGPENDING} + 31, is all. It's already the case that the limit can be exceeded by (in theory) up to {RLIMIT_NPROC}-1 in race conditions because the bump and the limit check are not atomic. (Obviously you can only get anywhere near that many with assloads of preemption, but exceeding it by a few is not too unlikely.) This patch also fixes that accounting so that it should not be possible to exceed {RLIMIT_SIGPENDING} + SIGRTMIN-1 queue items per user in races. Thanks, Roland Signed-off-by: Roland McGrath [EMAIL PROTECTED] --- linux-2.6/kernel/signal.c +++ linux-2.6/kernel/signal.c @@ -260,19 +260,23 @@ next_signal(struct sigpending *pending, return sig; } -static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags) +static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags, +int override_rlimit) { struct sigqueue *q = NULL; - if (atomic_read(t-user-sigpending) + atomic_inc(t-user-sigpending); + if (override_rlimit || + atomic_read(t-user-sigpending) = t-signal-rlim[RLIMIT_SIGPENDING].rlim_cur) q = kmem_cache_alloc(sigqueue_cachep, flags); - if (q) { + if (unlikely(q == NULL)) { + atomic_dec(t-user-sigpending); + } else { INIT_LIST_HEAD(q-list); q-flags = 0; q-lock = NULL; q-user = get_uid(t-user); - atomic_inc(q-user-sigpending); } return(q); } @@ -793,7 +797,9 @@ static int send_signal(int sig, struct s make sure at least one signal gets delivered and don't pass on the info struct. */ - q = __sigqueue_alloc(t, GFP_ATOMIC); + q = __sigqueue_alloc(t, GFP_ATOMIC, (sig SIGRTMIN +((unsigned long) info 2 || + info-si_code = 0))); if (q) { list_add_tail(q-list, signals-list); switch ((unsigned long) info) { @@ -1316,7 +1322,7 @@ struct sigqueue *sigqueue_alloc(void) { struct sigqueue *q; - if ((q = __sigqueue_alloc(current, GFP_KERNEL))) + if ((q = __sigqueue_alloc(current, GFP_KERNEL, 0))) q-flags |= SIGQUEUE_PREALLOC; return(q); } - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Roland McGrath ([EMAIL PROTECTED]) wrote: Indeed, I think your patch does not go far enough. I can read POSIX to say that the siginfo_t data must be available when `kill' was used, as well. How? I only see reference to filling in SI_USER for rt signals? Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs). This patch makes it allocate the siginfo_t, even when that exceeds {RLIMIT_SIGPENDING}, for any non-RT signal ( SIGRTMIN) not sent by sigqueue (actually, any signal that couldn't have been faked by a sigqueue call). Of course, in an extreme memory shortage situation, you are SOL and violate POSIX a little before you die horribly from being out of memory anyway. The LEGACY_QUEUE logic already ensures that, for non-RT signals, at most one is ever on the queue. So there really is no risk at all of unbounded resource consumption; the usage can reach {RLIMIT_SIGPENDING} + 31, is all. Good point. Although it's RLIMIT_SIGPENDING + (31 * user_nprocs). So that could be 31 * 8k, for example. thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Roland McGrath ([EMAIL PROTECTED]) wrote: Indeed, I think your patch does not go far enough. I can read POSIX to say that the siginfo_t data must be available when `kill' was used, as well. How? I only see reference to filling in SI_USER for rt signals? Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs). There is stuff about a SA_SIGINFO signal handler's siginfo_t argument shall contain the various specified information like si_pid/si_uid values for a kill caller. Good point. Although it's RLIMIT_SIGPENDING + (31 * user_nprocs). So that could be 31 * 8k, for example. And a good point back to you, sir! I think the right way to think about this in terms of resource consumption is that sizeof(struct sigqueue)*31 is part of the potential per-process overhead that make up the consumption units one should have in mind when choosing how to set the RLIMIT_NPROC limit. Thanks, Roland - 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: [PATCH] override RLIMIT_SIGPENDING for non-RT signals
* Roland McGrath ([EMAIL PROTECTED]) wrote: * Roland McGrath ([EMAIL PROTECTED]) wrote: Indeed, I think your patch does not go far enough. I can read POSIX to say that the siginfo_t data must be available when `kill' was used, as well. How? I only see reference to filling in SI_USER for rt signals? Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs). There is stuff about a SA_SIGINFO signal handler's siginfo_t argument shall contain the various specified information like si_pid/si_uid values for a kill caller. OK, guess it's odd corner case, since they aren't queued anyway. Good point. Although it's RLIMIT_SIGPENDING + (31 * user_nprocs). So that could be 31 * 8k, for example. And a good point back to you, sir! I think the right way to think about this in terms of resource consumption is that sizeof(struct sigqueue)*31 is part of the potential per-process overhead that make up the consumption units one should have in mind when choosing how to set the RLIMIT_NPROC limit. As in dynamic, and work with the patch that you sent to redo default sigpending as per nproc? thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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/