Re: [PATCH] ovl: restore vma->vm_file to old file

[Miklos Szeredi <miklos@szeredi.hu>]

On Tue, Apr 20, 2021 at 4:08 AM Chengguang Xu <cgxu519@mykernel.net> wrote:
>
> In the error case of ->mmap() we should also restore vma->vm_file
> to old file in order to keep correct file reference in error path.
>
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>  fs/overlayfs/file.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 6e454a294046..046a7adb02c5 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -439,6 +439,7 @@ static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
>         if (ret) {
>                 /* Drop reference count from new vm_file value */
>                 fput(realfile);
> +               vma->vm_file = file;

That's interesting: commit 1527f926fd04 ("mm: mmap: fix fput in error
path v2") which went into 5.11-rc1 seems to have broke the refcounting
in overlayfs in the name of cleaning up a workaround.   Wondering if
there's any other damage done by this "fix"?

Changing refcounting rules in core kernel is no easy matter, a full
audit of ->mmap instances (>200) should have been done beforehand.

I suggest reverting this commit as a first step.

Thanks,
Miklos