Re: [PATCH v5 03/18] fsnotify: generate pre-content permission event on open

[Amir Goldstein <amir73il@gmail.com>]

On Wed, Sep 4, 2024 at 10:29 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> From: Amir Goldstein <amir73il@gmail.com>
>
> FS_PRE_ACCESS or FS_PRE_MODIFY will be generated on open depending on
> file open mode.  The pre-content event will be generated in addition to
> FS_OPEN_PERM, but without sb_writers held and after file was truncated
> in case file was opened with O_CREAT and/or O_TRUNC.
>
> The event will have a range info of (0..0) to provide an opportunity
> to fill entire file content on open.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> Reviewed-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/namei.c               |  9 +++++++++
>  include/linux/fsnotify.h | 10 +++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 3a4c40e12f78..c16487e3742d 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3735,6 +3735,15 @@ static int do_open(struct nameidata *nd,
>         }
>         if (do_truncate)
>                 mnt_drop_write(nd->path.mnt);
> +
> +       /*
> +        * This permission hook is different than fsnotify_open_perm() hook.
> +        * This is a pre-content hook that is called without sb_writers held
> +        * and after the file was truncated.
> +        */
> +       if (!error)
> +               error = fsnotify_file_perm(file, MAY_OPEN);
> +

Josef,

Please change that for v6 to:

                   error = fsnotify_file_area_perm(file, MAY_OPEN,
&file->f_pos, 0);

...

>         return error;
>  }
>
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 7600a0c045ba..fb3837b8de4c 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -168,6 +168,10 @@ static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
>                 fsnotify_mask = FS_PRE_MODIFY;
>         else if (perm_mask & (MAY_READ | MAY_ACCESS))
>                 fsnotify_mask = FS_PRE_ACCESS;
> +       else if (perm_mask & MAY_OPEN && file->f_mode & FMODE_WRITER)
> +               fsnotify_mask = FS_PRE_MODIFY;
> +       else if (perm_mask & MAY_OPEN)
> +               fsnotify_mask = FS_PRE_ACCESS;
>         else
>                 return 0;
>
> @@ -176,10 +180,14 @@ static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
>
>  /*
>   * fsnotify_file_perm - permission hook before file access
> + *
> + * Called from read()/write() with perm_mask MAY_READ/MAY_WRITE.
> + * Called from open() with MAY_OPEN without sb_writers held and after the file
> + * was truncated. Note that this is a different event from fsnotify_open_perm().
>   */
>  static inline int fsnotify_file_perm(struct file *file, int perm_mask)
>  {
> -       return fsnotify_file_area_perm(file, perm_mask, NULL, 0);
> +       return fsnotify_file_area_perm(file, perm_mask, &file->f_pos, 0);
>  }

... and drop this change, because this change will make readdir()
start reporting odd file ranges and HSM won't be able to tell the
difference between an opendir() and a readdir().

And I will send an extra patch for reporting an event with
range [size..size] for truncate(size).

Thanks,
Amir.