On Mon, 10 May 2021 at 18:32, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> > I see, right. I agree that is unfortunate especially for stuff like audit
> > or fanotify permission events so we should fix that.
> >
>
> Miklos,
>
> Do you recall what is the reason for using FMODE_NONOTIFY
> for realfile?
Commit d989903058a8 ("ovl: do not generate duplicate fsnotify events
for "fake" path").
> I can see that events won't be generated anyway for watchers of
> underlying file, because fsnotify_file() looks at the "fake" path
> (i.e. the overlay file path).
>
> I recently looked at a similar issue w.r.t file_remove_privs() when
> I was looking at passing mnt context to notify_change() [1].
>
> My thinking was that we can change d_real() to provide the real path:
>
> static inline struct path d_real_path(struct path *path,
> const struct inode *inode)
> {
> struct realpath = {};
> if (!unlikely(dentry->d_flags & DCACHE_OP_REAL))
> return *path;
> dentry->d_op->d_real(path->dentry, inode, &realpath);
> return realpath;
> }
>
> static inline struct dentry *d_real(struct dentry *dentry,
> const struct inode *inode)
> {
> struct realpath = {};
> if (!unlikely(dentry->d_flags & DCACHE_OP_REAL))
> return dentry;
> dentry->d_op->d_real(path->dentry, inode, &realpath);
> return realpath.dentry;
> }
>
>
> Another option, instead of getting the realpath, just detect the
> mismatch of file_inode(file) != d_inode(path->dentry) in
> fanotify_file() and pass FSNOTIFY_EVENT_DENTRY data type
> with d_real() dentry to backend instead of FSNOTIFY_EVENT_PATH.
>
> For inotify it should be enough and for fanotify it is enough for
> FAN_REPORT_FID and legacy fanotify can report FAN_NOFD,
> so at least permission events listeners can identify the situation and
> be able to block access to unknown paths.
>
> Am I overcomplicating this?
>
> Any magic solution that I am missing?
Agree, dentry events should still happen.
Path events: what happens if you bind mount, then detach (lazy
umount)? Isn't that exactly the same as what overlayfs does on the
underlying mounts?
Thanks,
Miklos
