Re: [PATCH v1] ovl: Fix leaked dentry
On Mon, Mar 29, 2021 at 6:48 PM Mickaël Salaün wrote: > > From: Mickaël Salaün > > Since commit 6815f479ca90 ("ovl: use only uppermetacopy state in > ovl_lookup()"), overlayfs doesn't put temporary dentry when there is a > metacopy error, which leads to dentry leaks when shutting down the > related superblock: > > overlayfs: refusing to follow metacopy origin for (/file0) > ... > BUG: Dentry (ptrval){i=3f33,n=file3} still in use (1) [unmount of > overlay overlay] > ... > WARNING: CPU: 1 PID: 432 at umount_check.cold+0x107/0x14d > CPU: 1 PID: 432 Comm: unmount-overlay Not tainted 5.12.0-rc5 #1 > ... > RIP: 0010:umount_check.cold+0x107/0x14d > ... > Call Trace: >d_walk+0x28c/0x950 >? dentry_lru_isolate+0x2b0/0x2b0 >? __kasan_slab_free+0x12/0x20 >do_one_tree+0x33/0x60 >shrink_dcache_for_umount+0x78/0x1d0 >generic_shutdown_super+0x70/0x440 >kill_anon_super+0x3e/0x70 >deactivate_locked_super+0xc4/0x160 >deactivate_super+0xfa/0x140 >cleanup_mnt+0x22e/0x370 >__cleanup_mnt+0x1a/0x30 >task_work_run+0x139/0x210 >do_exit+0xb0c/0x2820 >? __kasan_check_read+0x1d/0x30 >? find_held_lock+0x35/0x160 >? lock_release+0x1b6/0x660 >? mm_update_next_owner+0xa20/0xa20 >? reacquire_held_locks+0x3f0/0x3f0 >? __sanitizer_cov_trace_const_cmp4+0x22/0x30 >do_group_exit+0x135/0x380 >__do_sys_exit_group.isra.0+0x20/0x20 >__x64_sys_exit_group+0x3c/0x50 >do_syscall_64+0x45/0x70 >entry_SYSCALL_64_after_hwframe+0x44/0xae > ... > VFS: Busy inodes after unmount of overlay. Self-destruct in 5 seconds. > Have a nice day... > > This fix has been tested with a syzkaller reproducer. > > Cc: Amir Goldstein > Cc: Miklos Szeredi > Cc: Vivek Goyal > Cc: # v5.7+ > Reported-by: syzbot > Fixes: 6815f479ca90 ("ovl: use only uppermetacopy state in ovl_lookup()") > Signed-off-by: Mickaël Salaün > Link: https://lore.kernel.org/r/20210329164907.2133175-1-...@digikod.net Thanks, applied. Miklos
Re: [PATCH v1] ovl: Fix leaked dentry
On Mon, Mar 29, 2021 at 06:49:07PM +0200, Mickaël Salaün wrote: > From: Mickaël Salaün > > Since commit 6815f479ca90 ("ovl: use only uppermetacopy state in > ovl_lookup()"), overlayfs doesn't put temporary dentry when there is a > metacopy error, which leads to dentry leaks when shutting down the > related superblock: > Hi, Thanks for finding and fixing this bug. Patch looks correct to me. We need to drop that reference to this. I am not able to trigger this warning on umount of overlayfs. I copied up a file with metacopy enabled and then remounted overlay again with metacopy disabled. That does hit this code and I see the warning. refusing to follow metacopy origin for (/foo.txt) This should have lead to leak of dentry pointed by "this". But after that I unmounted, overlay and that succeeds. Is there any additional step to be done to trigger this VFS warning. Vivek > overlayfs: refusing to follow metacopy origin for (/file0) > ... > BUG: Dentry (ptrval){i=3f33,n=file3} still in use (1) [unmount of > overlay overlay] > ... > WARNING: CPU: 1 PID: 432 at umount_check.cold+0x107/0x14d > CPU: 1 PID: 432 Comm: unmount-overlay Not tainted 5.12.0-rc5 #1 > ... > RIP: 0010:umount_check.cold+0x107/0x14d > ... > Call Trace: >d_walk+0x28c/0x950 >? dentry_lru_isolate+0x2b0/0x2b0 >? __kasan_slab_free+0x12/0x20 >do_one_tree+0x33/0x60 >shrink_dcache_for_umount+0x78/0x1d0 >generic_shutdown_super+0x70/0x440 >kill_anon_super+0x3e/0x70 >deactivate_locked_super+0xc4/0x160 >deactivate_super+0xfa/0x140 >cleanup_mnt+0x22e/0x370 >__cleanup_mnt+0x1a/0x30 >task_work_run+0x139/0x210 >do_exit+0xb0c/0x2820 >? __kasan_check_read+0x1d/0x30 >? find_held_lock+0x35/0x160 >? lock_release+0x1b6/0x660 >? mm_update_next_owner+0xa20/0xa20 >? reacquire_held_locks+0x3f0/0x3f0 >? __sanitizer_cov_trace_const_cmp4+0x22/0x30 >do_group_exit+0x135/0x380 >__do_sys_exit_group.isra.0+0x20/0x20 >__x64_sys_exit_group+0x3c/0x50 >do_syscall_64+0x45/0x70 >entry_SYSCALL_64_after_hwframe+0x44/0xae > ... > VFS: Busy inodes after unmount of overlay. Self-destruct in 5 seconds. > Have a nice day... > > This fix has been tested with a syzkaller reproducer. > > Cc: Amir Goldstein > Cc: Miklos Szeredi > Cc: Vivek Goyal > Cc: # v5.7+ > Reported-by: syzbot > Fixes: 6815f479ca90 ("ovl: use only uppermetacopy state in ovl_lookup()") > Signed-off-by: Mickaël Salaün > Link: https://lore.kernel.org/r/20210329164907.2133175-1-...@digikod.net > --- > fs/overlayfs/namei.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > index 3fe05fb5d145..424c594afd79 100644 > --- a/fs/overlayfs/namei.c > +++ b/fs/overlayfs/namei.c > @@ -921,6 +921,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct > dentry *dentry, > if ((uppermetacopy || d.metacopy) && !ofs->config.metacopy) { > err = -EPERM; > pr_warn_ratelimited("refusing to follow metacopy origin > for (%pd2)\n", dentry); > + dput(this); > goto out_put; > } > > > base-commit: a5e13c6df0e41702d2b2c77c8ad41677ebb065b3 > -- > 2.30.2 >
Re: [PATCH v1] ovl: Fix leaked dentry
On Mon, Mar 29, 2021 at 06:49:07PM +0200, Mickaël Salaün wrote: > From: Mickaël Salaün > > Since commit 6815f479ca90 ("ovl: use only uppermetacopy state in > ovl_lookup()"), overlayfs doesn't put temporary dentry when there is a > metacopy error, which leads to dentry leaks when shutting down the > related superblock: > > overlayfs: refusing to follow metacopy origin for (/file0) > ... > BUG: Dentry (ptrval){i=3f33,n=file3} still in use (1) [unmount of > overlay overlay] > ... > WARNING: CPU: 1 PID: 432 at umount_check.cold+0x107/0x14d > CPU: 1 PID: 432 Comm: unmount-overlay Not tainted 5.12.0-rc5 #1 > ... > RIP: 0010:umount_check.cold+0x107/0x14d > ... > Call Trace: >d_walk+0x28c/0x950 >? dentry_lru_isolate+0x2b0/0x2b0 >? __kasan_slab_free+0x12/0x20 >do_one_tree+0x33/0x60 >shrink_dcache_for_umount+0x78/0x1d0 >generic_shutdown_super+0x70/0x440 >kill_anon_super+0x3e/0x70 >deactivate_locked_super+0xc4/0x160 >deactivate_super+0xfa/0x140 >cleanup_mnt+0x22e/0x370 >__cleanup_mnt+0x1a/0x30 >task_work_run+0x139/0x210 >do_exit+0xb0c/0x2820 >? __kasan_check_read+0x1d/0x30 >? find_held_lock+0x35/0x160 >? lock_release+0x1b6/0x660 >? mm_update_next_owner+0xa20/0xa20 >? reacquire_held_locks+0x3f0/0x3f0 >? __sanitizer_cov_trace_const_cmp4+0x22/0x30 >do_group_exit+0x135/0x380 >__do_sys_exit_group.isra.0+0x20/0x20 >__x64_sys_exit_group+0x3c/0x50 >do_syscall_64+0x45/0x70 >entry_SYSCALL_64_after_hwframe+0x44/0xae > ... > VFS: Busy inodes after unmount of overlay. Self-destruct in 5 seconds. > Have a nice day... > > This fix has been tested with a syzkaller reproducer. > Looks good to me. I realized that dentry leak will happen on underlying filesystem so unmount of underlying filesystem will give this warning. I created nested overlayfs configuration and could reproduce this error and tested that this patch fixes it. Reviewed-by: Vivek Goyal Vivek > Cc: Amir Goldstein > Cc: Miklos Szeredi > Cc: Vivek Goyal > Cc: # v5.7+ > Reported-by: syzbot > Fixes: 6815f479ca90 ("ovl: use only uppermetacopy state in ovl_lookup()") > Signed-off-by: Mickaël Salaün > Link: https://lore.kernel.org/r/20210329164907.2133175-1-...@digikod.net > --- > fs/overlayfs/namei.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > index 3fe05fb5d145..424c594afd79 100644 > --- a/fs/overlayfs/namei.c > +++ b/fs/overlayfs/namei.c > @@ -921,6 +921,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct > dentry *dentry, > if ((uppermetacopy || d.metacopy) && !ofs->config.metacopy) { > err = -EPERM; > pr_warn_ratelimited("refusing to follow metacopy origin > for (%pd2)\n", dentry); > + dput(this); > goto out_put; > } > > > base-commit: a5e13c6df0e41702d2b2c77c8ad41677ebb065b3 > -- > 2.30.2 >
Re: [PATCH v1] ovl: Fix leaked dentry
On 29/03/2021 18:49, Mickaël Salaün wrote: > From: Mickaël Salaün > > Since commit 6815f479ca90 ("ovl: use only uppermetacopy state in > ovl_lookup()"), overlayfs doesn't put temporary dentry when there is a > metacopy error, which leads to dentry leaks when shutting down the > related superblock: > > overlayfs: refusing to follow metacopy origin for (/file0) > ... > BUG: Dentry (ptrval){i=3f33,n=file3} still in use (1) [unmount of > overlay overlay] > ... > WARNING: CPU: 1 PID: 432 at umount_check.cold+0x107/0x14d > CPU: 1 PID: 432 Comm: unmount-overlay Not tainted 5.12.0-rc5 #1 > ... > RIP: 0010:umount_check.cold+0x107/0x14d > ... > Call Trace: >d_walk+0x28c/0x950 >? dentry_lru_isolate+0x2b0/0x2b0 >? __kasan_slab_free+0x12/0x20 >do_one_tree+0x33/0x60 >shrink_dcache_for_umount+0x78/0x1d0 >generic_shutdown_super+0x70/0x440 >kill_anon_super+0x3e/0x70 >deactivate_locked_super+0xc4/0x160 >deactivate_super+0xfa/0x140 >cleanup_mnt+0x22e/0x370 >__cleanup_mnt+0x1a/0x30 >task_work_run+0x139/0x210 >do_exit+0xb0c/0x2820 >? __kasan_check_read+0x1d/0x30 >? find_held_lock+0x35/0x160 >? lock_release+0x1b6/0x660 >? mm_update_next_owner+0xa20/0xa20 >? reacquire_held_locks+0x3f0/0x3f0 >? __sanitizer_cov_trace_const_cmp4+0x22/0x30 >do_group_exit+0x135/0x380 >__do_sys_exit_group.isra.0+0x20/0x20 >__x64_sys_exit_group+0x3c/0x50 >do_syscall_64+0x45/0x70 >entry_SYSCALL_64_after_hwframe+0x44/0xae > ... > VFS: Busy inodes after unmount of overlay. Self-destruct in 5 seconds. > Have a nice day... > > This fix has been tested with a syzkaller reproducer. > > Cc: Amir Goldstein > Cc: Miklos Szeredi > Cc: Vivek Goyal > Cc: # v5.7+ The bogus commit 6815f479ca90 was applied on v5.7-rc2 but it is only available since v5.8+ . > Reported-by: syzbot > Fixes: 6815f479ca90 ("ovl: use only uppermetacopy state in ovl_lookup()") > Signed-off-by: Mickaël Salaün > Link: https://lore.kernel.org/r/20210329164907.2133175-1-...@digikod.net > --- > fs/overlayfs/namei.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > index 3fe05fb5d145..424c594afd79 100644 > --- a/fs/overlayfs/namei.c > +++ b/fs/overlayfs/namei.c > @@ -921,6 +921,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct > dentry *dentry, > if ((uppermetacopy || d.metacopy) && !ofs->config.metacopy) { > err = -EPERM; > pr_warn_ratelimited("refusing to follow metacopy origin > for (%pd2)\n", dentry); > + dput(this); > goto out_put; > } > > > base-commit: a5e13c6df0e41702d2b2c77c8ad41677ebb065b3 >
[PATCH v1] ovl: Fix leaked dentry
From: Mickaël Salaün Since commit 6815f479ca90 ("ovl: use only uppermetacopy state in ovl_lookup()"), overlayfs doesn't put temporary dentry when there is a metacopy error, which leads to dentry leaks when shutting down the related superblock: overlayfs: refusing to follow metacopy origin for (/file0) ... BUG: Dentry (ptrval){i=3f33,n=file3} still in use (1) [unmount of overlay overlay] ... WARNING: CPU: 1 PID: 432 at umount_check.cold+0x107/0x14d CPU: 1 PID: 432 Comm: unmount-overlay Not tainted 5.12.0-rc5 #1 ... RIP: 0010:umount_check.cold+0x107/0x14d ... Call Trace: d_walk+0x28c/0x950 ? dentry_lru_isolate+0x2b0/0x2b0 ? __kasan_slab_free+0x12/0x20 do_one_tree+0x33/0x60 shrink_dcache_for_umount+0x78/0x1d0 generic_shutdown_super+0x70/0x440 kill_anon_super+0x3e/0x70 deactivate_locked_super+0xc4/0x160 deactivate_super+0xfa/0x140 cleanup_mnt+0x22e/0x370 __cleanup_mnt+0x1a/0x30 task_work_run+0x139/0x210 do_exit+0xb0c/0x2820 ? __kasan_check_read+0x1d/0x30 ? find_held_lock+0x35/0x160 ? lock_release+0x1b6/0x660 ? mm_update_next_owner+0xa20/0xa20 ? reacquire_held_locks+0x3f0/0x3f0 ? __sanitizer_cov_trace_const_cmp4+0x22/0x30 do_group_exit+0x135/0x380 __do_sys_exit_group.isra.0+0x20/0x20 __x64_sys_exit_group+0x3c/0x50 do_syscall_64+0x45/0x70 entry_SYSCALL_64_after_hwframe+0x44/0xae ... VFS: Busy inodes after unmount of overlay. Self-destruct in 5 seconds. Have a nice day... This fix has been tested with a syzkaller reproducer. Cc: Amir Goldstein Cc: Miklos Szeredi Cc: Vivek Goyal Cc: # v5.7+ Reported-by: syzbot Fixes: 6815f479ca90 ("ovl: use only uppermetacopy state in ovl_lookup()") Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20210329164907.2133175-1-...@digikod.net --- fs/overlayfs/namei.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 3fe05fb5d145..424c594afd79 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -921,6 +921,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, if ((uppermetacopy || d.metacopy) && !ofs->config.metacopy) { err = -EPERM; pr_warn_ratelimited("refusing to follow metacopy origin for (%pd2)\n", dentry); + dput(this); goto out_put; } base-commit: a5e13c6df0e41702d2b2c77c8ad41677ebb065b3 -- 2.30.2