* [PATCH 1/3] metadump: clean up btree block region zeroing
2016-02-04 23:00 [PATCH 0/3 v2] metadump/restore: fix up zeroing and accounting Dave Chinner
@ 2016-02-04 23:00 ` Dave Chinner
2016-02-08 8:52 ` Christoph Hellwig
2016-02-04 23:00 ` [PATCH 2/3] metadump: bounds check btree block regions being zeroed Dave Chinner
2016-02-04 23:00 ` [PATCH 3/3] xfs_mdrestore: correctly account bytes read Dave Chinner
2 siblings, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2016-02-04 23:00 UTC (permalink / raw)
To: xfs
From: Dave Chinner <dchinner@redhat.com>
Abstract out all the common operations in the btree block zeroing
so that we only have one copy of offset/len calculations and don't
require lots of casts for the pointer arithmetic.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Tested-by: Arkadiusz Miskiewicz <arekm@maven.pl>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
db/metadump.c | 61 +++++++++++++++++++++++------------------------------------
1 file changed, 24 insertions(+), 37 deletions(-)
diff --git a/db/metadump.c b/db/metadump.c
index 8455fdd..a185da5 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -258,8 +258,8 @@ 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);
@@ -268,43 +268,36 @@ zero_btree_node(
case TYP_BMAPBTD:
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:
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:
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);
}
static void
@@ -316,8 +309,7 @@ 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);
@@ -325,29 +317,24 @@ zero_btree_leaf(
case TYP_BMAPBTA:
case TYP_BMAPBTD:
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:
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:
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
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/3] metadump: bounds check btree block regions being zeroed
2016-02-04 23:00 [PATCH 0/3 v2] metadump/restore: fix up zeroing and accounting Dave Chinner
2016-02-04 23:00 ` [PATCH 1/3] metadump: clean up btree block region zeroing Dave Chinner
@ 2016-02-04 23:00 ` Dave Chinner
2016-02-05 14:20 ` Brian Foster
2016-02-08 8:53 ` Christoph Hellwig
2016-02-04 23:00 ` [PATCH 3/3] xfs_mdrestore: correctly account bytes read Dave Chinner
2 siblings, 2 replies; 8+ messages in thread
From: Dave Chinner @ 2016-02-04 23:00 UTC (permalink / raw)
To: xfs
From: Dave Chinner <dchinner@redhat.com>
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.
Reported-by: Arkadiusz Miskiewicz <arekm@maven.pl>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
db/metadump.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/db/metadump.c b/db/metadump.c
index a185da5..26a3bd5 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,
@@ -262,10 +267,15 @@ zero_btree_node(
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 = (char *)&bkp[nrecs];
@@ -274,6 +284,9 @@ zero_btree_node(
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 = (char *)&ikp[nrecs];
@@ -282,6 +295,9 @@ zero_btree_node(
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 = (char *)&akp[nrecs];
@@ -300,6 +316,11 @@ zero_btree_node(
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,
@@ -312,20 +333,31 @@ zero_btree_leaf(
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[0])
+ return;
+
brp = XFS_BMBT_REC_ADDR(mp, block, 1);
zp = (char *)&brp[nrecs];
break;
case TYP_INOBT:
case TYP_FINOBT:
+ if (nrecs > mp->m_inobt_mxr[0])
+ return;
+
irp = XFS_INOBT_REC_ADDR(mp, block, 1);
zp = (char *)&irp[nrecs];
break;
case TYP_BNOBT:
case TYP_CNTBT:
+ if (nrecs > mp->m_alloc_mxr[0])
+ return;
+
arp = XFS_ALLOC_REC_ADDR(mp, block, 1);
zp = (char *)&arp[nrecs];
break;
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] metadump: bounds check btree block regions being zeroed
2016-02-04 23:00 ` [PATCH 2/3] metadump: bounds check btree block regions being zeroed Dave Chinner
@ 2016-02-05 14:20 ` Brian Foster
2016-02-08 8:53 ` Christoph Hellwig
1 sibling, 0 replies; 8+ messages in thread
From: Brian Foster @ 2016-02-05 14:20 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
On Fri, Feb 05, 2016 at 10:00:57AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> 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.
>
> Reported-by: Arkadiusz Miskiewicz <arekm@maven.pl>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> db/metadump.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/db/metadump.c b/db/metadump.c
> index a185da5..26a3bd5 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,
> @@ -262,10 +267,15 @@ zero_btree_node(
> 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 = (char *)&bkp[nrecs];
> @@ -274,6 +284,9 @@ zero_btree_node(
> 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 = (char *)&ikp[nrecs];
> @@ -282,6 +295,9 @@ zero_btree_node(
> 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 = (char *)&akp[nrecs];
> @@ -300,6 +316,11 @@ zero_btree_node(
> 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,
> @@ -312,20 +333,31 @@ zero_btree_leaf(
> 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[0])
> + return;
> +
> brp = XFS_BMBT_REC_ADDR(mp, block, 1);
> zp = (char *)&brp[nrecs];
> break;
> case TYP_INOBT:
> case TYP_FINOBT:
> + if (nrecs > mp->m_inobt_mxr[0])
> + return;
> +
> irp = XFS_INOBT_REC_ADDR(mp, block, 1);
> zp = (char *)&irp[nrecs];
> break;
> case TYP_BNOBT:
> case TYP_CNTBT:
> + if (nrecs > mp->m_alloc_mxr[0])
> + return;
> +
> arp = XFS_ALLOC_REC_ADDR(mp, block, 1);
> zp = (char *)&arp[nrecs];
> break;
> --
> 2.5.0
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] metadump: bounds check btree block regions being zeroed
2016-02-04 23:00 ` [PATCH 2/3] metadump: bounds check btree block regions being zeroed Dave Chinner
2016-02-05 14:20 ` Brian Foster
@ 2016-02-08 8:53 ` Christoph Hellwig
1 sibling, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-02-08 8:53 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] xfs_mdrestore: correctly account bytes read
2016-02-04 23:00 [PATCH 0/3 v2] metadump/restore: fix up zeroing and accounting Dave Chinner
2016-02-04 23:00 ` [PATCH 1/3] metadump: clean up btree block region zeroing Dave Chinner
2016-02-04 23:00 ` [PATCH 2/3] metadump: bounds check btree block regions being zeroed Dave Chinner
@ 2016-02-04 23:00 ` Dave Chinner
2016-02-08 8:53 ` Christoph Hellwig
2 siblings, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2016-02-04 23:00 UTC (permalink / raw)
To: xfs
From: Dave Chinner <dchinner@redhat.com>
Progess indication comes in the form of a "X MB read" output. This
doesn't match up with the actual number of bytes read from the
metadump file because it only accounts header blocks in the file,
not actual metadata blocks that are restored, Hence the number
reported is usually much lower than the size of the metadump file,
hence it's impossible to use to gauge progress of the restore.
While there, fix the progress output so that it overwrites the
previous progress output line correctly.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
mdrestore/xfs_mdrestore.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index ebc5e54..70a160c 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -133,7 +133,7 @@ perform_restore(
for (;;) {
if (show_progress && (bytes_read & ((1 << 20) - 1)) == 0)
- print_progress("%lld MB read\n", bytes_read >> 20);
+ print_progress("%lld MB read", bytes_read >> 20);
for (cur_index = 0; cur_index < mb_count; cur_index++) {
if (pwrite64(dst_fd, &block_buffer[cur_index <<
@@ -160,7 +160,7 @@ perform_restore(
1, src_f) != 1)
fatal("error reading from file: %s\n", strerror(errno));
- bytes_read += block_size;
+ bytes_read += block_size + (mb_count << tmb.mb_blocklog);
}
if (progress_since_warning)
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 8+ messages in thread