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

["Christian König" <christian.koenig@amd.com>]

Am 21.04.21 um 13:14 schrieb Miklos Szeredi:
> On Wed, Apr 21, 2021 at 1:03 PM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 21.04.21 um 11:47 schrieb Miklos Szeredi:
>> [SNIP]
>> Can you give wider context? In other words why did the patch broke the
>> reference counting in overlayfs?
> In the error case overlayfs would put the reference on realfile (which
> is vma->vm_file at that point) and mmap_region() would put the
> reference to the original file (which was vma->vm_file before being
> overridden).
>
> After your commit mmap_region() puts the ref on the override vm_file,
> but not on the original file.

Ah, of course. Double checking the mmap callback implementation of 
overlayfs that is rather obvious.

>>> Changing refcounting rules in core kernel is no easy matter, a full
>>> audit of ->mmap instances (>200) should have been done beforehand.
>> Which is pretty much what was done, see the follow up commit:
>>
>> commit 295992fb815e791d14b18ef7cdbbaf1a76211a31 (able/vma_file)
>> Author: Christian König <christian.koenig@amd.com>
>> Date:   Mon Sep 14 15:09:33 2020 +0200
>>
>>       mm: introduce vma_set_file function v5
>>
>>       Add the new vma_set_file() function to allow changing
>>       vma->vm_file with the necessary refcount dance.
>>
>> It just looks like I missed the case in overlayfs while doing this.
> Yes.  And apparently a number of other cases where vm_file is assigned...

Yeah, I wasn't aware that filesystems do that as well and only 
concentrated on the drivers.

Just did a "grep -R 'vm_file[[:space:]]*=' on the full kernel source and 
it only showed one more case in fs/coda/file.c.

Do you see any other occurrences I potentially missed?

Otherwise I would say I provide patches for those two cases in a minute.

Thanks,
Christian.

>
> Thanks,
> Miklos