Am 21.04.21 um 13:14 schrieb Miklos Szeredi:
On Wed, Apr 21, 2021 at 1:03 PM Christian König
<christian.koenig@xxxxxxx> 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@xxxxxxx>
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
