Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 4 Jul 2011 19:48:06 +0400
From: Vasiliy Kulikov <segoon@...nwall.com>
To: Oleg Nesterov <oleg@...hat.com>
Cc: akpm@...ux-foundation.org, Serge Hallyn <serge.hallyn@...onical.com>,
	daniel.lezcano@...e.fr, ebiederm@...ssion.com, mingo@...e.hu,
	rdunlap@...otime.net, tj@...nel.org,
	kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH] shm: handle separate PID namespaces case

On Mon, Jul 04, 2011 at 17:37 +0200, Oleg Nesterov wrote:
> On 07/04, Vasiliy Kulikov wrote:
> >
> > On Mon, Jul 04, 2011 at 17:05 +0200, Oleg Nesterov wrote:
> > > On 07/04, Vasiliy Kulikov wrote:
> > > >
> > > > @@ -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;
> > >
> > > I know absolutely nothing about ipc/, so probably I am wrong. But do
> > > we really need shm_lock()
> >
> > It is needed to protect against parallel reads.  To read one may just
> > hold shm_lock, but to write both shm_lock and rw_mutex are needed.
> 
> Hmm. Still can't understand... Once again, it seems to me we can
> check shp->shm_creator != current and return lockless.

Ah, I understood what you're saying.  Yes, as before shm_destroy() we
change only ->shm_creator, which is checked only under rw_mutex, it is
safe to lock it just before shm_destroy().  So, ipc_lock_by_ptr()
instead of shm_lock() and only if we're going to destroy the segment.


Thanks again! :)

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

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.