Messages in this thread | | | Date | Thu, 29 Apr 2004 09:17:39 -0300 | From | Marcelo Tosatti <> | Subject | Re: [PATCH] per-user signal pending and message queue limits |
| |
On Wed, Apr 28, 2004 at 06:33:15PM -0700, Chris Wright wrote: > * Marcelo Tosatti (marcelo.tosatti@cyclades.com) wrote: > > This should be OK for inclusion into -mm now, if no other comment is made. > > This patch doesn't account for sigqueue bits properly. The allocations > aren't always made in task context. So, it's trivial to illegitimately > drain off the new signal_pending counter, leaving the potential for the > original DoS unfixed.
How?
> And the setuid issues still seem to be there, > right?
The setuid issues are not there anymore in mqueue (because of find_user(info->creator_id) at mqueue_delete_inode() time. The issue is still present with signal accounting, but we have a "> 0" check for that. And usually only root/CAP_SET_SUID is able to hurt himself (get unaccountable values in its quota). I dont think this really matters yet.
> Couple other nits below: > > Some bits need to be converted to tabs from spaces.
Yeap, there are some places where the "MAX_USER_SIGNALS" "MAX_USER_MSGQUEUE" names get too big in the RLIM_INIT resource.h definition. I'm bad at coming up with names. Any better suggestion for that (has to be smaller and meaningful). It means "maximum pending signals per user".
> Ditto.
Alright, needs to be fixed.
> > +++ linux-2.6.5/include/linux/signal.h 2004-04-27 08:32:46.000000000 -0300 > > @@ -7,6 +7,10 @@ > > #include <asm/siginfo.h> > > > > #ifdef __KERNEL__ > > + > > +#define MAX_QUEUED_SIGNALS 4096 > > Besides right below, is this really used anymore? > > > +#define MAX_USER_SIGNALS (MAX_QUEUED_SIGNALS/4) > > here.
Not really. Can replace with MAX_USER_SIGNALS 1024...
> > diff -Nur --show-c-function a/linux-2.6.5/kernel/signal.c linux-2.6.5/kernel/signal.c > > --- a/linux-2.6.5/kernel/signal.c 2004-04-27 09:53:24.000000000 -0300 > > +++ linux-2.6.5/kernel/signal.c 2004-04-27 11:05:08.000000000 -0300 > > @@ -31,8 +31,7 @@ > > > > static kmem_cache_t *sigqueue_cachep; > > > > -atomic_t nr_queued_signals; > > -int max_queued_signals = 1024; > > +int max_queued_signals = MAX_QUEUED_SIGNALS; > > and here, but max_queued_signals is no longer really relevant, right? > Should we removed both nr_queued_signals and max_queued_signals and the > associated sysctl's? Or leave it, and give CAP_SYS_RESOURCE the ability > to do a full override? I chose the latter, although I'm inclined to > drop that bit. > > > static void flush_sigqueue(struct sigpending *queue) > > @@ -700,11 +707,13 @@ static int send_signal(int sig, struct s > > make sure at least one signal gets delivered and don't > > pass on the info struct. */ > > > > - if (atomic_read(&nr_queued_signals) < max_queued_signals) > > + if (atomic_read(¤t->user->signal_pending) <= > > current may not be valid here. > > I have a diff between your patch and what I'm testing, but it's > cluttered a bit by the fact that I've also merged it up to 2.6.6-rc3 > I can send you the full patch if that's easier.
Please do so. Thanks!! - 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/
| |