Re: [RFC][PATCH 0/7] fanotify: add support for more events

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Marko Rauhamaa]

Amir Goldstein:
> This series is a prep work for using fanotify to monitor all events in
> a file system with a single watch.
>
> [...]
>
> I am posting this WIP to get feedback on the idea and to find out if
> there are any users out there interested in the improved fanotify
> capabilities and/or in the super block monitoring use case.

My employer certainly is in need of monitoring a whole filesystem. We
have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I was
thinking something like a FAN_MARK_FILESYSTEM would be needed.

(There are some other needed features but filesystem monitoring is the
most pressing one.)


Jan Kara:
> Careful here. In the world of user namespaces and containers you have
> to be really careful so that events from one container don't leak into
> another container despite they live in the same physical filesystem,
> just a different bind mount.

Obviously, proper care needs to be taken, but a namespace should not be
able smuggle filesystem events past fanotify monitoring.


Marko

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

On Thu, Oct 13, 2016 at 8:35 PM, Marko Rauhamaa
<marko.rauhamaa@f-secure.com> wrote:
>
> Amir Goldstein:
>> This series is a prep work for using fanotify to monitor all events in
>> a file system with a single watch.
>>
>> [...]
>>
>> I am posting this WIP to get feedback on the idea and to find out if
>> there are any users out there interested in the improved fanotify
>> capabilities and/or in the super block monitoring use case.
>
> My employer certainly is in need of monitoring a whole filesystem. We
> have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I was
> thinking something like a FAN_MARK_FILESYSTEM would be needed.
>

I have a POC of monitoring entire file system, while filtering to namespace
only the events that should be visible to its mounts.
I need to get the patches into shape and shake them a bit, then
I will post them and I am hoping that others could test them for their
use case as well.

I keep hearing about people that wanted that feature, but those people will
need to come forward and voice their use cases.

> (There are some other needed features but filesystem monitoring is the
> most pressing one.)
>
>
> Jan Kara:
>> Careful here. In the world of user namespaces and containers you have
>> to be really careful so that events from one container don't leak into
>> another container despite they live in the same physical filesystem,
>> just a different bind mount.
>
> Obviously, proper care needs to be taken, but a namespace should not be
> able smuggle filesystem events past fanotify monitoring.
>

I agree.

Cheers,
Amir.

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Marko Rauhamaa]

Amir Goldstein <amir73il@gmail.com>:

> On Thu, Oct 13, 2016 at 8:35 PM, Marko Rauhamaa
>> My employer certainly is in need of monitoring a whole filesystem. We
>> have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I
>> was thinking something like a FAN_MARK_FILESYSTEM would be needed.
>
> [...]
>
> I keep hearing about people that wanted that feature, but those people will
> need to come forward and voice their use cases.

Well, F-Secure's Linux Security product monitors files to detect
malware. Files are analyzed for viruses and unexpected modifications to
system files are flagged.


Other fanotify deficiencies include:

 * the offending process can die without leaving a trace because
   FAN_CLOSE_WRITE events do not block (instead of blocking, it would be
   enough for the /proc/$PID directory to stay available while the
   related fanotify fd is open)

 * the (e)uid and (e)gid of the offending process are not conveyed in
   the fanotify event

 * the FAN_OPEN_PERM event does not carry the mode so write access
   cannot be denied

 * there is no (PERM or non-PERM) event generated by the first
   modification (FAN_MODIFY generates a flurry of events;
   FAN_CLOSE_WRITE does not get generated unless the file is closed)


Marko

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

On Fri, Oct 14, 2016 at 11:28 AM, Marko Rauhamaa
<marko.rauhamaa@f-secure.com> wrote:
> Amir Goldstein <amir73il@gmail.com>:
>
>> On Thu, Oct 13, 2016 at 8:35 PM, Marko Rauhamaa
>>> My employer certainly is in need of monitoring a whole filesystem. We
>>> have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I
>>> was thinking something like a FAN_MARK_FILESYSTEM would be needed.
>>
>> [...]
>>
>> I keep hearing about people that wanted that feature, but those people will
>> need to come forward and voice their use cases.
>
> Well, F-Secure's Linux Security product monitors files to detect
> malware. Files are analyzed for viruses and unexpected modifications to
> system files are flagged.
>

This I could have guessed :-)

Let me rephrse my question - please argue why monitoring a file system
instead of a mount point is important for your use case and more impotantly,
please argue why you cannot achive the same result by monitoring all the
relevant mount points from user space.

