[kernel-hardening] Re: [PATCH] shm: handle separate PID namespaces case

[Vasiliy Kulikov <segoon@openwall.com>]

Hi Serge,

On Tue, Jul 05, 2011 at 09:26 -0500, Serge Hallyn wrote:
> > diff --git a/ipc/shm.c b/ipc/shm.c
> > index 22006f1..3baae98 100644
> > --- a/ipc/shm.c
> > +++ b/ipc/shm.c
> > @@ -239,7 +239,23 @@ static int shm_try_destroy_current(int id, void *p, void *data)
> >  	if (IS_ERR(shp))
> >  		return 0;
> >  
> > -	if (shp->shm_cprid != task_tgid_vnr(current)) {
> > +	if (shp->shm_creator != current) {
> > +		shm_unlock(shp);
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * Mark it as orphaned to destroy the segment when
> > +	 * kernel.shm_forced_rmid is changed.
> > +	 * It is noop if the following shm_may_destroy() returns true.
> > +	 */
> > +	shp->shm_creator = NULL;
> 
> This function, shm_try_destroy_current(), only gets called by shm_exit()
> if the shm_forced_rmid is set, right?  So something funky can happen if
> first shm_forced_rmid is 0 and some get created and the creating tasks
> exits, then shm_forced_rmid gets set to one, and the task pointer gets
> reused?

No, sinse this patch exit_shm() iterates all segments regardless of
shm_forced_rmid value (it is tried to _destroy_ the segment only if
shm_forced_rmid==1).  The ->shm_creator is set when the segment is
created and explicitly NULL'ed when the task exits.  As it has such
explicit rules, the ref counting is not needed at all.

Also ->shm_creator is not needed for anything, but for tracking whether
the creator has already exited, so keeping a reference neither to task
nor to pid is needed at all.


Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments