Skip to content

Commit

Permalink
Linux: always check or verify return of igrab()
Browse files Browse the repository at this point in the history
zhold() wraps igrab() on Linux, and igrab() may fail when the inode 
is in the process of being deleted.  This means zhold() must only be
called when a reference exists and therefore it cannot be deleted. 
This is the case for all existing consumers so add a VERIFY and a
comment explaining this requirement.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Adam Moss <c@yotes.com>
Closes openzfs#11704
  • Loading branch information
adamdmoss authored and RageLtMan committed May 31, 2021
1 parent c07251a commit 6603d90
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 4 deletions.
8 changes: 7 additions & 1 deletion include/os/linux/zfs/sys/zfs_znode_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ extern "C" {
#define zn_has_cached_data(zp) ((zp)->z_is_mapped)
#define zn_rlimit_fsize(zp, uio) (0)

#define zhold(zp) igrab(ZTOI((zp)))
/*
* zhold() wraps igrab() on Linux, and igrab() may fail when the
* inode is in the process of being deleted. As zhold() must only be
* called when a ref already exists - so the inode cannot be
* mid-deletion - we VERIFY() this.
*/
#define zhold(zp) VERIFY3P(igrab(ZTOI((zp))), !=, NULL)
#define zrele(zp) iput(ZTOI((zp)))

/* Called on entry to each ZFS inode and vfs operation. */
Expand Down
3 changes: 2 additions & 1 deletion module/os/linux/zfs/zfs_ctldir.c
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,8 @@ struct inode *
zfsctl_root(znode_t *zp)
{
ASSERT(zfs_has_ctldir(zp));
igrab(ZTOZSB(zp)->z_ctldir);
/* Must have an existing ref, so igrab() cannot return NULL */
VERIFY3P(igrab(ZTOZSB(zp)->z_ctldir), !=, NULL);
return (ZTOZSB(zp)->z_ctldir);
}

Expand Down
6 changes: 5 additions & 1 deletion module/os/linux/zfs/zfs_vfsops.c
Original file line number Diff line number Diff line change
Expand Up @@ -1734,7 +1734,11 @@ zfs_vget(struct super_block *sb, struct inode **ipp, fid_t *fidp)
VERIFY(zfsctl_root_lookup(*ipp, "snapshot", ipp,
0, kcred, NULL, NULL) == 0);
} else {
igrab(*ipp);
/*
* Must have an existing ref, so igrab()
* cannot return NULL
*/
VERIFY3P(igrab(*ipp), !=, NULL);
}
ZFS_EXIT(zfsvfs);
return (0);
Expand Down
3 changes: 2 additions & 1 deletion module/os/linux/zfs/zpl_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,8 @@ zpl_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)

crhold(cr);
ip->i_ctime = current_time(ip);
igrab(ip); /* Use ihold() if available */
/* Must have an existing ref, so igrab() cannot return NULL */
VERIFY3P(igrab(ip), !=, NULL);

cookie = spl_fstrans_mark();
error = -zfs_link(ITOZ(dir), ITOZ(ip), dname(dentry), cr, 0);
Expand Down

0 comments on commit 6603d90

Please sign in to comment.