From mboxrd@z Thu Jan 1 00:00:00 1970 Reply-To: kernel-hardening@lists.openwall.com Date: Tue, 5 Jul 2011 10:57:55 -0500 From: Serge Hallyn Message-ID: <20110705155755.GB14784@peqn> References: <201106292214.p5TMEtHg015372@imap1.linux-foundation.org> <20110630134855.GA6165@mail.hallyn.com> <20110630135718.GA13406@albatros> <20110703180028.GA26742@albatros> <20110704115523.GA11252@albatros> <20110705142659.GA18290@peqn> <20110705145033.GA3052@albatros> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110705145033.GA3052@albatros> Subject: [kernel-hardening] Re: [PATCH] shm: handle separate PID namespaces case To: Vasiliy Kulikov Cc: akpm@linux-foundation.org, daniel.lezcano@free.fr, ebiederm@xmission.com, mingo@elte.hu, oleg@redhat.com, rdunlap@xenotime.net, tj@kernel.org, kernel-hardening@lists.openwall.com List-ID: Quoting 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. 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