mirror of
https://git.proxmox.com/git/mirror_zfs.git
synced 2024-12-26 19:19:32 +03:00
Fix unprotected zfs_znode_dmu_fini
In original code, zfs_znode_dmu_fini is called in zfs_rmnode without zfs_znode_hold_enter. It seems to assume it's ok to do so when the znode is unlinked. However this assumption is not correct, as zfs_zget can be called by NFS through zpl_fh_to_dentry as pointed out by Christian in https://github.com/openzfs/zfs/pull/12767, which could result in a use-after-free bug. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Co-authored-by: Ryan Moeller <ryan@iXsystems.com> Signed-off-by: Chunwei Chen <david.chen@nutanix.com> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes #12767 Closes #14364
This commit is contained in:
parent
a379083d9f
commit
c6dab6dd39
@ -272,6 +272,8 @@ extern int zfs_freesp(znode_t *, uint64_t, uint64_t, int, boolean_t);
|
|||||||
extern void zfs_znode_init(void);
|
extern void zfs_znode_init(void);
|
||||||
extern void zfs_znode_fini(void);
|
extern void zfs_znode_fini(void);
|
||||||
extern int zfs_znode_hold_compare(const void *, const void *);
|
extern int zfs_znode_hold_compare(const void *, const void *);
|
||||||
|
extern znode_hold_t *zfs_znode_hold_enter(zfsvfs_t *, uint64_t);
|
||||||
|
extern void zfs_znode_hold_exit(zfsvfs_t *, znode_hold_t *);
|
||||||
extern int zfs_zget(zfsvfs_t *, uint64_t, znode_t **);
|
extern int zfs_zget(zfsvfs_t *, uint64_t, znode_t **);
|
||||||
extern int zfs_rezget(znode_t *);
|
extern int zfs_rezget(znode_t *);
|
||||||
extern void zfs_zinactive(znode_t *);
|
extern void zfs_zinactive(znode_t *);
|
||||||
|
@ -426,6 +426,7 @@ zfs_rmnode(znode_t *zp)
|
|||||||
zfsvfs_t *zfsvfs = zp->z_zfsvfs;
|
zfsvfs_t *zfsvfs = zp->z_zfsvfs;
|
||||||
objset_t *os = zfsvfs->z_os;
|
objset_t *os = zfsvfs->z_os;
|
||||||
dmu_tx_t *tx;
|
dmu_tx_t *tx;
|
||||||
|
uint64_t z_id = zp->z_id;
|
||||||
uint64_t acl_obj;
|
uint64_t acl_obj;
|
||||||
uint64_t xattr_obj;
|
uint64_t xattr_obj;
|
||||||
uint64_t count;
|
uint64_t count;
|
||||||
@ -445,8 +446,10 @@ zfs_rmnode(znode_t *zp)
|
|||||||
* Not enough space to delete some xattrs.
|
* Not enough space to delete some xattrs.
|
||||||
* Leave it in the unlinked set.
|
* Leave it in the unlinked set.
|
||||||
*/
|
*/
|
||||||
|
ZFS_OBJ_HOLD_ENTER(zfsvfs, z_id);
|
||||||
zfs_znode_dmu_fini(zp);
|
zfs_znode_dmu_fini(zp);
|
||||||
zfs_znode_free(zp);
|
zfs_znode_free(zp);
|
||||||
|
ZFS_OBJ_HOLD_EXIT(zfsvfs, z_id);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
@ -464,8 +467,10 @@ zfs_rmnode(znode_t *zp)
|
|||||||
* Not enough space or we were interrupted by unmount.
|
* Not enough space or we were interrupted by unmount.
|
||||||
* Leave the file in the unlinked set.
|
* Leave the file in the unlinked set.
|
||||||
*/
|
*/
|
||||||
|
ZFS_OBJ_HOLD_ENTER(zfsvfs, z_id);
|
||||||
zfs_znode_dmu_fini(zp);
|
zfs_znode_dmu_fini(zp);
|
||||||
zfs_znode_free(zp);
|
zfs_znode_free(zp);
|
||||||
|
ZFS_OBJ_HOLD_EXIT(zfsvfs, z_id);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -501,8 +506,10 @@ zfs_rmnode(znode_t *zp)
|
|||||||
* which point we'll call zfs_unlinked_drain() to process it).
|
* which point we'll call zfs_unlinked_drain() to process it).
|
||||||
*/
|
*/
|
||||||
dmu_tx_abort(tx);
|
dmu_tx_abort(tx);
|
||||||
|
ZFS_OBJ_HOLD_ENTER(zfsvfs, z_id);
|
||||||
zfs_znode_dmu_fini(zp);
|
zfs_znode_dmu_fini(zp);
|
||||||
zfs_znode_free(zp);
|
zfs_znode_free(zp);
|
||||||
|
ZFS_OBJ_HOLD_EXIT(zfsvfs, z_id);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -386,7 +386,6 @@ void
|
|||||||
zfs_znode_dmu_fini(znode_t *zp)
|
zfs_znode_dmu_fini(znode_t *zp)
|
||||||
{
|
{
|
||||||
ASSERT(MUTEX_HELD(ZFS_OBJ_MUTEX(zp->z_zfsvfs, zp->z_id)) ||
|
ASSERT(MUTEX_HELD(ZFS_OBJ_MUTEX(zp->z_zfsvfs, zp->z_id)) ||
|
||||||
zp->z_unlinked ||
|
|
||||||
ZFS_TEARDOWN_INACTIVE_WRITE_HELD(zp->z_zfsvfs));
|
ZFS_TEARDOWN_INACTIVE_WRITE_HELD(zp->z_zfsvfs));
|
||||||
|
|
||||||
sa_handle_destroy(zp->z_sa_hdl);
|
sa_handle_destroy(zp->z_sa_hdl);
|
||||||
|
@ -649,6 +649,8 @@ zfs_rmnode(znode_t *zp)
|
|||||||
objset_t *os = zfsvfs->z_os;
|
objset_t *os = zfsvfs->z_os;
|
||||||
znode_t *xzp = NULL;
|
znode_t *xzp = NULL;
|
||||||
dmu_tx_t *tx;
|
dmu_tx_t *tx;
|
||||||
|
znode_hold_t *zh;
|
||||||
|
uint64_t z_id = zp->z_id;
|
||||||
uint64_t acl_obj;
|
uint64_t acl_obj;
|
||||||
uint64_t xattr_obj;
|
uint64_t xattr_obj;
|
||||||
uint64_t links;
|
uint64_t links;
|
||||||
@ -666,8 +668,9 @@ zfs_rmnode(znode_t *zp)
|
|||||||
* Not enough space to delete some xattrs.
|
* Not enough space to delete some xattrs.
|
||||||
* Leave it in the unlinked set.
|
* Leave it in the unlinked set.
|
||||||
*/
|
*/
|
||||||
|
zh = zfs_znode_hold_enter(zfsvfs, z_id);
|
||||||
zfs_znode_dmu_fini(zp);
|
zfs_znode_dmu_fini(zp);
|
||||||
|
zfs_znode_hold_exit(zfsvfs, zh);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -686,7 +689,9 @@ zfs_rmnode(znode_t *zp)
|
|||||||
* Not enough space or we were interrupted by unmount.
|
* Not enough space or we were interrupted by unmount.
|
||||||
* Leave the file in the unlinked set.
|
* Leave the file in the unlinked set.
|
||||||
*/
|
*/
|
||||||
|
zh = zfs_znode_hold_enter(zfsvfs, z_id);
|
||||||
zfs_znode_dmu_fini(zp);
|
zfs_znode_dmu_fini(zp);
|
||||||
|
zfs_znode_hold_exit(zfsvfs, zh);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -726,7 +731,9 @@ zfs_rmnode(znode_t *zp)
|
|||||||
* which point we'll call zfs_unlinked_drain() to process it).
|
* which point we'll call zfs_unlinked_drain() to process it).
|
||||||
*/
|
*/
|
||||||
dmu_tx_abort(tx);
|
dmu_tx_abort(tx);
|
||||||
|
zh = zfs_znode_hold_enter(zfsvfs, z_id);
|
||||||
zfs_znode_dmu_fini(zp);
|
zfs_znode_dmu_fini(zp);
|
||||||
|
zfs_znode_hold_exit(zfsvfs, zh);
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -271,7 +271,7 @@ zfs_znode_held(zfsvfs_t *zfsvfs, uint64_t obj)
|
|||||||
return (held);
|
return (held);
|
||||||
}
|
}
|
||||||
|
|
||||||
static znode_hold_t *
|
znode_hold_t *
|
||||||
zfs_znode_hold_enter(zfsvfs_t *zfsvfs, uint64_t obj)
|
zfs_znode_hold_enter(zfsvfs_t *zfsvfs, uint64_t obj)
|
||||||
{
|
{
|
||||||
znode_hold_t *zh, *zh_new, search;
|
znode_hold_t *zh, *zh_new, search;
|
||||||
@ -304,7 +304,7 @@ zfs_znode_hold_enter(zfsvfs_t *zfsvfs, uint64_t obj)
|
|||||||
return (zh);
|
return (zh);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void
|
void
|
||||||
zfs_znode_hold_exit(zfsvfs_t *zfsvfs, znode_hold_t *zh)
|
zfs_znode_hold_exit(zfsvfs_t *zfsvfs, znode_hold_t *zh)
|
||||||
{
|
{
|
||||||
int i = ZFS_OBJ_HASH(zfsvfs, zh->zh_obj);
|
int i = ZFS_OBJ_HASH(zfsvfs, zh->zh_obj);
|
||||||
@ -357,7 +357,7 @@ zfs_znode_sa_init(zfsvfs_t *zfsvfs, znode_t *zp,
|
|||||||
void
|
void
|
||||||
zfs_znode_dmu_fini(znode_t *zp)
|
zfs_znode_dmu_fini(znode_t *zp)
|
||||||
{
|
{
|
||||||
ASSERT(zfs_znode_held(ZTOZSB(zp), zp->z_id) || zp->z_unlinked ||
|
ASSERT(zfs_znode_held(ZTOZSB(zp), zp->z_id) ||
|
||||||
RW_WRITE_HELD(&ZTOZSB(zp)->z_teardown_inactive_lock));
|
RW_WRITE_HELD(&ZTOZSB(zp)->z_teardown_inactive_lock));
|
||||||
|
|
||||||
sa_handle_destroy(zp->z_sa_hdl);
|
sa_handle_destroy(zp->z_sa_hdl);
|
||||||
|
Loading…
Reference in New Issue
Block a user