Re: [BUG][PATCH][2.6.36-rc3] fanotify: Do not ignore result of permission decisions

[Eric Paris <eparis@redhat.com>]

On Fri, 2010-10-08 at 17:11 +0100, Tvrtko Ursulin wrote:
> On Friday 08 Oct 2010 14:00:40 Andreas Gruenbacher wrote:
> > Tvrtko,
> >
> > On Wednesday 08 September 2010 10:24:04 Tvrtko Ursulin wrote:

> > >                 }
> > >
> > > +#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
> > > +               /* If a listener denied on a permission event we will
> > > remember the reason +                  and run the rest with a
> > > non-permission mask only. This allows other +                  listeners
> > > to receive non-permission notifications but we do not care +
> > >      about further permission checks and want to deny this event. */ +
> > >             if (unlikely((res == -EPERM) && (mask &
> > > FS_ALL_PERM_EVENTS))) {
> >
> > We should probably stop processing here whenever res != 0, for any mask.
> > (The fanotify and inotify handle_event callbacks can return -ENOMEM right
> > now, but this doesn't seem very useful and should probably be fixed:
> > fsnotify has no way of doing anything about -ENOMEM.
> 
> I wasn't 100% sure of how fanotify works, or in other words can there be more
> events in this mask after perm events are removed. If it can then we should
> send them to other listeners. Eric?
> 
> Also if ENOMEM, why not try sending to other listeners?

No, you'll never have a PERM_EVENT and something else at the same time.
If it's a PERM_EVENT denial we wouldn't really want to even notify
others, since the operation won't have actually taken place (perm event
denials should be passed all the way back up the stack)

As to ENOMEM that's a little harder I guess.  We don't have to stop just
because inotify got ENOMEM, but we should stop if PERM_EVENTS got an
ENOMEM.

I'd really rather not put CONFIG_FANOTIFY_ACCESS_PERMISSIONS in this
file.  fsnotify.c should be notifier agnostic.

How about just:

if (unlikely(ret && (mask & ALL_PERM)))
	return ret