Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753142AbcJJQku (ORCPT ); Mon, 10 Oct 2016 12:40:50 -0400 Received: from mx2.suse.de ([195.135.220.15]:56106 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752274AbcJJQkt (ORCPT ); Mon, 10 Oct 2016 12:40:49 -0400 Date: Mon, 10 Oct 2016 18:40:46 +0200 From: Jan Kara To: Nikolay Borisov Cc: "Eric W. Biederman" , john@johnmccutchan.com, eparis@parisplace.org, viro@zeniv.linux.org.uk, jack@suse.cz, serge@hallyn.com, avagin@openvz.org, linux-kernel@vger.kernel.org, containers@lists.linux-foundation.org Subject: Re: [PATCH] inotify: Convert to using per-namespace limits Message-ID: <20161010164046.GG24081@quack2.suse.cz> References: <1475837161-4626-1-git-send-email-kernel@kyup.com> <8737k86n7q.fsf@x220.int.ebiederm.org> <57FB38C3.9090803@kyup.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57FB38C3.9090803@kyup.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2215 Lines: 50 On Mon 10-10-16 09:44:19, Nikolay Borisov wrote: > On 10/07/2016 09:14 PM, Eric W. Biederman wrote: > > Nikolay Borisov writes: > > > >> This patchset converts inotify to using the newly introduced > >> per-userns sysctl infrastructure. > >> > >> Currently the inotify instances/watches are being accounted in the > >> user_struct structure. This means that in setups where multiple > >> users in unprivileged containers map to the same underlying > >> real user (i.e. pointing to the same user_struct) the inotify limits > >> are going to be shared as well, allowing one user(or application) to exhaust > >> all others limits. > >> > >> Fix this by switching the inotify sysctls to using the > >> per-namespace/per-user limits. This will allow the server admin to > >> set sensible global limits, which can further be tuned inside every > >> individual user namespace. > >> > >> Signed-off-by: Nikolay Borisov > >> --- > >> Hello Eric, > >> > >> I saw you've finally sent your pull request for 4.9 and it > >> includes your implementatino of the ucount infrastructure. So > >> here is my respin of the inotify patches using that. > > > > Thanks. I will take a good hard look at this after -rc1 when things are > > stable enough that I can start a new development branch. > > > > I am a little concerned that the old sysctls have gone away. If no one > > cares it is fine, but if someone depends on them existing that may count > > as an unnecessary userspace regression. But otherwise skimming through > > this code it looks good. > > So this indeed this is real issue and I meant to write something about > it. Anyway, in order to preserve those sysctl what can be done is to > hook them up with a custom sysctl handler taking the ns from the proc > mount and the euid of current? I think this is a good approach, but > let's wait and see if anyone will have objections to completely > eliminating those sysctls. Well, I believe just discarding those sysctls is not an option - I'm pretty sure there are scripts out there which tune these sysctls and those would stop working. IMO not acceptable regression. Honza -- Jan Kara SUSE Labs, CR