xfs
[Top] [All Lists]

[PATCH 1/2] metadump: bounds check btree block regions being zeroed

To: xfs@xxxxxxxxxxx
Subject: [PATCH 1/2] metadump: bounds check btree block regions being zeroed
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 14 Dec 2015 12:01:05 +1100
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1450054866-32720-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1450054866-32720-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

Arkadiusz Miskiewicz reported that metadump was crashing on one of
his corrupted filesystems, and the trace indicated that it was
zeroing unused regions in inode btree blocks when it failed. The
btree block had a corrupt nrecs field, which was resulting in an out
of bounds memset() occurring.

Ensure that the region being generated for zeroing is within bounds
before executing the zeroing. While there, abstract the repeated
boiler plate code so that it is simpler to maintain and extend the
zeroing code to new types of btrees in future.

Reported-by: Arkadiusz Miskiewicz <arekm@xxxxxxxx>
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 db/metadump.c | 93 +++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 56 insertions(+), 37 deletions(-)

diff --git a/db/metadump.c b/db/metadump.c
index 8cdcb92..cc5d1c7 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -246,6 +246,11 @@ write_buf(
        return seenint() ? -EINTR : 0;
 }
 
+/*
+ * We could be processing a corrupt block, so we can't trust any of
+ * the offsets or lengths to be within the buffer range. Hence check
+ * carefully!
+ */
 static void
 zero_btree_node(
        struct xfs_btree_block  *block,
@@ -258,55 +263,64 @@ zero_btree_node(
        xfs_inobt_key_t         *ikp;
        xfs_alloc_ptr_t         *app;
        xfs_alloc_key_t         *akp;
-       void                    *zp1, *zp2;
-       int                     zlen1, zlen2;
+       char                    *zp1, *zp2;
+       char                    *key_end;
 
        nrecs = be16_to_cpu(block->bb_numrecs);
+       if (nrecs < 0)
+               return;
 
        switch (btype) {
        case TYP_BMAPBTA:
        case TYP_BMAPBTD:
+               if (nrecs > mp->m_bmap_dmxr[1])
+                       return;
+
                bkp = XFS_BMBT_KEY_ADDR(mp, block, 1);
                bpp = XFS_BMBT_PTR_ADDR(mp, block, 1, mp->m_bmap_dmxr[1]);
-               zp1 = &bkp[nrecs];
-               zlen1 = (char *)&bpp[0] - (char *)&bkp[nrecs];
-               zp2 = &bpp[nrecs];
-               zlen2 = (char *)block + mp->m_sb.sb_blocksize -
-                                                       (char *)&bpp[nrecs];
+               zp1 = (char *)&bkp[nrecs];
+               zp2 = (char *)&bpp[nrecs];
+               key_end = (char *)bpp;
                break;
        case TYP_INOBT:
        case TYP_FINOBT:
+               if (nrecs > mp->m_inobt_mxr[1])
+                       return;
+
                ikp = XFS_INOBT_KEY_ADDR(mp, block, 1);
                ipp = XFS_INOBT_PTR_ADDR(mp, block, 1, mp->m_inobt_mxr[1]);
-               zp1 = &ikp[nrecs];
-               zlen1 = (char *)&ipp[0] - (char *)&ikp[nrecs];
-               zp2 = &ipp[nrecs];
-               zlen2 = (char *)block + mp->m_sb.sb_blocksize -
-                                                       (char *)&ipp[nrecs];
+               zp1 = (char *)&ikp[nrecs];
+               zp2 = (char *)&ipp[nrecs];
+               key_end = (char *)ipp;
                break;
        case TYP_BNOBT:
        case TYP_CNTBT:
+               if (nrecs > mp->m_alloc_mxr[1])
+                       return;
+
                akp = XFS_ALLOC_KEY_ADDR(mp, block, 1);
                app = XFS_ALLOC_PTR_ADDR(mp, block, 1, mp->m_alloc_mxr[1]);
-               zp1 = &akp[nrecs];
-               zlen1 = (char *)&app[0] - (char *)&akp[nrecs];
-               zp2 = &app[nrecs];
-               zlen2 = (char *)block + mp->m_sb.sb_blocksize -
-                                                       (char *)&app[nrecs];
+               zp1 = (char *)&akp[nrecs];
+               zp2 = (char *)&app[nrecs];
+               key_end = (char *)app;
                break;
        default:
-               zp1 = NULL;
-               break;
+               return;
        }
 
-       if (zp1 && zp2) {
-               /* Zero from end of keys to beginning of pointers */
-               memset(zp1, 0, zlen1);
-               /* Zero from end of pointers to end of block */
-               memset(zp2, 0, zlen2);
-       }
+
+       /* Zero from end of keys to beginning of pointers */
+       memset(zp1, 0, key_end - zp1);
+
+       /* Zero from end of pointers to end of block */
+       memset(zp2, 0, (char *)block + mp->m_sb.sb_blocksize - zp2);
 }
 
+/*
+ * We could be processing a corrupt block, so we can't trust any of
+ * the offsets or lengths to be within the buffer range. Hence check
+ * carefully!
+ */
 static void
 zero_btree_leaf(
        struct xfs_btree_block  *block,
@@ -316,38 +330,43 @@ zero_btree_leaf(
        struct xfs_bmbt_rec     *brp;
        struct xfs_inobt_rec    *irp;
        struct xfs_alloc_rec    *arp;
-       void                    *zp;
-       int                     zlen;
+       char                    *zp;
 
        nrecs = be16_to_cpu(block->bb_numrecs);
+       if (nrecs < 0)
+               return;
 
        switch (btype) {
        case TYP_BMAPBTA:
        case TYP_BMAPBTD:
+               if (nrecs > mp->m_bmap_dmxr[1])
+                       return;
+
                brp = XFS_BMBT_REC_ADDR(mp, block, 1);
-               zp = &brp[nrecs];
-               zlen = (char *)block + mp->m_sb.sb_blocksize - (char 
*)&brp[nrecs];
+               zp = (char *)&brp[nrecs];
                break;
        case TYP_INOBT:
        case TYP_FINOBT:
+               if (nrecs > mp->m_inobt_mxr[1])
+                       return;
+
                irp = XFS_INOBT_REC_ADDR(mp, block, 1);
-               zp = &irp[nrecs];
-               zlen = (char *)block + mp->m_sb.sb_blocksize - (char 
*)&irp[nrecs];
+               zp = (char *)&irp[nrecs];
                break;
        case TYP_BNOBT:
        case TYP_CNTBT:
+               if (nrecs > mp->m_alloc_mxr[1])
+                       return;
+
                arp = XFS_ALLOC_REC_ADDR(mp, block, 1);
-               zp = &arp[nrecs];
-               zlen = (char *)block + mp->m_sb.sb_blocksize - (char 
*)&arp[nrecs];
+               zp = (char *)&arp[nrecs];
                break;
        default:
-               zp = NULL;
-               break;
+               return;
        }
 
        /* Zero from end of records to end of block */
-       if (zp && zlen < mp->m_sb.sb_blocksize)
-               memset(zp, 0, zlen);
+       memset(zp, 0, (char *)block + mp->m_sb.sb_blocksize - zp);
 }
 
 static void
-- 
2.5.0

<Prev in Thread] Current Thread [Next in Thread>