linux-kernel - Re: [RFC PATCH 5/7] Fsnotify: Add a wrapper around the fsnotify function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxikoipcS9g6ShSovBUN+N=+CZGeKc0J27YQO3LYqcdLnA@mail.gmail.com>
Date:   Tue, 26 Oct 2021 17:37:55 +0300
From:   Amir Goldstein <amir73il@...il.com>
To:     Ioannis Angelakopoulos <iangelak@...hat.com>
Cc:     linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        virtio-fs-list <virtio-fs@...hat.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Jan Kara <jack@...e.cz>, Al Viro <viro@...iv.linux.org.uk>,
        Miklos Szeredi <miklos@...redi.hu>,
        Vivek Goyal <vgoyal@...hat.com>
Subject: Re: [RFC PATCH 5/7] Fsnotify: Add a wrapper around the fsnotify function

On Mon, Oct 25, 2021 at 11:47 PM Ioannis Angelakopoulos
<iangelak@...hat.com> wrote:
>
> Generally, inotify events are generated locally by calling the "fsnotify"
> function in fs/notify/fsnotify.c and various helper functions. However, now
> we expect events to arrive from the FUSE server. Thus, without any
> intervention a user space application will receive two events. One event is
> generated locally and one arrives from the server.
>
> Hence, to avoid duplicate events we need to "suppress" the local events
> generated by the guest kernel for FUSE inodes. To achieve this we add a
> wrapper around the "fsnotify" function in fs/notify/fsnotify.c that
> checks if the remote inotify is enabled and based on the check either it
> "suppresses" or lets through a local event.
>
> The wrapper will be called in the place of the original "fsnotify" call
> that is responsible for the event notification (now renamed as
> "__fsnotify").
>
> When the remote inotify is not enabled, all local events will be let
> through as expected. This process is completely transparent to user space.
>
> Signed-off-by: Ioannis Angelakopoulos <iangelak@...hat.com>
> ---
>  fs/notify/fsnotify.c             | 35 ++++++++++++++++++++++++++++++--
>  include/linux/fsnotify_backend.h | 14 ++++++++++++-
>  2 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 963e6ce75b96..848a824c29c4 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -440,7 +440,7 @@ static void fsnotify_iter_next(struct fsnotify_iter_info *iter_info)
>  }
>
>  /*
> - * fsnotify - This is the main call to fsnotify.
> + * __fsnotify - This is the main call to fsnotify.
>   *
>   * The VFS calls into hook specific functions in linux/fsnotify.h.
>   * Those functions then in turn call here.  Here will call out to all of the
> @@ -459,7 +459,7 @@ static void fsnotify_iter_next(struct fsnotify_iter_info *iter_info)
>   *             if both are non-NULL event may be reported to both.
>   * @cookie:    inotify rename cookie
>   */
> -int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
> +int __fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
>              const struct qstr *file_name, struct inode *inode, u32 cookie)
>  {
>         const struct path *path = fsnotify_data_path(data, data_type);
> @@ -552,6 +552,37 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
>
>         return ret;
>  }
> +
> +/*
> + * Wrapper around fsnotify. The main functionality is to filter local events in
> + * case the inode belongs to a filesystem that supports remote events
> + */
> +int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
> +            const struct qstr *file_name, struct inode *inode, u32 cookie)
> +{
> +
> +       if (inode != NULL || dir != NULL) {
> +               /*
> +                * Check if the fsnotify_event operation is available which
> +                * will let the remote inotify events go through and suppress
> +                * the local events
> +                */
> +               if (inode && inode->i_op->fsnotify_event) {
> +                       return inode->i_op->fsnotify_event(mask, data,
> +                                                          data_type, dir,
> +                                                          file_name, inode,
> +                                                          cookie);
> +               }
> +               if (dir && dir->i_op->fsnotify_event) {
> +                       return dir->i_op->fsnotify_event(mask, data,
> +                                                        data_type, dir,
> +                                                        file_name, inode,
> +                                                        cookie);
> +               }
> +       }
> +

That's not the way to accomplish what you want to do.

Assuming that we agree to let filesystem silence VFS fsnotify hooks
it should be done using an inode flag, similar to file flag FMODE_NONOTIFY.

But the reason you want to do that seems a bit odd.
Duplicate events are going to be merged with fanotify and I think that
you ruled out fanotify for the wrong reasons
(you should have only ruled out permission events)

Thanks,
Amir.