[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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
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.
