[CRIU] [SRU][F][G][H][PATCH v2] UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files

[Alexander Mihalicyn]

Hi, Christian!

Thanks for your attention to the patch!
This field is introduced with aufs
(a3a49a17226f99a5e5dbdcbd328398cbf3c2959e) and it allows to have two struct
files connected with
vma. One for memory management needs and seconds for things like
proc/<pid>/map_files, numa_maps and so on.

Alex

On Tue, Apr 27, 2021 at 12:47 PM Christian Brauner <
christian.brauner at ubuntu.com> wrote:

> On Mon, Apr 26, 2021 at 11:11:21AM +0300, alexander at mihalicyn.com wrote:
> > From: Alexander Mikhalitsyn <alexander at mihalicyn.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1857257
> >
> > The hack was introduced in ("UBUNTU: SAUCE: overlayfs: allow with
> > shiftfs as underlay") and it broke checkpoint/restore of docker
> > contains:
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1857257
> >
> > The following script can be used to trigger the issue:
> >   #!/bin/bash
> >
> >   cat > test.py << EOF
> >   import sys
> >
> >   f = open("/proc/self/maps")
> >
> >   for l in f.readlines():
> >     if "python" not in l:
> >       continue
> >     print(l)
> >     s = l.split()
> >     start, end = s[0].split("-")
> >     fname = s[-1]
> >     print(start, end, fname)
> >     break
> >   else:
> >     sys.exit(1)
> >
> >   test_file1 = open(fname)
> >   test_file2 = open("/proc/self/map_files/%s-%s" % (start, end))
> >
> >   fdinfo1 = open("/proc/self/fdinfo/%d" % test_file1.fileno()).read()
> >   fdinfo2 = open("/proc/self/fdinfo/%d" % test_file2.fileno()).read()
> >
> >   if fdinfo1 != fdinfo2:
> >     print("FAIL")
> >     print(test_file1)
> >     print(fdinfo1)
> >     print(test_file2)
> >     print(fdinfo2)
> >     sys.exit(1)
> >   print("PASS")
> >   EOF
> >   sudo docker run -it --privileged --rm -v `pwd`:/mnt python python
> /mnt/test.py
> >
> > Thanks to Andrei Vagin for the reproducer and investigation of this
> problem.
> >
> > Cc: Andrei Vagin <avagin at gmail.com>
> > Cc: Adrian Reber <areber at redhat.com>
> > Cc: Christian Brauner <christian.brauner at ubuntu.com>
> > Cc: Stefan Bader <stefan.bader at canonical.com>
> > Cc: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski at canonical.com>
> >
> > Fixes: d24b8a5 ("UBUNTU: SAUCE: overlayfs: allow with shiftfs as
> underlay")
> > Signed-off-by: Alexander Mikhalitsyn <alexander at mihalicyn.com>
> > ---
>
> Hey,
>
> Thanks for the patch!
> Fwiw, Andrei already tried to fix this a while ago but it caused another
> regression which forced us to revert the fix for this issue.
>
> >  fs/overlayfs/file.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> > index 0d3ea0cf3e98..5fa520d0798e 100644
> > --- a/fs/overlayfs/file.c
> > +++ b/fs/overlayfs/file.c
> > @@ -325,6 +325,18 @@ static int ovl_fsync(struct file *file, loff_t
> start, loff_t end, int datasync)
> >       return ret;
> >  }
> >
> > +/* handle vma->vm_prfile */
> > +static void ovl_vm_prfile_set(struct vm_area_struct *vma,
> > +                           struct file *file)
> > +{
> > +     get_file(file);
> > +     vma->vm_prfile = file;
> > +#ifndef CONFIG_MMU
> > +     get_file(file);
> > +     vma->vm_region->vm_prfile = file;
> > +#endif
> > +}
>
> I'm confused struct vm_area_struct doesn't have a vm_prfile entry. Can
> you explain, please?
>
> > +
> >  static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
> >  {
> >       struct file *realfile = file->private_data;
> > @@ -351,6 +363,23 @@ 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);
> >       }
> > --
> > 2.30.2
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20210427/ae3811e1/attachment-0001.html>