Commit ef80de94 authored by Darrick J. Wong's avatar Darrick J. Wong
Browse files

xfs: attr fork iext must be loaded before calling xfs_attr_is_leaf 



Christoph noticed that the xfs_attr_is_leaf in xfs_attr_get_ilocked can
access the incore extent tree of the attr fork, but nothing in the
xfs_attr_get path guarantees that the incore tree is actually loaded.

Most of the time it is, but seeing as xfs_attr_is_leaf ignores the
return value of xfs_iext_get_extent I guess we've been making choices
based on random stack contents and nobody's complained?

Reported-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarDarrick J. Wong <djwong@kernel.org>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
parent cda60317

fs/xfs/libxfs/xfs_attr.c

+17 −0
Original line number Diff line number Diff line
@@ -87,6 +87,8 @@ xfs_attr_is_leaf( 
	struct xfs_iext_cursor	icur;

	struct xfs_bmbt_irec	imap;



	ASSERT(!xfs_need_iread_extents(ifp));



	if (ifp->if_nextents != 1 || ifp->if_format != XFS_DINODE_FMT_EXTENTS)

		return false;
@@ -224,11 +226,21 @@ int 
xfs_attr_get_ilocked(

	struct xfs_da_args	*args)

{

	int			error;



	xfs_assert_ilocked(args->dp, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL);



	if (!xfs_inode_hasattr(args->dp))

		return -ENOATTR;



	/*

	 * The incore attr fork iext tree must be loaded for xfs_attr_is_leaf

	 * to work correctly.

	 */

	error = xfs_iread_extents(args->trans, args->dp, XFS_ATTR_FORK);

	if (error)

		return error;



	if (args->dp->i_af.if_format == XFS_DINODE_FMT_LOCAL)

		return xfs_attr_shortform_getvalue(args);

	if (xfs_attr_is_leaf(args->dp))
@@ -870,6 +882,11 @@ xfs_attr_lookup( 
		return -ENOATTR;

	}



	/* Prerequisite for xfs_attr_is_leaf */

	error = xfs_iread_extents(args->trans, args->dp, XFS_ATTR_FORK);

	if (error)

		return error;



	if (xfs_attr_is_leaf(dp)) {

		error = xfs_attr_leaf_hasname(args, &bp);

fs/xfs/xfs_attr_item.c

+36 −6
Original line number Diff line number Diff line
@@ -498,6 +498,25 @@ xfs_attri_validate( 
	return xfs_verify_ino(mp, attrp->alfi_ino);

}



static int

xfs_attri_iread_extents(

	struct xfs_inode		*ip)

{

	struct xfs_trans		*tp;

	int				error;



	error = xfs_trans_alloc_empty(ip->i_mount, &tp);

	if (error)

		return error;



	xfs_ilock(ip, XFS_ILOCK_EXCL);

	error = xfs_iread_extents(tp, ip, XFS_ATTR_FORK);

	xfs_iunlock(ip, XFS_ILOCK_EXCL);

	xfs_trans_cancel(tp);



	return error;

}



static inline struct xfs_attr_intent *

xfs_attri_recover_work(

	struct xfs_mount		*mp,
@@ -508,13 +527,22 @@ xfs_attri_recover_work( 
{

	struct xfs_attr_intent		*attr;

	struct xfs_da_args		*args;

	struct xfs_inode		*ip;

	int				local;

	int				error;



	error = xlog_recover_iget(mp,  attrp->alfi_ino, ipp);

	error = xlog_recover_iget(mp,  attrp->alfi_ino, &ip);

	if (error)

		return ERR_PTR(error);



	if (xfs_inode_has_attr_fork(ip)) {

		error = xfs_attri_iread_extents(ip);

		if (error) {

			xfs_irele(ip);

			return ERR_PTR(error);

		}

	}



	attr = kzalloc(sizeof(struct xfs_attr_intent) +

			sizeof(struct xfs_da_args), GFP_KERNEL | __GFP_NOFAIL);

	args = (struct xfs_da_args *)(attr + 1);
@@ -531,7 +559,7 @@ xfs_attri_recover_work( 
	attr->xattri_nameval = xfs_attri_log_nameval_get(nv);

	ASSERT(attr->xattri_nameval);



	args->dp = *ipp;

	args->dp = ip;

	args->geo = mp->m_attr_geo;

	args->whichfork = XFS_ATTR_FORK;

	args->name = nv->name.i_addr;
@@ -561,6 +589,7 @@ xfs_attri_recover_work( 
	}



	xfs_defer_add_item(dfp, &attr->xattri_list);

	*ipp = ip;

	return attr;

}
@@ -615,16 +644,17 @@ xfs_attr_recover_work( 
		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,

				&attrip->attri_format,

				sizeof(attrip->attri_format));

	if (error) {

		xfs_trans_cancel(tp);

		goto out_unlock;

	}

	if (error)

		goto out_cancel;



	error = xfs_defer_ops_capture_and_commit(tp, capture_list);

out_unlock:

	xfs_iunlock(ip, XFS_ILOCK_EXCL);

	xfs_irele(ip);

	return error;

out_cancel:

	xfs_trans_cancel(tp);

	goto out_unlock;

}



/* Re-log an intent item to push the log tail forward. */

fs/xfs/xfs_attr_list.c

+7 −0
Original line number Diff line number Diff line
@@ -544,6 +544,7 @@ xfs_attr_list_ilocked( 
	struct xfs_attr_list_context	*context)

{

	struct xfs_inode		*dp = context->dp;

	int				error;



	xfs_assert_ilocked(dp, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL);
@@ -554,6 +555,12 @@ xfs_attr_list_ilocked( 
		return 0;

	if (dp->i_af.if_format == XFS_DINODE_FMT_LOCAL)

		return xfs_attr_shortform_list(context);



	/* Prerequisite for xfs_attr_is_leaf */

	error = xfs_iread_extents(NULL, dp, XFS_ATTR_FORK);

	if (error)

		return error;



	if (xfs_attr_is_leaf(dp))

		return xfs_attr_leaf_list(context);

	return xfs_attr_node_list(context);