All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruen@suse.de>
To: Eric Paris <eparis@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Matt Helsley <matthltc@us.ibm.com>,
	torvalds@linux-foundation.org, linux-kernel@vger.kernel.org,
	viro@zeniv.linux.org.uk, akpm@linux-foundation.org,
	Michael Kerrisk <michael.kerrisk@gmail.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [GIT PULL] notification tree: directory events
Date: Fri, 20 Aug 2010 13:25:03 +0200	[thread overview]
Message-ID: <201008201325.04218.agruen@suse.de> (raw)
In-Reply-To: <1282275497.21419.2073.camel@acb20005.ipt.aol.com>

On Friday 20 August 2010 05:38:17 Eric Paris wrote:
> So the actual bugs you have reported, I see two.
> 
> The (nearly) unbounded number of potential outstanding notifications
> events is a known situation, pointed out in previous discussions well
> before this commit and is one of the (numerous) reasons why fanotify is
> at this time CAP_SYS_ADMIN only.  It is something that is difficult to
> address while still making fanotify useful for permissions gating.  But
> the issue is clearly noted.

Clearly noting and blissfully ignoring the problem is not enough
unfortunately; this needs to be addressed now.  I have already pointed out 
(quoted below) that permission gating is not the worst problem here; the worst 
problem are listeners whose fanotify event queue just grows and grows.  There 
is no throttling, and no guarantee that even a listener which simply reads and 
completely ignores all events will manage to keep up.  The system will run out 
of memory eventually.

Here is a quote from a previous message about this problem that only went to 
linux-kernel:

On Tuesday 17 August 2010 05:39:47 Eric Paris wrote:
> On Mon, 2010-08-16 at 22:32 +0200, Andreas Gruenbacher wrote:
> > Q: What prevents the system from going out of memory when a listener
> > decides to stop reading events or simply can't keep up?  There doesn't
> > seem to be a limit on the queue depth.  Listeners currently need
> > CAP_SYS_ADMIN, but somehow limiting the queue depth and throttling when
> > things start to go bad still sounds like a reasonable thing to do,
> > right?
> 
> It's an interesting question and obviously one that I've thought about.
> You remember when we talked previously I said the hardest part left was
> allowing non-root users to use the interface.  It gets especially
> difficult when thinking about perm-events.  I was specifically told not
> to timeout or drop those.  But when dealing with non-root users using
> perm events?   As for pure notification we can do something like inotify
> does quite easily.
> 
> I'm not certain exactly what the best semantics are for non trusted
> users, so I didn't push any patches that way.  Suggestions welcome   :)

The system will happily go OOM for trusted users and non-perm events if the 
listener doesn't keep up, so some throttling, dropping, or both needs to 
happen for non-perm events.  This is the critical case.  Doing what inotify 
does (queue an overflow event and drop further events) seems to make sense 
here.

The situation with perm-events is less severe because the number of 
outstanding perm events is bounded by the number of running processes.  This 
may be enough of a limit.

I don't think we need to worry about perm-events for untrusted users.  We can 
start supporting some kinds of non-perm-events for untrusted users later; this 
won't change the existing interface.

Andreas

  parent reply	other threads:[~2010-08-20 11:25 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-06 15:58 [GIT PULL] notification tree - try 37! Eric Paris
2010-08-06 23:34 ` Matt Helsley
2010-08-07  0:06   ` Christoph Hellwig
2010-08-07 19:15     ` Eric Paris
2010-08-07 20:55       ` Matt Helsley
2010-08-16 20:32       ` Andreas Gruenbacher
2010-08-17  3:39         ` Eric Paris
2010-08-17  4:03           ` Matt Helsley
     [not found]           ` <1282016387.21419.113.camel-u/cB4NFi02V49Jlha2NJH1aTQe2KTcn/@public.gmane.org>
2010-08-17  4:03             ` Matt Helsley
2010-08-17  8:09           ` Andreas Gruenbacher
2010-08-17 15:08             ` Eric Paris
2010-08-19 20:24               ` Andreas Gruenbacher
2010-08-19 20:32                 ` Andreas Gruenbacher
2010-08-19 20:42                 ` Eric Paris
2010-08-19 21:07                   ` Andreas Gruenbacher
2010-08-19 21:22                     ` Andreas Gruenbacher
2010-08-20  3:50                     ` Eric Paris
2010-08-20 12:38                       ` Andreas Gruenbacher
2010-08-23 16:46                         ` Eric Paris
2010-08-23 22:38                           ` Andreas Gruenbacher
2010-08-20  0:00             ` Andreas Gruenbacher
2010-08-17  8:38           ` Andreas Gruenbacher
2010-08-17 15:24             ` Eric Paris
2010-08-17 15:48               ` Andreas Gruenbacher
2010-08-18 14:18               ` Andreas Gruenbacher
2010-08-17  9:45           ` Tvrtko Ursulin
2010-08-17 10:01             ` Andreas Gruenbacher
2010-08-17 10:12               ` Tvrtko Ursulin
2010-08-17 10:55                 ` Tvrtko Ursulin
2010-08-17 15:27                   ` Eric Paris
2010-08-18 15:47         ` [GIT PULL] notification tree: directory events Andreas Gruenbacher
2010-08-18 15:59           ` Eric Paris
2010-08-18 16:42             ` Christoph Hellwig
2010-08-18 17:07               ` Eric Paris
2010-08-19 12:44             ` Andreas Gruenbacher
2010-08-19 15:00               ` Eric Paris
2010-08-19 23:41                 ` Andreas Gruenbacher
2010-08-20  3:38                   ` Eric Paris
2010-08-20  5:19                     ` Andreas Dilger
2010-08-20  9:21                       ` Christoph Hellwig
2010-08-20 15:29                         ` Andreas Gruenbacher
2010-08-20 20:39                           ` Andreas Dilger
2010-08-20  9:09                     ` Tvrtko Ursulin
2010-08-20 11:07                     ` Andreas Gruenbacher
2010-08-20 11:25                     ` Andreas Gruenbacher [this message]
2010-08-20 12:16                     ` Andreas Gruenbacher

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=201008201325.04218.agruen@suse.de \
    --to=agruen@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=eparis@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthltc@us.ibm.com \
    --cc=michael.kerrisk@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.