Re: [PATCH 0/3] extend get/setrlimit to support setting rlimitsexternal to a process (v7)
From: Neil Horman
Date: Thu Nov 05 2009 - 15:48:58 EST
On Wed, Nov 04, 2009 at 12:26:32PM +0100, Ingo Molnar wrote:
> * Neil Horman <nhorman@xxxxxxxxxxxxx> wrote:
> > On Mon, Nov 02, 2009 at 07:51:37PM +0100, Ingo Molnar wrote:
> > >
> > > * Neil Horman <nhorman@xxxxxxxxxxxxx> wrote:
> > >
> > > > > Have you ensured that no rlimit gets propagated during task init
> > > > > into some other value - under the previously correct assumption that
> > > > > rlimits dont change asynchronously under the feet of tasks?
> > > >
> > > > I've looked, and the only place that I see the rlim array getting
> > > > copied is via copy_signal when we're in the clone path. The
> > > > entire rlim array is copied from old task_struct to new
> > > > task_struct under the protection of the current->group_leader task
> > > > lock, which I also hold when updating via sys_setprlimit, so I
> > > > think we're safe in this case.
> > >
> > > I mean - do we set up any data structure based on a particular
> > > rlimit, that can get out of sync with the rlimit being updated?
> > >
> > > A prominent example would be the stack limit - we base address
> > > layout decisions on it. Check arch/x86/mm/mmap.c. RLIM_INFINITY has
> > > a special meaning plus we also set mmap_base() based on the rlim.
> > Ah, I didn't consider those. Yes it looks like some locking might be
> > needed for cases like that. what would you suggest, simply grabbing
> > the task lock before looking at the rlim array? That seems a bit
> > heavy handed, especially if we want to use the locking consistently.
> > What if we just converted the int array of rlimit to atomic_t's?
> > Would that be sufficient, or still to heavy?
Just to provide a quick update on this, it appears that (unbeknowst to me),
Jiri Slaby got almost this exact same feature in via the linux-next tree:
It still likely needs an audit to make sure theres no race with task access on
the rlimit array, but it doesn't currently require additional security checks
because the only access for a process to another processes limits is by writing
to the /proc/<pid>/limits file, as I had initial proposed. I think theres still
value in the sysscall, so I'll keep going with that aspect, but the rest of the
work appears done.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/