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: <20110629151436.9be479fb.akpm@linux-foundation.org>
Date:	Wed, 29 Jun 2011 15:14:36 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Vasiliy Kulikov <segoon@...nwall.com>
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, 22 Jun 2011 19:25:14 +0400
Vasiliy Kulikov <segoon@...nwall.com> wrote:

> This patch adds support for shm_rmid_forced sysctl.  If set to 1, all
> shared memory objects in current ipc namespace will be automatically
> forced to use IPC_RMID.  POSIX way of handling shmem allows to create
> shm objects and call shmdt() leaving shm object associated with no
> process, thus consuming memory not counted via rlimits.  With
> shm_rmid_forced=1 the shared memory object is counted at least for one
> process, so OOM killer may effectively kill the fat process holding
> the shared memory.
> 
> It obviously breaks POSIX, some programs relying on the feature would
> stop working.  So, set shm_rmid_forced=1 only if you're sure nobody uses
> "orphaned" memory.  shm_rmid_forced=0 by default for compatability
> reasons.
> 
> The feature was previously impemented in -ow as a configure option.
> 

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

Perhaps shm_rmid_forced should have had bool type.

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

> +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 ;)


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