Re: [PATCH v2 1/3] fs: get mnt_writers count for an open backing file's real path

On Mon, Oct 9, 2023 at 9:43 AM Al Viro [off-list ref] wrote:

On Sat, Oct 07, 2023 at 11:44:31AM +0300, Amir Goldstein wrote:

quoted

+static inline void file_put_write_access(struct file *file)
+{
+     put_write_access(file->f_inode);
+     mnt_put_write_access(file->f_path.mnt);
+     if (unlikely(file->f_mode & FMODE_BACKING)) {
+             struct path *real_path = backing_file_real_path(file);
+
+             if (real_path->mnt)
+                     mnt_put_write_access(real_path->mnt);

IDGI.  Where do we get FMODE_BACKING combined with NULL real_path.mnt *AND*
put_file_access() possibly called?  Or file_get_write_access(), for
that matter...

Right.
I was being over prudent here.

FMODE_BACKING is set only in alloc_empty_backing_file().  The only caller
is backing_file_open(), which immediately sets real_path to its third
argument.  That could only come from ovl_open_realfile().  And if that
had been called with buggered struct path, it would have already blown
up on mnt_idmap(realpath->mnt).

The only interval where such beasts exist is from
        ff->file.f_mode |= FMODE_BACKING | FMODE_NOACCOUNT;
        return &ff->file;
in alloc_empty_backing_file() through

        f->f_path = *path;
        path_get(real_path);
        *backing_file_real_path(f) = *real_path;

in backing_file_open().  Where would that struct file (just allocated,
never seen outside of local variables in those two scopes) be passed
to get_file_write_access() or put_file_access()?

Or am I misreading something?

No. You are right.
I admit that I did consider adding use cases in the future
where a backing_file real_path is initialized lazily, but that
is not the case with current overlayfs code, so we don't
need to worry about that now.

Thanks,
Amir.