xfs: fail if xattr inactivation hits a hole

The child buffer read in xfs_attr3_node_inactive() should never
reach a hole in the attr fork. If this occurs, it is likely due to a
bug. Prior to commit cd87d867 ("xfs: don't crash on unexpected holes
in dir/attr btrees"), this would result in a crash. Now that the
crash has been fixed, this is a silent failure.

Pass -1 to xfs_da3_node_read() from xfs_da3_node_inactive() to
indicate that reading from a hole is an error. This logs an error to
syslog and fails the inode inactivation, leaving the inode on the AG
unlinked list until removed by xfs_repair (or log recovery). Also
update the subsequent code to reflect that the read now returns a
non-NULL buffer or an error.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index e3a950e..52818ea 100644 (file)
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -251,47 +251,44 @@ xfs_attr3_node_inactive(
                 * traversal of the tree so we may deal with many blocks
                 * before we come back to this one.
                 */
-               error = xfs_da3_node_read(*trans, dp, child_fsb, -2, &child_bp,
-                                               XFS_ATTR_FORK);
+               error = xfs_da3_node_read(*trans, dp, child_fsb, -1, &child_bp,
+                                         XFS_ATTR_FORK);
                if (error)
                        return error;
-               if (child_bp) {
-                                               /* save for re-read later */
-                       child_blkno = XFS_BUF_ADDR(child_bp);
 
-                       /*
-                        * Invalidate the subtree, however we have to.
-                        */
-                       info = child_bp->b_addr;
-                       switch (info->magic) {
-                       case cpu_to_be16(XFS_DA_NODE_MAGIC):
-                       case cpu_to_be16(XFS_DA3_NODE_MAGIC):
-                               error = xfs_attr3_node_inactive(trans, dp,
-                                                       child_bp, level + 1);
-                               break;
-                       case cpu_to_be16(XFS_ATTR_LEAF_MAGIC):
-                       case cpu_to_be16(XFS_ATTR3_LEAF_MAGIC):
-                               error = xfs_attr3_leaf_inactive(trans, dp,
-                                                       child_bp);
-                               break;
-                       default:
-                               error = -EIO;
-                               xfs_trans_brelse(*trans, child_bp);
-                               break;
-                       }
-                       if (error)
-                               return error;
+               /* save for re-read later */
+               child_blkno = XFS_BUF_ADDR(child_bp);
 
-                       /*
-                        * Remove the subsidiary block from the cache
-                        * and from the log.
-                        */
-                       error = xfs_da_get_buf(*trans, dp, 0, child_blkno,
-                               &child_bp, XFS_ATTR_FORK);
-                       if (error)
-                               return error;
-                       xfs_trans_binval(*trans, child_bp);
+               /*
+                * Invalidate the subtree, however we have to.
+                */
+               info = child_bp->b_addr;
+               switch (info->magic) {
+               case cpu_to_be16(XFS_DA_NODE_MAGIC):
+               case cpu_to_be16(XFS_DA3_NODE_MAGIC):
+                       error = xfs_attr3_node_inactive(trans, dp, child_bp,
+                                                       level + 1);
+                       break;
+               case cpu_to_be16(XFS_ATTR_LEAF_MAGIC):
+               case cpu_to_be16(XFS_ATTR3_LEAF_MAGIC):
+                       error = xfs_attr3_leaf_inactive(trans, dp, child_bp);
+                       break;
+               default:
+                       error = -EIO;
+                       xfs_trans_brelse(*trans, child_bp);
+                       break;
                }
+               if (error)
+                       return error;
+
+               /*
+                * Remove the subsidiary block from the cache and from the log.
+                */
+               error = xfs_da_get_buf(*trans, dp, 0, child_blkno, &child_bp,
+                                      XFS_ATTR_FORK);
+               if (error)
+                       return error;
+               xfs_trans_binval(*trans, child_bp);
 
                /*
                 * If we're not done, re-read the parent to get the next