Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757229Ab1F2WPD (ORCPT ); Wed, 29 Jun 2011 18:15:03 -0400 Received: from smtp1.linux-foundation.org ([140.211.169.13]:57126 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753571Ab1F2WO7 (ORCPT ); Wed, 29 Jun 2011 18:14:59 -0400 Date: Wed, 29 Jun 2011 15:14:36 -0700 From: Andrew Morton To: Vasiliy Kulikov Cc: kernel-hardening@lists.openwall.com, Randy Dunlap , "Eric W. Biederman" , "Serge E. Hallyn" , Daniel Lezcano , Oleg Nesterov , Tejun Heo , Ingo Molnar , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org Subject: Re: [RFC] ipc: introduce shm_rmid_forced sysctl Message-Id: <20110629151436.9be479fb.akpm@linux-foundation.org> In-Reply-To: <20110622152514.GA9521@albatros> References: <20110622152514.GA9521@albatros> X-Mailer: Sylpheed 3.0.2 (GTK+ 2.20.1; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2772 Lines: 83 On Wed, 22 Jun 2011 19:25:14 +0400 Vasiliy Kulikov 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/