ACK: [SRU][J/I][PATCH 1/1] UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files

[Kleber Souza]

On 14.04.22 17:57, Andrea Righi wrote:
> From: Alexander Mikhalitsyn <alexander at mihalicyn.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1967924
> 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>
> Acked-by: Seth Forshee <seth.forshee at canonical.com>
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski at canonical.com>
> Signed-off-by: Kelsey Skunberg <kelsey.skunberg at canonical.com>
> [small refactoring to add the dependency of AUFS for vma->vm_prfile]
> Signed-off-by: Andrea Righi <andrea.righi at canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

Thanks

> ---
>   fs/overlayfs/file.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index d0c96ca0202e..362dd17b8a00 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -490,6 +490,41 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
>   	return ret;
>   }
>   
> +#if IS_ENABLED(CONFIG_AUFS_FS)
> +/*
> + * 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
> + */
> +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
> +	/* Drop reference count from previous vm_file value */
> +	fput(file);
> +}
> +#else /* !CONFIG_AUFS_FS */
> +static void ovl_vm_prfile_set(struct vm_area_struct *vma,
> +			      struct file *file)
> +{
> +}
> +#endif/* CONFIG_AUFS_FS */
> +
>   static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
>   {
>   	struct file *realfile = file->private_data;
> @@ -507,6 +542,10 @@ static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
>   	old_cred = ovl_override_creds(file_inode(file)->i_sb);
>   	ret = call_mmap(vma->vm_file, vma);
>   	revert_creds(old_cred);
> +
> +	if (!ret)
> +		ovl_vm_prfile_set(vma, file);
> +
>   	ovl_file_accessed(file);
>   
>   	return ret;