c/mirror_zfs
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_zfs.git synced 2024-11-17 10:01:01 +03:00
Code Issues Packages Projects Releases Wiki Activity

zfs_domount: fix double-disown of dataset / double-free of zfsvfs_t

Browse Source 
Before this patch, in zfs_domount, if zfs_root or d_make_root fails, we
leave zfsvfs != NULL. This will lead to execution of the error handling
`if` statement at the `out` label, and hence to a call to
dmu_objset_disown and zfsvfs_free.

However, zfs_umount, which we call upon failure of zfs_root and
d_make_root already does dmu_objset_disown and zfsvfs_free.

I suppose this patch rather adds to the brittleness of this part of the
code base, but I don't want to invest more time in this right now.
To add a regression test, we'd need some kind of fault injection
facility for zfs_root or d_make_root, which doesn't exist right now.
And even then, I think that regression test would be too closely tied
to the implementation.

To repro the double-disown / double-free, do the following:
1. patch zfs_root to always return an error
2. mount a ZFS filesystem

Here's the stack trace you would see then:

  VERIFY3(ds->ds_owner == tag) failed (0000000000000000 == ffff9142361e8000)
  PANIC at dsl_dataset.c:1003:dsl_dataset_disown()
  Showing stack for process 28332
  CPU: 2 PID: 28332 Comm: zpool Tainted: G           O      5.10.103-1.nutanix.el7.x86_64 #1
  Call Trace:
   dump_stack+0x74/0x92
   spl_dumpstack+0x29/0x2b [spl]
   spl_panic+0xd4/0xfc [spl]
   dsl_dataset_disown+0xe9/0x150 [zfs]
   dmu_objset_disown+0xd6/0x150 [zfs]
   zfs_domount+0x17b/0x4b0 [zfs]
   zpl_mount+0x174/0x220 [zfs]
   legacy_get_tree+0x2b/0x50
   vfs_get_tree+0x2a/0xc0
   path_mount+0x2fa/0xa70
   do_mount+0x7c/0xa0
   __x64_sys_mount+0x8b/0xe0
   do_syscall_64+0x38/0x50
   entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Co-authored-by: Christian Schwarz <christian.schwarz@nutanix.com>
Signed-off-by: Christian Schwarz <christian.schwarz@nutanix.com>
Closes #14025
This commit is contained in:
Christian Schwarz 2022-10-14 20:46:47 +02:00 committed by GitHub
parent 642c2dee44
commit 4d5aef3ba9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 2 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
module/os/linux/zfs/zfs_vfsops.c
View File

@ -1556,6 +1556,7 @@ zfs_domount(struct super_block *sb, zfs_mnt_t *zm, int silent)
error = zfs_root(zfsvfs, &root_inode); error = zfs_root(zfsvfs, &root_inode);
if (error) { if (error) {
(void) zfs_umount(sb); (void) zfs_umount(sb);
zfsvfs = NULL; /* avoid double-free; first in zfs_umount */
goto out; goto out;
} }
@ -1563,6 +1564,7 @@ zfs_domount(struct super_block *sb, zfs_mnt_t *zm, int silent)
sb->s_root = d_make_root(root_inode); sb->s_root = d_make_root(root_inode);
if (sb->s_root == NULL) { if (sb->s_root == NULL) {
(void) zfs_umount(sb); (void) zfs_umount(sb);
zfsvfs = NULL; /* avoid double-free; first in zfs_umount */
error = SET_ERROR(ENOMEM); error = SET_ERROR(ENOMEM);
goto out; goto out;
} }