xfs_repair: fix get_agino_buf to avoid corrupting inodes

The inode buffering code tries to read inodes in units of chunks,
which are the larger of 8K or 1 FSB.  Each chunk gets its own xfs_buf,
which means that get_agino_buf must calculate the disk address of the
chunk and feed that to libxfs_readbuf in order to find the inode data
correctly.  The current code simply grabs the chunk for the start
inode and indexes from that, which corrupts memory because the start
inode and the target inode could be in different inode chunks.  That
causes the assert in rmap.c to blow when we clear the reflink flag.

(Also fix some minor errors in the debugging printfs.)

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 8b22eb402b6b2f8559b5d014de9118265d288204..0fea9cb2bb72cfe5f6d7044a1b8a86364f70fe1c 100644 (file)
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1038,9 +1038,9 @@ libxfs_readbufr_map(struct xfs_buftarg *btp, struct xfs_buf *bp, int flags)
        if (!error)
                bp->b_flags |= LIBXFS_B_UPTODATE;
 #ifdef IO_DEBUG
-       printf("%lx: %s: read %u bytes, error %d, blkno=0x%llx(0x%llx), %p\n",
-               pthread_self(), __FUNCTION__, , error,
-               (long long)LIBXFS_BBTOOFF64(blkno), (long long)blkno, bp);
+       printf("%lx: %s: read %lu bytes, error %d, blkno=%llu(%llu), %p\n",
+               pthread_self(), __FUNCTION__, buf - (char *)bp->b_addr, error,
+               (long long)LIBXFS_BBTOOFF64(bp->b_bn), (long long)bp->b_bn, bp);
 #endif
        return error;
 }
@@ -1070,7 +1070,7 @@ libxfs_readbuf_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
        if (!error)
                libxfs_readbuf_verify(bp, ops);
 
-#ifdef IO_DEBUG
+#ifdef IO_DEBUGX
        printf("%lx: %s: read %lu bytes, error %d, blkno=%llu(%llu), %p\n",
                pthread_self(), __FUNCTION__, buf - (char *)bp->b_addr, error,
                (long long)LIBXFS_BBTOOFF64(bp->b_bn), (long long)bp->b_bn, bp);

diff --git a/repair/dinode.c b/repair/dinode.c
index 512a6687a6500ced5c329660b806f52c99ccc644..16e0a0637fd732c549fa15ba0f0dfc3cb8df3bb7 100644 (file)
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -847,43 +847,58 @@ scan_bmbt_reclist(
 }
 
 /*
- * these two are meant for routines that read and work with inodes
- * one at a time where the inodes may be in any order (like walking
- * the unlinked lists to look for inodes).  the caller is responsible
- * for writing/releasing the buffer.
+ * Grab the buffer backing an inode.  This is meant for routines that
+ * work with inodes one at a time in any order (like walking the
+ * unlinked lists to look for inodes).  The caller is responsible for
+ * writing/releasing the buffer.
  */
-xfs_buf_t *
-get_agino_buf(xfs_mount_t       *mp,
-               xfs_agnumber_t  agno,
-               xfs_agino_t     agino,
-               xfs_dinode_t    **dipp)
+struct xfs_buf *
+get_agino_buf(
+       struct xfs_mount        *mp,
+       xfs_agnumber_t          agno,
+       xfs_agino_t             agino,
+       struct xfs_dinode       **dipp)
 {
-       ino_tree_node_t *irec;
-       xfs_buf_t *bp;
-       int size;
-
-       if ((irec = find_inode_rec(mp, agno, agino)) == NULL)
-               return(NULL);
+       struct xfs_buf          *bp;
+       int                     cluster_size;
+       int                     ino_per_cluster;
+       xfs_agino_t             cluster_agino;
+       xfs_daddr_t             cluster_daddr;
+       xfs_daddr_t             cluster_blks;
 
-       size = MAX(1, XFS_FSB_TO_BB(mp,
+       /*
+        * Inode buffers have been read into memory in inode_cluster_size
+        * chunks (or one FSB).  To find the correct buffer for an inode,
+        * we must find the buffer for its cluster, add the appropriate
+        * offset, and return that.
+        */
+       cluster_size = MAX(mp->m_inode_cluster_size, mp->m_sb.sb_blocksize);
+       ino_per_cluster = cluster_size / mp->m_sb.sb_inodesize;
+       cluster_agino = agino & ~(ino_per_cluster - 1);
+       cluster_blks = XFS_FSB_TO_DADDR(mp, MAX(1,
                        mp->m_inode_cluster_size >> mp->m_sb.sb_blocklog));
-       bp = libxfs_readbuf(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno,
-               XFS_AGINO_TO_AGBNO(mp, irec->ino_startnum)), size, 0,
-               &xfs_inode_buf_ops);
+       cluster_daddr = XFS_AGB_TO_DADDR(mp, agno,
+                       XFS_AGINO_TO_AGBNO(mp, cluster_agino));
+
+#ifdef XR_INODE_TRACE
+       printf("cluster_size %d ipc %d clusagino %d daddr %lld sectors %lld\n",
+               cluster_size, ino_per_cluster, cluster_agino, cluster_daddr,
+               cluster_blks);
+#endif
+
+       bp = libxfs_readbuf(mp->m_dev, cluster_daddr, cluster_blks,
+                       0, &xfs_inode_buf_ops);
        if (!bp) {
                do_warn(_("cannot read inode (%u/%u), disk block %" PRIu64 "\n"),
-                       agno, irec->ino_startnum,
-                       XFS_AGB_TO_DADDR(mp, agno,
-                               XFS_AGINO_TO_AGBNO(mp, irec->ino_startnum)));
-               return(NULL);
+                       agno, cluster_agino, cluster_daddr);
+               return NULL;
        }
 
-       *dipp = xfs_make_iptr(mp, bp, agino -
-               XFS_OFFBNO_TO_AGINO(mp, XFS_AGINO_TO_AGBNO(mp,
-                                               irec->ino_startnum),
-               0));
-
-       return(bp);
+       *dipp = xfs_make_iptr(mp, bp, agino - cluster_agino);
+       ASSERT(!xfs_sb_version_hascrc(&mp->m_sb) ||
+                       XFS_AGINO_TO_INO(mp, agno, agino) ==
+                       be64_to_cpu((*dipp)->di_ino));
+       return bp;
 }
 
 /*

diff --git a/repair/dinode.h b/repair/dinode.h
index 5aebf5b3863832e3d8fb900c3eaff473b6e6190a..61d073630bd43ddb6e59284ceb6b363a51649bc1 100644 (file)
--- a/repair/dinode.h
+++ b/repair/dinode.h
@@ -113,12 +113,12 @@ void
 check_uncertain_aginodes(xfs_mount_t   *mp,
                        xfs_agnumber_t  agno);
 
-xfs_buf_t *
-get_agino_buf(xfs_mount_t      *mp,
-               xfs_agnumber_t  agno,
-               xfs_agino_t     agino,
-               xfs_dinode_t    **dipp);
-
+struct xfs_buf *
+get_agino_buf(
+       struct xfs_mount        *mp,
+       xfs_agnumber_t          agno,
+       xfs_agino_t             agino,
+       struct xfs_dinode       **dipp);
 
 void dinode_bmbt_translation_init(void);
 char * get_forkname(int whichfork);