Comment 14 for bug 1967924

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote (last edit ):

Hi Andrea,

thanks a lot!

1. In my original patch here:
https://patchwork.ozlabs.<email address hidden>/

there was no "fput" call inside ovl_vm_prfile_set helper. fput was in the outer ovl_mmap() function in fs/overlayfs/file.c

Your last fix looks fully correct.

2. If we compare ovl_mmap function between 5.8 branch and 5.15:
https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/focal/tree/fs/overlayfs/file.c?h=hwe-5.8#n461

https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/focal/tree/fs/overlayfs/file.c?h=hwe-5.15-next&id=e3e60b65fa4db4722bd4b02a9dae58f7ff5d83d2#n528

we can see that ovl_mmap function was changed after:
https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/focal/commit/fs/overlayfs/file.c?h=hwe-5.15-next&id=2896900e22f8212606a1837d89a6bbce314ceeda

that's the point where the problem was introduced and ovl_vm_prfile_set with this "fput" become incorrect.

Right now, with your fix all seems correct.

3. But there is a question. Can we split this thing out of AUFS? As I mentioned before all that we really need here is this part:
https://github.com/JPyke3/mbp-manjaro-kernel/blob/master/aufs5-mmap.patch

Why we can't remove this `#if IS_ENABLED(CONFIG_AUFS_FS)` and use "shadow file" helper all the time?
I'm ready to prepare and test full version of patches to achieve that if it's needed.

Regards,
Alex