For example, the argument I used against the legacy recursive intotify watch
of all directories in the treee is the poor ability to scale well over
millions of directories.


>
> Other fanotify deficiencies include:
>
>  * the offending process can die without leaving a trace because
>    FAN_CLOSE_WRITE events do not block (instead of blocking, it would be
>    enough for the /proc/$PID directory to stay available while the
>    related fanotify fd is open)
>
>  * the (e)uid and (e)gid of the offending process are not conveyed in
>    the fanotify event
>
>  * the FAN_OPEN_PERM event does not carry the mode so write access
>    cannot be denied
>
>  * there is no (PERM or non-PERM) event generated by the first
>    modification (FAN_MODIFY generates a flurry of events;
>    FAN_CLOSE_WRITE does not get generated unless the file is closed)
>
>

I am not sure I will be able help you wrt those extra requirements.

Amir.

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Marko Rauhamaa]

Amir Goldstein <amir73il@gmail.com>:
> Let me rephrse my question - please argue why monitoring a file system
> instead of a mount point is important for your use case and more impotantly,
> please argue why you cannot achive the same result by monitoring all the
> relevant mount points from user space.

We are already doing that and missing fanotify events that are shrouded
by namespaces. Namespaces are popular through the use of containers, but
not only that. Distros are using namespaces to protect the private files
of services:

   <URL: http://fedoraproject.org/wiki/Features/ServicesPrivateTmp>

   <URL: https://access.redhat.com/blogs/766093/posts/1976243>.

> For example, the argument I used against the legacy recursive intotify
> watch of all directories in the treee is the poor ability to scale
> well over millions of directories.

Sure, that would be untenable. I'd argue the namespace issue is at least
equally tough because you'd have to keep monitoring namespaces deep
under the /proc hierarchy where processes come and go, there is no
epollable notification scheme available (to my knowledge) and race
conditions are inevitable. It would be virtually impossible for a
top-level fanotify monitor to keep track of what is going on.


Marko

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

On Thu, Oct 13, 2016 at 9:42 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Oct 13, 2016 at 8:35 PM, Marko Rauhamaa
> <marko.rauhamaa@f-secure.com> wrote:
>>
>> Amir Goldstein:
>>> This series is a prep work for using fanotify to monitor all events in
>>> a file system with a single watch.
>>>
>>> [...]
>>>
>>> I am posting this WIP to get feedback on the idea and to find out if
>>> there are any users out there interested in the improved fanotify
>>> capabilities and/or in the super block monitoring use case.
>>
>> My employer certainly is in need of monitoring a whole filesystem. We
>> have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I was
>> thinking something like a FAN_MARK_FILESYSTEM would be needed.
>>
>
> I have a POC of monitoring entire file system, while filtering to namespace
> only the events that should be visible to its mounts.
> I need to get the patches into shape and shake them a bit, then
> I will post them and I am hoping that others could test them for their
> use case as well.
>

Marko,

The super block watch patches are available on my git tree at:
https://github.com/amir73il/linux/commits/fanotify_sb-4.9

I won't bother to post them to the list unless someone shows interest.
Note that beyond the ability to watch a file system, they also
provide all the legacy intotify events (create/remove), so maybe
that's also useful to you guys.

Anyway, if you are interested and willing to give them a try
please send me feedback.

Thanks,
Amir.

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Marko Rauhamaa]

Amir Goldstein <amir73il@gmail.com>:

> Marko,
>
> The super block watch patches are available on my git tree at:
> https://github.com/amir73il/linux/commits/fanotify_sb-4.9
>
> I won't bother to post them to the list unless someone shows interest.

Thank you, Amir! I will investigate your changes next week.


Marko

[RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

Hi all,

This series is a prep work for using fanotify to monitor all
events in a file system with a single watch.

The end result is indented to be an alternative to the recursive
inotify watches scheme, which has its problems.

This first part adds support for most inotify events to fanotify
when watching a directory.

The next part will add support for watching a super block,
which is not the same as watching a mount point.

I am posting this WIP to get feedback on the idea and to find
out if there are any users out there interested in the improved
fanotify capabilities and/or in the super block monitoring
use case.

Amir Goldstein (7):
  fsnotify: pass dentry instead of inode when available
  fsnotify: annotate filename events
  fanotify: new init flag FAN_EVENT_INFO_PARENT
  fanotify: store mount point from which an inode watch was added
  fanotify: support events with data type FSNOTIFY_EVENT_DENTRY
  fanotify: add support for create/attrib/rename/delete events
  fanotify: pass filename info for filename events

 fs/notify/fanotify/fanotify.c      | 85 +++++++++++++++++++++++++++++++----
 fs/notify/fanotify/fanotify.h      | 24 +++++++++-
 fs/notify/fanotify/fanotify_user.c | 92 ++++++++++++++++++++++++++++++++++----
 fs/notify/fdinfo.c                 |  4 +-
 fs/notify/fsnotify.c               |  2 +-
 fs/notify/inode_mark.c             |  1 +
 fs/notify/mark.c                   | 15 +++++--
 include/linux/fsnotify.h           | 46 ++++++++++++++-----
 include/linux/fsnotify_backend.h   | 24 +++++++---
 include/uapi/linux/fanotify.h      | 41 ++++++++++++++---
 10 files changed, 287 insertions(+), 47 deletions(-)

-- 
2.7.4

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

Hi Aleksander,

I dug up this thread from Tracker ml:
https://mail.gnome.org/archives/tracker-list/2011-September/msg00089.html

Linking to this posting on LKML:
https://lkml.org/lkml/2009/3/27/166

Is the Tracker project interested is a solution for efficient
monitoring of entire file system?
If only interested in unprivileged monitoring and/or recursive
directory monitoring,
the work could be extended in this direction (see details below)

Cheers,
Amir.


On Mon, Oct 10, 2016 at 10:12 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Hi all,
>
> This series is a prep work for using fanotify to monitor all
> events in a file system with a single watch.
>
> The end result is indented to be an alternative to the recursive
> inotify watches scheme, which has its problems.
>
> This first part adds support for most inotify events to fanotify
> when watching a directory.
>

Please note that in my current implementation of directory watch, you
get the directory fd
for create/move/delete events and the filename information is optional
data to event by an opt-in flag to fanotify_init.
Without filename data, those events can be more compact and easily merged,
in case user is only interested in coalesced "something changed here" event.

> The next part will add support for watching a super block,
> which is not the same as watching a mount point.
>

FYI, I also intend to (optionally) replace the fd information with
file handle for a super block watch.
That has a few benefits:
- Avoids the need to take references on path/dentries in the event queue
- Could pave the path to relaxing CAP_ADMIN requirement
- It serves as a replacement the rename cookie (and it persists across
any number of renames)

> I am posting this WIP to get feedback on the idea and to find
> out if there are any users out there interested in the improved
> fanotify capabilities and/or in the super block monitoring
> use case.
>
> Amir Goldstein (7):
>   fsnotify: pass dentry instead of inode when available
>   fsnotify: annotate filename events
>   fanotify: new init flag FAN_EVENT_INFO_PARENT
>   fanotify: store mount point from which an inode watch was added
>   fanotify: support events with data type FSNOTIFY_EVENT_DENTRY
>   fanotify: add support for create/attrib/rename/delete events
>   fanotify: pass filename info for filename events
>
>  fs/notify/fanotify/fanotify.c      | 85 +++++++++++++++++++++++++++++++----
>  fs/notify/fanotify/fanotify.h      | 24 +++++++++-
>  fs/notify/fanotify/fanotify_user.c | 92 ++++++++++++++++++++++++++++++++++----
>  fs/notify/fdinfo.c                 |  4 +-
>  fs/notify/fsnotify.c               |  2 +-
>  fs/notify/inode_mark.c             |  1 +
>  fs/notify/mark.c                   | 15 +++++--
>  include/linux/fsnotify.h           | 46 ++++++++++++++-----
>  include/linux/fsnotify_backend.h   | 24 +++++++---
>  include/uapi/linux/fanotify.h      | 41 ++++++++++++++---
>  10 files changed, 287 insertions(+), 47 deletions(-)
>
> --
> 2.7.4
>

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Jan Kara]

Hi,

On Mon 10-10-16 22:12:57, Amir Goldstein wrote:
> This series is a prep work for using fanotify to monitor all
> events in a file system with a single watch.
> 
> The end result is indented to be an alternative to the recursive
> inotify watches scheme, which has its problems.
> 
> This first part adds support for most inotify events to fanotify
> when watching a directory.

Well, and how is this better than what inotify supports? The proclaimed
advantage of fanotify() was that you get open file handle in the event
which prevents races when directory tree changes before you manage to
process inotify event. However with directory changes, you are again
starting to report names and so things become racy again. So I don't quite
see a value of reimplementing this in fanotify. The only benefit I see is
that you can watch the whole mountpoint with one watch instead of having to
add watch to every directory. Is that such a huge win? What is a use case
for that?

