linux-kernel - Re: [RFC] ipc: introduce shm_rmid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110630092103.GA4252@albatros>
Date:	Thu, 30 Jun 2011 13:21:03 +0400
From:	Vasiliy Kulikov <segoon@...nwall.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	kernel-hardening@...ts.openwall.com,
	Randy Dunlap <rdunlap@...otime.net>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"Serge E. Hallyn" <serge.hallyn@...onical.com>,
	Daniel Lezcano <daniel.lezcano@...e.fr>,
	Oleg Nesterov <oleg@...hat.com>, Tejun Heo <tj@...nel.org>,
	Ingo Molnar <mingo@...e.hu>, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [RFC] ipc: introduce shm_rmid_forced sysctl

On Wed, Jun 29, 2011 at 15:14 -0700, Andrew Morton wrote:
> What a horrid patch.  But given the POSIX (mis?)feature I don't see a
> better way, and the feature seems desirable.  Sigh.
> 
> What sort of users would want to turn this on, and why?

A shared environment, e.g. a shared web hosting.  With multiple ipc
namespaces (on a VPS) each namespace can steal a few megabytes of memory
without any per user accounting.  Even if all user's processes are
killed, the orphaned shm is still kept in RAM.  It doesn't lead to OOM,
but it prevents from creating new segments and just annoys.


> > --- a/include/linux/ipc_namespace.h
> > +++ b/include/linux/ipc_namespace.h
> > @@ -44,6 +44,7 @@ struct ipc_namespace {
> >  	size_t		shm_ctlall;
> >  	int		shm_ctlmni;
> >  	int		shm_tot;
> > +	int		shm_rmid_forced;
> >  
> >  	struct notifier_block ipcns_nb;
> 
> Please send a patch which adds a nice comment to this field.

OK.

> Perhaps shm_rmid_forced should have had bool type.

OK.  The sysctl handler should parse it OK if feed it with sizeof(bool).

> > --- a/ipc/shm.c
> > +++ b/ipc/shm.c
> > @@ -74,6 +74,7 @@ void shm_init_ns(struct ipc_namespace *ns)
> >  	ns->shm_ctlmax = SHMMAX;
> >  	ns->shm_ctlall = SHMALL;
> >  	ns->shm_ctlmni = SHMMNI;
> > +	ns->shm_rmid_forced = 0;
> >  	ns->shm_tot = 0;
> >  	ipc_init_ids(&shm_ids(ns));
> >  }
> 
> The problem is that nobody will test your feature.  So for testing
> purposes, let's enable the feature by default.  I assume this:
> 
> --- a/ipc/shm.c~ipc-introduce-shm_rmid_forced-sysctl-ipc-introduce-shm_rmid_forced-sysctl-testing
> +++ a/ipc/shm.c
> @@ -74,7 +74,7 @@ void shm_init_ns(struct ipc_namespace *n
>  	ns->shm_ctlmax = SHMMAX;
>  	ns->shm_ctlall = SHMALL;
>  	ns->shm_ctlmni = SHMMNI;
> -	ns->shm_rmid_forced = 0;
> +	ns->shm_rmid_forced = 1;
>  	ns->shm_tot = 0;
>  	ipc_init_ids(&shm_ids(ns));
>  }
> 
> will do that?

Sure if it helps testing :)

> > +static bool shm_may_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp)
> > +{
> > +	return (shp->shm_nattch == 0) &&
> > +	       (ns->shm_rmid_forced ||
> > +		(shp->shm_perm.mode & SHM_DEST));
> > +}
> 
> Just because the existing code is crappily documented doesn't mean that
> we have to copy that ;)

OK ;)


Thank you!

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/