xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get
PATCH 1/1 xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get
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);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.
---
fs/xfs/libxfs/xfs_attr.c +2 -0
@@ -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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 12 lines · Eric Sandeen
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);
Quoted 8 lines
It (more or less) only shows up in the above scenario, because
xfs_metadumpobfuscates attr names, but chooses names which keep the same hash value - andxfs_da3_node_lookup_intdoes: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
Quoted 28 lines
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 aboutXFS_DA_OP_OKNOENT.Simply setting
XFS_DA_OP_OKNOENTinxfs_attr_getsolves 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 <[email protected]>---
fs/xfs/libxfs/xfs_attr.c +2
-0@@ -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))
Quoted 5 lines
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs