[SRU][F][PATCH 1/1] UBUNTU: SAUCE: overlayfs: fix reference count mismatch

[Andrea Righi]

On Wed, Jun 28, 2023 at 05:32:21PM +0200, Aleksandr Mikhalitsyn wrote:
> On Thu, Jun 22, 2023 at 11:38 AM Andrea Righi
> <andrea.righi at canonical.com> wrote:
> >
> > BugLink: https://bugs.launchpad.net/bugs/2016398
> >
> > Opened files reported in /proc/pid/map_files can be shows with the wrong
> > mount point using overlayfs with filesystem namspaces.
> >
> > This incorrect behavior is fixed:
> >
> >   UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files
> >
> > However, the fix introduced a new regression, the reference to the
> > original file stored in vma->vm_prfile is not properly released when
> > vma->vm_prfile is replaced with a new file.
> >
> > This can cause a reference counter unbalance, leading errors such as
> > "target is busy" when trying to unmount overlayfs, even if the
> > filesystem has not active reference.
> >
> > Fix by properly releasing the original file stored in vm_prfile.
> >
> > Fixes: 508fdae3f62dd ("UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files")
> > Signed-off-by: Andrea Righi <andrea.righi at canonical.com>
> > ---
> >  fs/overlayfs/file.c | 59 +++++++++++++++++++++++++++------------------
> >  1 file changed, 36 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> > index 366a4267d5f8..565010d2c9dd 100644
> > --- a/fs/overlayfs/file.c
> > +++ b/fs/overlayfs/file.c
> > @@ -373,17 +373,48 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> >         return ret;
> >  }
> >
> > -/* handle vma->vm_prfile */
> > +/*
> > + * In map_files_get_link() (fs/proc/base.c)
> > + * we need to determine correct path from overlayfs.
> > + * But real_mount(realfile->f_path.mnt) may be not
> > + * equal to real_mount(file->f_path.mnt). In such case
> > + * fdinfo of the same file which was opened from
> > + * /proc/<pid>/map_files/... and "usual" path
> > + * will show different mnt_id.
> > + *
> > + * We solve issue like in aufs by using additional
> > + * field on struct vm_area_struct called "vm_prfile"
> > + * which is used only for fdinfo/"printing" needs.
> > + *
> > + * See also mm/prfile.c
> > + */
> > +#ifdef CONFIG_MMU
> >  static void ovl_vm_prfile_set(struct vm_area_struct *vma,
> >                               struct file *file)
> >  {
> >         get_file(file);
> > -       vma->vm_prfile = file;
> > -#ifndef CONFIG_MMU
> > +       swap(vma->vm_prfile, file);
> > +       /* Drop reference count from previous file value */
> > +       if (file)
> > +               fput(file);
> > +}
> > +#else
> > +static void ovl_vm_prfile_set(struct vm_area_struct *vma,
> > +                             struct file *file)
> > +{
> > +       struct file *vm_region_file = file;
> > +
> >         get_file(file);
> > -       vma->vm_region->vm_prfile = file;
> > -#endif
> > +       get_file(vm_region_file);
> > +       swap(vma->vm_prfile, file);
> > +       swap(vma->vm_region->vm_prfile, vm_region_file);
> > +       /* Drop reference count from previous file values */
> > +       if (file)
> > +               fput(file);
> > +       if (vm_region_file)
> > +               fput(vm_region_file);
> >  }
> > +#endif
> >
> >  static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
> >  {
> > @@ -411,25 +442,7 @@ static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
> >                 vma->vm_file = file;
> >                 fput(realfile);
> >         } else {
> > -               /*
> > -                * In map_files_get_link() (fs/proc/base.c)
> > -                * we need to determine correct path from overlayfs.
> > -                * But real_mount(realfile->f_path.mnt) may be not
> > -                * equal to real_mount(file->f_path.mnt). In such case
> > -                * fdinfo of the same file which was opened from
> > -                * /proc/<pid>/map_files/... and "usual" path
> > -                * will show different mnt_id.
> > -                *
> > -                * We solve issue like in aufs by using additional
> > -                * field on struct vm_area_struct called "vm_prfile"
> > -                * which is used only for fdinfo/"printing" needs.
> > -                *
> > -                * See also mm/prfile.c
> > -                */
> >                 ovl_vm_prfile_set(vma, file);
> > -
> > -               /* Drop reference count from previous vm_file value */
> > -               fput(file);
> 
> I'm not sure that we need to touch this line because it is not from
> the Ubuntu Sauce patch, but from the original Linux kernel tree.
> Removing it can cause a leak.

That line is not from the SAUCE patch, but before my change
ovl_vm_prfile_set() wasn't doing an fput(file). Now that
ovl_vm_prfile_set() is also taking care of doing fput(file), I think we
need to remove that line, otherwise we would release the file twice.

-Andrea