On Mon, Oct 9, 2023 at 9:43 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Sat, Oct 07, 2023 at 11:44:31AM +0300, Amir Goldstein wrote:
> > +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.
