Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110705155755.GB14784@peqn>
Date: Tue, 5 Jul 2011 10:57:55 -0500
From: Serge Hallyn <serge.hallyn@...onical.com>
To: Vasiliy Kulikov <segoon@...nwall.com>
Cc: akpm@...ux-foundation.org, daniel.lezcano@...e.fr,
	ebiederm@...ssion.com, mingo@...e.hu, oleg@...hat.com,
	rdunlap@...otime.net, tj@...nel.org,
	kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH] shm: handle separate PID namespaces case

Quoting Vasiliy Kulikov (segoon@...nwall.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.

Ok, cool.

> 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.

Do you have all of these patches applied in one git tree?  Could
you do a quick 'git diff master..' (assuming master is Linus' tree)
and send the result (or send a git url), and I'll take a last closer
look at the patch tomorrow?

thanks,
-serge

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.