Re: [PATCH] xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Jul 22, 2015, at 11:59 PM, Eric Sandeen <sandeen@xxxxxxxxxx> wrote:
> 
> It's entirely possible for userspace to ask for an xattr which
> does not exist.
> 
> Normally, there is no problem whatsoever when we ask for such
> a thing, but when we look at an obfuscated metadump image
> on a debug kernel with selinux, we trip over this ASSERT in
> xfs_da3_path_shift():
> 
>        *result = -ENOENT;      /* we're out of our tree */
>        ASSERT(args->op_flags & XFS_DA_OP_OKNOENT);
> 
> It (more or less) only shows up in the above scenario, because
> xfs_metadump obfuscates attr names, but chooses names which
> keep the same hash value - and xfs_da3_node_lookup_int does:
> 
>        if (((retval == -ENOENT) || (retval == -ENOATTR)) &&
>            (blk->hashval == args->hashval)) {
>                error = xfs_da3_path_shift(state, &state->path, 1, 1,
>                                                 &retval);


Oh, and add: thanks to dchinner for spotting the above conditional that got us there ;)

-Eric

> IOWS, we only get down to the xfs_da3_path_shift() ASSERT
> if we are looking for an xattr which doesn't exist, but we
> find xattrs on disk which have the same hash, and so might be
> a hash collision, so we try the path shift.  When *that*
> fails to find what we're looking for, we hit the assert about
> XFS_DA_OP_OKNOENT.
> 
> Simply setting XFS_DA_OP_OKNOENT in xfs_attr_get solves this
> rather corner-case problem with no ill side effects.  It's
> fine for an attr name lookup to fail.
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 3349c9a..ff06557 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -139,6 +139,8 @@ xfs_attr_get(
> 
>    args.value = value;
>    args.valuelen = *valuelenp;
> +    /* Entirely possible to look up a name which doesn't exist */
> +    args.op_flags = XFS_DA_OP_OKNOENT;
> 
>    lock_mode = xfs_ilock_attr_map_shared(ip);
>    if (!xfs_inode_hasattr(ip))
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs



[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux