[PATCH v2] ovl: do not generate duplicate fsnotify events for "fake" path

[PATCH v2] ovl: do not generate duplicate fsnotify events for "fake" path

[Amir Goldstein]

Overlayfs "fake" path is used for stacked file operations on
underlying files.  Operations on files with "fake" path must not
generate fsnotify events with path data, because those events have
already been generated at overlayfs layer and because the reported
event->fd for fanotify marks on underlying inode/filesystem will
have the wrong path (the overlayfs path).

Link: https://lore.kernel.org/linux-fsdevel/20190423065024.12695-1-jencce.kernel@gmail.com/
Reported-by: Murphy Zhou <jencce.kernel@gmail.com>
Fixes: d1d04ef8572b ("ovl: stack file ops")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Miklos,

V1 was addressed to Jan and was trying to teach fsnotify about fake path.
V2 leaves fsnotify subsystem alone and just sets the FMODE_NONOTIFY
flag on realfile. All the rest of the complications with mark mounts
are irrelevant.

Same extended fanotify06 that was used to verify V1 also verified V2.

Thanks,
Amir.

 fs/overlayfs/file.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index ddfd93f13cc5..7d2f01957e40 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -29,10 +29,11 @@ static struct file *ovl_open_realfile(const struct file *file,
 	struct inode *inode = file_inode(file);
 	struct file *realfile;
 	const struct cred *old_cred;
+	int flags = file->f_flags | O_NOATIME | FMODE_NONOTIFY;
 
 	old_cred = ovl_override_creds(inode->i_sb);
-	realfile = open_with_fake_path(&file->f_path, file->f_flags | O_NOATIME,
-				       realinode, current_cred());
+	realfile = open_with_fake_path(&file->f_path, flags, realinode,
+				       current_cred());
 	revert_creds(old_cred);
 
 	pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n",
@@ -50,7 +51,7 @@ static int ovl_change_flags(struct file *file, unsigned int flags)
 	int err;
 
 	/* No atime modificaton on underlying */
-	flags |= O_NOATIME;
+	flags |= O_NOATIME | FMODE_NONOTIFY;
 
 	/* If some flag changed that cannot be changed then something's amiss */
 	if (WARN_ON((file->f_flags ^ flags) & ~OVL_SETFL_MASK))
-- 
2.17.1

Re: [PATCH v2] ovl: do not generate duplicate fsnotify events for "fake" path

[kbuild test robot]

Hi Amir,

I love your patch! Perhaps something to improve:

[auto build test WARNING on miklos-vfs/overlayfs-next]
[also build test WARNING on v5.1-rc6 next-20190424]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Amir-Goldstein/ovl-do-not-generate-duplicate-fsnotify-events-for-fake-path/20190425-030408
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)

>> fs/overlayfs/file.c:32:49: sparse: sparse: restricted fmode_t degrades to integer
   fs/overlayfs/file.c:54:30: sparse: sparse: restricted fmode_t degrades to integer

vim +32 fs/overlayfs/file.c

    25	
    26	static struct file *ovl_open_realfile(const struct file *file,
    27					      struct inode *realinode)
    28	{
    29		struct inode *inode = file_inode(file);
    30		struct file *realfile;
    31		const struct cred *old_cred;
  > 32		int flags = file->f_flags | O_NOATIME | FMODE_NONOTIFY;
    33	
    34		old_cred = ovl_override_creds(inode->i_sb);
    35		realfile = open_with_fake_path(&file->f_path, flags, realinode,
    36					       current_cred());
    37		revert_creds(old_cred);
    38	
    39		pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n",
    40			 file, file, ovl_whatisit(inode, realinode), file->f_flags,
    41			 realfile, IS_ERR(realfile) ? 0 : realfile->f_flags);
    42	
    43		return realfile;
    44	}
    45	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH v2] ovl: do not generate duplicate fsnotify events for "fake" path

[Murphy Zhou]

On Thu, Apr 25, 2019 at 12:39 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Overlayfs "fake" path is used for stacked file operations on
> underlying files.  Operations on files with "fake" path must not
> generate fsnotify events with path data, because those events have
> already been generated at overlayfs layer and because the reported
> event->fd for fanotify marks on underlying inode/filesystem will
> have the wrong path (the overlayfs path).
>
> Link: https://lore.kernel.org/linux-fsdevel/20190423065024.12695-1-jencce.kernel@gmail.com/
> Reported-by: Murphy Zhou <jencce.kernel@gmail.com>
> Fixes: d1d04ef8572b ("ovl: stack file ops")
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>
> Miklos,
>
> V1 was addressed to Jan and was trying to teach fsnotify about fake path.
> V2 leaves fsnotify subsystem alone and just sets the FMODE_NONOTIFY
> flag on realfile. All the rest of the complications with mark mounts
> are irrelevant.
>
> Same extended fanotify06 that was used to verify V1 also verified V2.

Thanks for fixing this!

My overlayfs tests on this patch looks good.

M

>
> Thanks,
> Amir.
>
>  fs/overlayfs/file.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index ddfd93f13cc5..7d2f01957e40 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -29,10 +29,11 @@ static struct file *ovl_open_realfile(const struct file *file,
>         struct inode *inode = file_inode(file);
>         struct file *realfile;
>         const struct cred *old_cred;
> +       int flags = file->f_flags | O_NOATIME | FMODE_NONOTIFY;
>
>         old_cred = ovl_override_creds(inode->i_sb);
> -       realfile = open_with_fake_path(&file->f_path, file->f_flags | O_NOATIME,
> -                                      realinode, current_cred());
> +       realfile = open_with_fake_path(&file->f_path, flags, realinode,
> +                                      current_cred());
>         revert_creds(old_cred);
>
>         pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n",
> @@ -50,7 +51,7 @@ static int ovl_change_flags(struct file *file, unsigned int flags)
>         int err;
>
>         /* No atime modificaton on underlying */
> -       flags |= O_NOATIME;
> +       flags |= O_NOATIME | FMODE_NONOTIFY;
>
>         /* If some flag changed that cannot be changed then something's amiss */
>         if (WARN_ON((file->f_flags ^ flags) & ~OVL_SETFL_MASK))
> --
> 2.17.1
>

Re: [PATCH v2] ovl: do not generate duplicate fsnotify events for "fake" path

[Miklos Szeredi]

On Sat, Apr 27, 2019 at 11:09 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>
> On Thu, Apr 25, 2019 at 12:39 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > Overlayfs "fake" path is used for stacked file operations on
> > underlying files.  Operations on files with "fake" path must not
> > generate fsnotify events with path data, because those events have
> > already been generated at overlayfs layer and because the reported
> > event->fd for fanotify marks on underlying inode/filesystem will
> > have the wrong path (the overlayfs path).
> >
> > Link: https://lore.kernel.org/linux-fsdevel/20190423065024.12695-1-jencce.kernel@gmail.com/
> > Reported-by: Murphy Zhou <jencce.kernel@gmail.com>
> > Fixes: d1d04ef8572b ("ovl: stack file ops")
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Miklos,
> >
> > V1 was addressed to Jan and was trying to teach fsnotify about fake path.
> > V2 leaves fsnotify subsystem alone and just sets the FMODE_NONOTIFY
> > flag on realfile. All the rest of the complications with mark mounts
> > are irrelevant.
> >
> > Same extended fanotify06 that was used to verify V1 also verified V2.
>
> Thanks for fixing this!
>
> My overlayfs tests on this patch looks good.

Thanks, applied.

Miklos