Re: [PATCH 1/6] xfs: remove more ondisk directory corruption asserts

Brian Foster
2019-07-05T14:49:07Z

On Wed, Jun 26, 2019 at 01:46:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Continue our game of replacing ASSERTs for corrupt ondisk metadata with
> EFSCORRUPTED returns.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_da_btree.c | 19 ++++++++++++-------
> fs/xfs/libxfs/xfs_dir2_node.c | 3 ++-
> 2 files changed, 14 insertions(+), 8 deletions(-)
>
>
...
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index 16731d2d684b..f7f3fb458019 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -743,7 +743,8 @@ xfs_dir2_leafn_lookup_for_entry(
> ents = dp->d_ops->leaf_ents_p(leaf);
>
> xfs_dir3_leaf_check(dp, bp);
> - ASSERT(leafhdr.count > 0);
> + if (leafhdr.count <= 0)
> + return -EFSCORRUPTED;

This error return bubbles up to xfs_dir2_leafn_lookup_int() and
xfs_da3_node_lookup_int(). The latter has a direct return value as well
as a *result return parameter, which unconditionally carries the return
value from xfs_dir2_leafn_lookup_int(). xfs_da3_node_lookup_int() has
multiple callers, but a quick look at one (xfs_attr_node_addname())
suggests we might not handle corruption errors properly via the *result
parameter. Perhaps we also need to fix up xfs_da3_node_lookup_int() to
return particular errors directly?

Brian

>
> /*
> * Look up the hash value in the leaf entries.
>