All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
To: Nikolay Borisov <kernel-6AxghH7DbtA@public.gmane.org>
Cc: jack-AlSwsSmVLrQ@public.gmane.org,
	avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	"Eric W. Biederman"
	<ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>,
	eparis-FjpueFixGhCM4zKIHC2jIg@public.gmane.org,
	john-jueV0HHMeujJJrXXpGQQMAC/G2K4zDHf@public.gmane.org
Subject: Re: [PATCH] inotify: Convert to using per-namespace limits
Date: Mon, 10 Oct 2016 18:40:46 +0200	[thread overview]
Message-ID: <20161010164046.GG24081@quack2.suse.cz> (raw)
In-Reply-To: <57FB38C3.9090803-6AxghH7DbtA@public.gmane.org>

On Mon 10-10-16 09:44:19, Nikolay Borisov wrote:
> On 10/07/2016 09:14 PM, Eric W. Biederman wrote:
> > Nikolay Borisov <kernel-6AxghH7DbtA@public.gmane.org> 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 <kernel-6AxghH7DbtA@public.gmane.org>
> >> ---
> >> 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 <jack-IBi9RG/b67k@public.gmane.org>
SUSE Labs, CR

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: Nikolay Borisov <kernel@kyup.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	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
Date: Mon, 10 Oct 2016 18:40:46 +0200	[thread overview]
Message-ID: <20161010164046.GG24081@quack2.suse.cz> (raw)
In-Reply-To: <57FB38C3.9090803@kyup.com>

On Mon 10-10-16 09:44:19, Nikolay Borisov wrote:
> On 10/07/2016 09:14 PM, Eric W. Biederman wrote:
> > Nikolay Borisov <kernel@kyup.com> 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 <kernel@kyup.com>
> >> ---
> >> 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 <jack@suse.com>
SUSE Labs, CR

  parent reply	other threads:[~2016-10-10 16:40 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-07 10:46 [PATCH] inotify: Convert to using per-namespace limits Nikolay Borisov
2016-10-07 10:46 ` Nikolay Borisov
2016-10-07 18:14 ` Eric W. Biederman
     [not found]   ` <8737k86n7q.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
2016-10-10  6:44     ` Nikolay Borisov
2016-10-10  6:44       ` Nikolay Borisov
     [not found]       ` <57FB38C3.9090803-6AxghH7DbtA@public.gmane.org>
2016-10-10 16:40         ` Jan Kara [this message]
2016-10-10 16:40           ` Jan Kara
     [not found]           ` <20161010164046.GG24081-4I4JzKEfoa/jFM9bn6wA6Q@public.gmane.org>
2016-10-10 20:49             ` Eric W. Biederman
2016-10-10 20:49               ` Eric W. Biederman
2016-10-10 21:54               ` Nikolay Borisov
     [not found]                 ` <CAJFSNy7q39M_=fZ2nOzPEhLcQ3o6B6ELFE-jsbkRuxsst1G1JQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-10 22:39                   ` Eric W. Biederman
2016-10-10 22:39                     ` Eric W. Biederman
     [not found]               ` <87eg3o3p6l.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
2016-10-10 21:54                 ` Nikolay Borisov
2016-10-11  7:36             ` [PATCH v2] " Nikolay Borisov
2016-10-11  7:36               ` Nikolay Borisov
2016-10-14  2:31               ` [inotify] 464e1236c3: BUG kmalloc-512 (Not tainted): Freepointer corrupt kernel test robot
2016-10-14  2:31                 ` [lkp] " kernel test robot
     [not found]               ` <1476171382-11911-1-git-send-email-kernel-6AxghH7DbtA@public.gmane.org>
2016-10-14  2:31                 ` kernel test robot
2016-10-24  6:47                 ` [PATCH v2] inotify: Convert to using per-namespace limits Nikolay Borisov
2016-10-24  6:47                   ` Nikolay Borisov
     [not found]                   ` <674564a4-ff05-b4d6-4a98-8cfc2e609325-6AxghH7DbtA@public.gmane.org>
2016-10-27 15:46                     ` Eric W. Biederman
2016-12-08  1:40                     ` Eric W. Biederman
2016-12-08  1:40                       ` Eric W. Biederman
     [not found]                       ` <87twafci52.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-12-08  6:58                         ` Nikolay Borisov
2016-12-08  6:58                           ` Nikolay Borisov
2016-12-09  2:50                           ` Eric W. Biederman
     [not found]                           ` <e86b3a4f-51d5-ea61-93f3-883fe2dbcf34-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-12-08  8:14                             ` Nikolay Borisov
2016-12-08  8:14                               ` Nikolay Borisov
     [not found]                               ` <100ecab5-6656-9ed6-c0f4-b9c97eae506a-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-12-09  5:38                                 ` Eric W. Biederman
2016-12-09  5:38                                   ` Eric W. Biederman
2016-12-09  2:50                             ` Eric W. Biederman
2016-10-27 15:46                   ` Eric W. Biederman
2016-10-24  7:48                 ` Jan Kara
2016-10-24  7:48                   ` Jan Kara
2016-11-14  6:04                 ` Serge E. Hallyn
2016-11-14  6:04                   ` Serge E. Hallyn
     [not found] ` <1475837161-4626-1-git-send-email-kernel-6AxghH7DbtA@public.gmane.org>
2016-10-07 18:14   ` [PATCH] " Eric W. Biederman
2016-10-09  5:55   ` [lkp] [inotify] 1109954e99: BUG kmalloc-512 (Not tainted): Freepointer corrupt kernel test robot
2016-10-09  5:55     ` kernel test robot
2016-10-09  5:55     ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161010164046.GG24081@quack2.suse.cz \
    --to=jack-alswssmvlrq@public.gmane.org \
    --cc=avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=eparis-FjpueFixGhCM4zKIHC2jIg@public.gmane.org \
    --cc=john-jueV0HHMeujJJrXXpGQQMAC/G2K4zDHf@public.gmane.org \
    --cc=kernel-6AxghH7DbtA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.