> The next part will add support for watching a super block,
> which is not the same as watching a mount point.

Careful here. In the world of user namespaces and containers you have to be
really careful so that events from one container don't leak into another
container despite they live in the same physical filesystem, just a
different bind mount. I believe chroot / bind mounts was one of the reasons
why fanotify ended with mountpoints and not with superblocks. But I guess
Eric or Al remember this better.

> I am posting this WIP to get feedback on the idea and to find
> out if there are any users out there interested in the improved
> fanotify capabilities and/or in the super block monitoring
> use case.

Well, I hope you have some usecases in mind when you propose this ;) I've
been asked about fanotify for superblocks but so far I think that doing it
in kernel would be a headache (mostly security-wise) and doing it in
userspace - watch every mountpoint in /proc/mounts - may be less
error-prone.

								Honza

> 
> Amir Goldstein (7):
>   fsnotify: pass dentry instead of inode when available
>   fsnotify: annotate filename events
>   fanotify: new init flag FAN_EVENT_INFO_PARENT
>   fanotify: store mount point from which an inode watch was added
>   fanotify: support events with data type FSNOTIFY_EVENT_DENTRY
>   fanotify: add support for create/attrib/rename/delete events
>   fanotify: pass filename info for filename events
> 
>  fs/notify/fanotify/fanotify.c      | 85 +++++++++++++++++++++++++++++++----
>  fs/notify/fanotify/fanotify.h      | 24 +++++++++-
>  fs/notify/fanotify/fanotify_user.c | 92 ++++++++++++++++++++++++++++++++++----
>  fs/notify/fdinfo.c                 |  4 +-
>  fs/notify/fsnotify.c               |  2 +-
>  fs/notify/inode_mark.c             |  1 +
>  fs/notify/mark.c                   | 15 +++++--
>  include/linux/fsnotify.h           | 46 ++++++++++++++-----
>  include/linux/fsnotify_backend.h   | 24 +++++++---
>  include/uapi/linux/fanotify.h      | 41 ++++++++++++++---
>  10 files changed, 287 insertions(+), 47 deletions(-)
> 
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

On Oct 11, 2016 9:42 PM, "Jan Kara" <jack@suse.cz> wrote:
>
> Hi,
>
> On Mon 10-10-16 22:12:57, Amir Goldstein wrote:
> > This series is a prep work for using fanotify to monitor all
> > events in a file system with a single watch.
> >
> > The end result is indented to be an alternative to the recursive
> > inotify watches scheme, which has its problems.
> >
> > This first part adds support for most inotify events to fanotify
> > when watching a directory.
>
> Well, and how is this better than what inotify supports? The proclaimed
> advantage of fanotify() was that you get open file handle in the event
> which prevents races when directory tree changes before you manage to
> process inotify event. However with directory changes, you are again
> starting to report names and so things become racy again. So I don't quite
> see a value of reimplementing this in fanotify.

For this part, I did not intend to add any benefit compared to inotify.
This is prep work to align fanotify with inotify feature set before I begin to
enhance fanotify to surpass inotify feature set. Besides, inotify was deemed
a bad API (rightfully IMO), and since my work requires adding more info to
the event data I expected that any attempt to modify the inotify API
would be rejected and re-directed towards using the newer and more flexible
fanotify API instead.

Also, please note that there is still a subtle and important difference between
inotify dir watch and fanotify dir watch presented by this work.
The child file changes are still reported with fd of the accessed
files themselves.
The dir entry name changes are reported with fd of the dir so there is
not really
any race issues as far as I can tell, only a recorded history of entry
name changes.
In fact, it is easy to report fd of the file in case of create and
move-to events,
but I chose the semantics of identifying the directory for all entry
name changes.

> The only benefit I see is
> that you can watch the whole mountpoint with one watch instead of having to
> add watch to every directory. Is that such a huge win? What is a use case
> for that?
>

Our use case is watching over 10M directories for continuous backup.
Maybe much more and for a workload where most of those directories are
seldom accessed. Inotify pins the directory inode for every watched directory.
This is not very scalable in terms of memory usage.

