From 3b6dd9a9aeeada19d0c820ff68e979243a888bb6 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 9 Apr 2021 10:27:34 -0700 Subject: xfs: fix return of uninitialized value in variable error A previous commit removed a call to xfs_attr3_leaf_read that assigned an error return code to variable error. We now have a few early error return paths to label 'out' that return error if error is set; however error now is uninitialized so potentially garbage is being returned. Fix this by setting error to zero to restore the original behaviour where error was zero at the label 'restart'. Addresses-Coverity: ("Uninitialized scalar variable") Fixes: 07120f1abdff ("xfs: Add xfs_has_attr and subroutines") Signed-off-by: Colin Ian King Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_attr.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/xfs/libxfs/xfs_attr.c') diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 472b3039eabb..902e5f7e6642 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -928,6 +928,7 @@ restart: * Search to see if name already exists, and get back a pointer * to where it should go. */ + error = 0; retval = xfs_attr_node_hasname(args, &state); if (retval != -ENOATTR && retval != -EEXIST) goto out; -- cgit v1.2.3 From 2ac131df03d4f06bb0d825335663cc5064421993 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Apr 2021 11:15:10 -0700 Subject: xfs: rename and simplify xfs_bmap_one_block xfs_bmap_one_block is only called for the attribute fork. Move it to xfs_attr.c, drop the unused whichfork argument and code only executed for the data fork and rename the result to xfs_attr_is_leaf. Signed-off-by: Christoph Hellwig Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_attr.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) (limited to 'fs/xfs/libxfs/xfs_attr.c') diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 902e5f7e6642..fd61c67f5739 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -70,6 +70,26 @@ xfs_inode_hasattr( return 1; } +/* + * Returns true if the there is exactly only block in the attr fork, in which + * case the attribute fork consists of a single leaf block entry. + */ +bool +xfs_attr_is_leaf( + struct xfs_inode *ip) +{ + struct xfs_ifork *ifp = ip->i_afp; + struct xfs_iext_cursor icur; + struct xfs_bmbt_irec imap; + + if (ifp->if_nextents != 1 || ifp->if_format != XFS_DINODE_FMT_EXTENTS) + return false; + + xfs_iext_first(ifp, &icur); + xfs_iext_get_extent(ifp, &icur, &imap); + return imap.br_startoff == 0 && imap.br_blockcount == 1; +} + /*======================================================================== * Overall external interface routines. *========================================================================*/ @@ -89,7 +109,7 @@ xfs_attr_get_ilocked( if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) return xfs_attr_shortform_getvalue(args); - if (xfs_bmap_one_block(args->dp, XFS_ATTR_FORK)) + if (xfs_attr_is_leaf(args->dp)) return xfs_attr_leaf_get(args); return xfs_attr_node_get(args); } @@ -293,7 +313,7 @@ xfs_attr_set_args( return error; } - if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) { + if (xfs_attr_is_leaf(dp)) { error = xfs_attr_leaf_addname(args); if (error != -ENOSPC) return error; @@ -347,7 +367,7 @@ xfs_has_attr( return xfs_attr_sf_findname(args, NULL, NULL); } - if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) { + if (xfs_attr_is_leaf(dp)) { error = xfs_attr_leaf_hasname(args, &bp); if (bp) @@ -374,7 +394,7 @@ xfs_attr_remove_args( } else if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) { ASSERT(dp->i_afp->if_flags & XFS_IFINLINE); error = xfs_attr_shortform_remove(args); - } else if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) { + } else if (xfs_attr_is_leaf(dp)) { error = xfs_attr_leaf_removename(args); } else { error = xfs_attr_node_removename(args); @@ -1283,7 +1303,7 @@ xfs_attr_node_removename( /* * If the result is small enough, push it all into the inode. */ - if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) + if (xfs_attr_is_leaf(dp)) error = xfs_attr_node_shrink(args, state); out: -- cgit v1.2.3 From 605e74e29218bb22edd5ddcf90a4d37df00446cc Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Apr 2021 11:15:10 -0700 Subject: xfs: simplify xfs_attr_remove_args Directly return from the subfunctions and avoid the error variable. Also remove the not really needed dp local variable. Signed-off-by: Christoph Hellwig Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_attr.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'fs/xfs/libxfs/xfs_attr.c') diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index fd61c67f5739..43ef85678cba 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -386,21 +386,16 @@ int xfs_attr_remove_args( struct xfs_da_args *args) { - struct xfs_inode *dp = args->dp; - int error; + if (!xfs_inode_hasattr(args->dp)) + return -ENOATTR; - if (!xfs_inode_hasattr(dp)) { - error = -ENOATTR; - } else if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) { - ASSERT(dp->i_afp->if_flags & XFS_IFINLINE); - error = xfs_attr_shortform_remove(args); - } else if (xfs_attr_is_leaf(dp)) { - error = xfs_attr_leaf_removename(args); - } else { - error = xfs_attr_node_removename(args); + if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) { + ASSERT(args->dp->i_afp->if_flags & XFS_IFINLINE); + return xfs_attr_shortform_remove(args); } - - return error; + if (xfs_attr_is_leaf(args->dp)) + return xfs_attr_leaf_removename(args); + return xfs_attr_node_removename(args); } /* -- cgit v1.2.3 From 0779f4a68d4df539a7ea624f7e1560f48aa46ad9 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Apr 2021 11:15:11 -0700 Subject: xfs: remove XFS_IFINLINE Just check for an inline format fork instead of the using the equivalent in-memory XFS_IFINLINE flag. Signed-off-by: Christoph Hellwig Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_attr.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'fs/xfs/libxfs/xfs_attr.c') diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 43ef85678cba..96146f425e50 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -362,10 +362,8 @@ xfs_has_attr( if (!xfs_inode_hasattr(dp)) return -ENOATTR; - if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) { - ASSERT(dp->i_afp->if_flags & XFS_IFINLINE); + if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) return xfs_attr_sf_findname(args, NULL, NULL); - } if (xfs_attr_is_leaf(dp)) { error = xfs_attr_leaf_hasname(args, &bp); @@ -389,10 +387,8 @@ xfs_attr_remove_args( if (!xfs_inode_hasattr(args->dp)) return -ENOATTR; - if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) { - ASSERT(args->dp->i_afp->if_flags & XFS_IFINLINE); + if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) return xfs_attr_shortform_remove(args); - } if (xfs_attr_is_leaf(args->dp)) return xfs_attr_leaf_removename(args); return xfs_attr_node_removename(args); -- cgit v1.2.3