On Wed, Apr 21, 2021 at 1:25 PM Christian König <christian.koenig@xxxxxxx> wrote: > > 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? No, the others seem to be okay. Thanks, Miklos