> > The next part will add support for watching a super block,
> > which is not the same as watching a mount point.
>
> Careful here. In the world of user namespaces and containers you have to be
> really careful so that events from one container don't leak into another
> container despite they live in the same physical filesystem, just a
> different bind mount. I believe chroot / bind mounts was one of the reasons
> why fanotify ended with mountpoints and not with superblocks. But I guess
> Eric or Al remember this better.
>

[cc ebiederman]

There are 2 sides to this argument. On the one hand, leaking file access
information (pid info and read/perm access events for that matter) across
namespaces should be done carefully or not done at all.
On the other hand, no namespace has exclusive ownership of the file system.
If a directory is accessible from several mount points be it different
namespaces or not, file system modifications affect all mounts from which
this directory is visible, so it makes sense to notify listeners from any mount
at least about MODIFY|ATTRIB|CREATE|MOVE|DELETE events in that directory.

Further more, fanotify currently requires CAP_ADMIN and I intend to enforce the
same capabilties required to mount a super block for watching a super block.
So containers with user namespace or with no CAP_ADMIN won't be able to use this
feature, but the container daemon will be able to use it to monitor
all files accessible
to the host. In my eyes this is a usability improvement, not an
information leak.

> > I am posting this WIP to get feedback on the idea and to find
> > out if there are any users out there interested in the improved
> > fanotify capabilities and/or in the super block monitoring
> > use case.
>
> Well, I hope you have some usecases in mind when you propose this ;) I've
> been asked about fanotify for superblocks

For file access events only? or for directory changes as well?
Because that feature is not avaialbe with fanotify mount watch
(neither with my work).

> but so far I think that doing it
> in kernel would be a headache (mostly security-wise) and doing it in
> userspace - watch every mountpoint in /proc/mounts - may be less
> error-prone.

Hmm, so I am all for implementing in userspace and monitoring mounts
would have been much better than monitoring super block.
In fact, our use case is actually recursive directory subtree watch and
super block watch is a compromise, because it requires more filtering
in userspace.
The reason I am aiming for super block watch is actually technical -
all of the directory fsnotify hooks do not have the 'file' or 'path' reference,
only the 'dentry' (and some only have the 'inode'), because they are buried deep
inside vfs helpers. I considered the option of propagating the dir
file reference
into the vfs helpers, but that seems unnecessarily complicated. Especially,
in light of the fact that I believe in the right of all mount namespaces to get
notified of changes in fs visible from their mounts.

Let me state my roadmap, because I realize it is not clear from this WIP:
1. fanotify supports directory events
2. fanotify supports super block watch
3. fanotify optionally reports file handles instead of file descriptors
4. fanotify filters out events on dentries that are not accessible
from the mount
    from which the watch was added

The implication of step #4 above it that the fsnotify infrastructure will report
any event on the super block to the fanotify backend and fanotify will
filter every reported dentry by walking up to root (at least for slow path),
looking for the root dentry of the mount from which the inode mark was added.

For some workloads it will make more sense to do recursive directory watch
the old inotify way and for some workloads and it would make sense to do
a filtered root watch. In either case, vfs performance does not suffer any
degradation outside the scope of the watch (either single dir or sb root).

Does this plan make sense?

Cheers,
Amir.


>
>                                                                 Honza
>
> >
> > Amir Goldstein (7):
> >   fsnotify: pass dentry instead of inode when available
> >   fsnotify: annotate filename events
> >   fanotify: new init flag FAN_EVENT_INFO_PARENT
> >   fanotify: store mount point from which an inode watch was added
> >   fanotify: support events with data type FSNOTIFY_EVENT_DENTRY
> >   fanotify: add support for create/attrib/rename/delete events
> >   fanotify: pass filename info for filename events
> >
> >  fs/notify/fanotify/fanotify.c      | 85 +++++++++++++++++++++++++++++++----
> >  fs/notify/fanotify/fanotify.h      | 24 +++++++++-
> >  fs/notify/fanotify/fanotify_user.c | 92 ++++++++++++++++++++++++++++++++++----
> >  fs/notify/fdinfo.c                 |  4 +-
> >  fs/notify/fsnotify.c               |  2 +-
> >  fs/notify/inode_mark.c             |  1 +
> >  fs/notify/mark.c                   | 15 +++++--
> >  include/linux/fsnotify.h           | 46 ++++++++++++++-----
> >  include/linux/fsnotify_backend.h   | 24 +++++++---
> >  include/uapi/linux/fanotify.h      | 41 ++++++++++++++---
> >  10 files changed, 287 insertions(+), 47 deletions(-)
> >
> > --
> > 2.7.4
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR