* [PATCH 0/1] Try to squash metadump data leaks
@ 2018-10-04 20:57 Stefan Ring
2018-10-04 20:57 ` [PATCH 1/1] xfs_metadump: Zap more stale data Stefan Ring
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Ring @ 2018-10-04 20:57 UTC (permalink / raw)
To: linux-xfs
This is inspired by a thread last year when someone intended to
collect metadata about filesystems and I would have been happy to
help, except that I noticed lots of left-over data in the dump that
should never have been there. I would not have worried that much about
some fragments of Python code or directory listings, but the
possibility of recognizable customer data (potentially even
cryptographic keys) made it unthinkable to share this.
My method of coming up with these patches was: Pipe a metadump of my
reference image through "strings -n 10" and scroll until something
recognizable catches my eye. This did not take too long, usually. Find
the origin of the found leak and squash it (using "XFS File System
Structure" from the wiki). Repeat until there is nothing recognizable
left. Said image is a 1.1 TB volume created in early 2012 and used
daily ever since on our development server, containing about 12
million inodes (mostly hundreds of checkouts of our main Mercurial
repo with about 15000 files in it).
I have not submitted a patch before, and I don't think I will be
particularly pushy with this one. It exists mostly to inform you of my
findings. I have not dealt at all with a v3 filesystem. TBH, I don't
even know what this is and how to create one. Looking at the metadump
code as it exists now, it would likely have been much safer to copy
just the required contents as opposed to copying everything and then
trying to find every nook and cranny where unwanted stuff could seep
through.
Stefan Ring (1):
xfs_metadump: Zap more stale data
db/metadump.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 69 insertions(+), 5 deletions(-)
--
2.14.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-04 20:57 [PATCH 0/1] Try to squash metadump data leaks Stefan Ring
@ 2018-10-04 20:57 ` Stefan Ring
2018-10-04 22:23 ` Darrick J. Wong
2018-10-05 20:35 ` Stefan Ring
0 siblings, 2 replies; 11+ messages in thread
From: Stefan Ring @ 2018-10-04 20:57 UTC (permalink / raw)
To: linux-xfs
I have empirically found and tried to fix some places where stale data was not properly zeroed out.
In the order of the code changes:
The "freeindex" blocks in inode directories, from last entry to end of block.
XFS_DIR2_LEAF1_MAGIC, from last entry to end of block.
In btree format inodes before as well as after the btree pointers.
In dev inodes, everything after the header.
---
db/metadump.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 69 insertions(+), 5 deletions(-)
diff --git a/db/metadump.c b/db/metadump.c
index cc2ae9af..e7159cd1 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -1421,12 +1421,42 @@ process_sf_attr(
memset(asfep, 0, XFS_DFORK_ASIZE(dip, mp) - ino_attr_size);
}
+static void
+process_dir_free_block(
+ char *block)
+{
+ struct xfs_dir2_free *free;
+ struct xfs_dir3_icfree_hdr freehdr;
+
+ if (!zero_stale_data)
+ return;
+
+ free = (struct xfs_dir2_free *)block;
+ M_DIROPS(mp)->free_hdr_from_disk(&freehdr, free);
+
+ /* Zero out space from end of bests[] to end of block */
+ if (freehdr.magic == XFS_DIR2_FREE_MAGIC) {
+ __be16 *bests;
+ char *high;
+ int used;
+
+ bests = M_DIROPS(mp)->free_bests_p(free);
+ high = (char *)&bests[freehdr.nvalid];
+ used = high - (char*)free;
+ memset(high, 0, mp->m_sb.sb_blocksize - used);
+ iocur_top->need_crc = 1;
+ }
+ else if (show_warnings)
+ print_warning("invalid magic in dir inode %llu free block",
+ (unsigned long long)cur_ino);
+}
+
static void
process_dir_leaf_block(
char *block)
{
struct xfs_dir2_leaf *leaf;
- struct xfs_dir3_icleaf_hdr leafhdr;
+ struct xfs_dir3_icleaf_hdr leafhdr;
if (!zero_stale_data)
return;
@@ -1449,6 +1479,18 @@ process_dir_leaf_block(
lbp = xfs_dir2_leaf_bests_p(ltp);
memset(free, 0, (char *)lbp - free);
iocur_top->need_crc = 1;
+ } else
+ if (leafhdr.magic == XFS_DIR2_LEAFN_MAGIC ||
+ leafhdr.magic == XFS_DIR3_LEAFN_MAGIC) {
+ struct xfs_dir2_leaf_entry *ents;
+ char *free;
+ int used;
+
+ ents = M_DIROPS(mp)->leaf_ents_p(leaf);
+ free = (char *)&ents[leafhdr.count];
+ used = free - (char*)leaf;
+ memset(free, 0, mp->m_sb.sb_blocksize - used);
+ iocur_top->need_crc = 1;
}
}
@@ -1499,7 +1541,7 @@ process_dir_data_block(
if (show_warnings)
print_warning(
"invalid magic in dir inode %llu block %ld",
- (long long)cur_ino, (long)offset);
+ (unsigned long long)cur_ino, (long)offset);
return;
}
@@ -1813,8 +1855,7 @@ process_single_fsb_objects(
switch (btype) {
case TYP_DIR2:
if (o >= mp->m_dir_geo->freeblk) {
- /* TODO, zap any stale data */
- break;
+ process_dir_free_block(dp);
} else if (o >= mp->m_dir_geo->leafblk) {
process_dir_leaf_block(dp);
} else {
@@ -2115,6 +2156,20 @@ process_btinode(
}
pp = XFS_BMDR_PTR_ADDR(dib, 1, maxrecs);
+
+ if (zero_stale_data) {
+ /* Space before btree pointers */
+ char *top;
+ int used;
+ top = (char*)XFS_BMDR_PTR_ADDR(dib, 1, nrecs);
+ memset(top, 0, (char*)pp - top);
+
+ /* Space after btree pointers */
+ top = (char*)&pp[nrecs];
+ used = top - (char*)dip;
+ memset(top, 0, mp->m_sb.sb_inodesize - used);
+ }
+
for (i = 0; i < nrecs; i++) {
xfs_agnumber_t ag;
xfs_agblock_t bno;
@@ -2250,7 +2305,16 @@ process_inode(
case S_IFREG:
success = process_inode_data(dip, TYP_DATA);
break;
- default: ;
+ default:
+ if (XFS_DFORK_NEXTENTS(dip, XFS_ATTR_FORK) ||
+ XFS_DFORK_NEXTENTS(dip, XFS_DATA_FORK)) {
+ if (show_warnings)
+ print_warning("inode %llu has unexpected extents",
+ (unsigned long long)cur_ino);
+ success = 0;
+ }
+ else
+ memset(XFS_DFORK_DPTR(dip), 0, mp->m_sb.sb_inodesize - (XFS_DFORK_DPTR(dip) - (char*)dip));
}
nametable_clear();
--
2.14.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-04 20:57 ` [PATCH 1/1] xfs_metadump: Zap more stale data Stefan Ring
@ 2018-10-04 22:23 ` Darrick J. Wong
2018-10-05 20:46 ` Stefan Ring
2018-10-05 20:35 ` Stefan Ring
1 sibling, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2018-10-04 22:23 UTC (permalink / raw)
To: Stefan Ring; +Cc: linux-xfs
On Thu, Oct 04, 2018 at 10:57:49PM +0200, Stefan Ring wrote:
> I have empirically found and tried to fix some places where stale data was not properly zeroed out.
>
> In the order of the code changes:
>
> The "freeindex" blocks in inode directories, from last entry to end of block.
>
> XFS_DIR2_LEAF1_MAGIC, from last entry to end of block.
>
> In btree format inodes before as well as after the btree pointers.
>
> In dev inodes, everything after the header.
Mostly looks ok, but with some style issues:
> ---
> db/metadump.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 69 insertions(+), 5 deletions(-)
>
> diff --git a/db/metadump.c b/db/metadump.c
> index cc2ae9af..e7159cd1 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -1421,12 +1421,42 @@ process_sf_attr(
> memset(asfep, 0, XFS_DFORK_ASIZE(dip, mp) - ino_attr_size);
> }
>
> +static void
> +process_dir_free_block(
> + char *block)
> +{
> + struct xfs_dir2_free *free;
> + struct xfs_dir3_icfree_hdr freehdr;
> +
> + if (!zero_stale_data)
> + return;
> +
> + free = (struct xfs_dir2_free *)block;
> + M_DIROPS(mp)->free_hdr_from_disk(&freehdr, free);
> +
> + /* Zero out space from end of bests[] to end of block */
> + if (freehdr.magic == XFS_DIR2_FREE_MAGIC) {
How about XFS_DIR3_FREE_MAGIC ?
> + __be16 *bests;
> + char *high;
> + int used;
> +
> + bests = M_DIROPS(mp)->free_bests_p(free);
> + high = (char *)&bests[freehdr.nvalid];
> + used = high - (char*)free;
> + memset(high, 0, mp->m_sb.sb_blocksize - used);
> + iocur_top->need_crc = 1;
> + }
> + else if (show_warnings)
Though perhaps this should be structured as:
switch (freehdr.magic) {
case XFS_DIR3_FREE_MAGIC:
case XFS_DIR2_FREE_MAGIC:
/* clear stuff */
break;
default:
if (show_warnings)
print_warning(...);
break;
}
> + print_warning("invalid magic in dir inode %llu free block",
> + (unsigned long long)cur_ino);
> +}
> +
> static void
> process_dir_leaf_block(
> char *block)
> {
> struct xfs_dir2_leaf *leaf;
> - struct xfs_dir3_icleaf_hdr leafhdr;
> + struct xfs_dir3_icleaf_hdr leafhdr;
>
> if (!zero_stale_data)
> return;
> @@ -1449,6 +1479,18 @@ process_dir_leaf_block(
> lbp = xfs_dir2_leaf_bests_p(ltp);
> memset(free, 0, (char *)lbp - free);
> iocur_top->need_crc = 1;
> + } else
> + if (leafhdr.magic == XFS_DIR2_LEAFN_MAGIC ||
> + leafhdr.magic == XFS_DIR3_LEAFN_MAGIC) {
Convert this whole thing to a switch?
> + struct xfs_dir2_leaf_entry *ents;
> + char *free;
> + int used;
> +
> + ents = M_DIROPS(mp)->leaf_ents_p(leaf);
> + free = (char *)&ents[leafhdr.count];
> + used = free - (char*)leaf;
> + memset(free, 0, mp->m_sb.sb_blocksize - used);
> + iocur_top->need_crc = 1;
> }
> }
>
> @@ -1499,7 +1541,7 @@ process_dir_data_block(
> if (show_warnings)
> print_warning(
> "invalid magic in dir inode %llu block %ld",
> - (long long)cur_ino, (long)offset);
> + (unsigned long long)cur_ino, (long)offset);
Unrelated (but still good) change.
> return;
> }
>
> @@ -1813,8 +1855,7 @@ process_single_fsb_objects(
> switch (btype) {
> case TYP_DIR2:
> if (o >= mp->m_dir_geo->freeblk) {
> - /* TODO, zap any stale data */
> - break;
> + process_dir_free_block(dp);
> } else if (o >= mp->m_dir_geo->leafblk) {
> process_dir_leaf_block(dp);
> } else {
> @@ -2115,6 +2156,20 @@ process_btinode(
> }
>
> pp = XFS_BMDR_PTR_ADDR(dib, 1, maxrecs);
> +
> + if (zero_stale_data) {
> + /* Space before btree pointers */
> + char *top;
> + int used;
> + top = (char*)XFS_BMDR_PTR_ADDR(dib, 1, nrecs);
Blank line between "int used;" and "top =...", please.
> + memset(top, 0, (char*)pp - top);
> +
> + /* Space after btree pointers */
> + top = (char*)&pp[nrecs];
> + used = top - (char*)dip;
> + memset(top, 0, mp->m_sb.sb_inodesize - used);
> + }
> +
> for (i = 0; i < nrecs; i++) {
> xfs_agnumber_t ag;
> xfs_agblock_t bno;
> @@ -2250,7 +2305,16 @@ process_inode(
> case S_IFREG:
> success = process_inode_data(dip, TYP_DATA);
> break;
> - default: ;
> + default:
> + if (XFS_DFORK_NEXTENTS(dip, XFS_ATTR_FORK) ||
> + XFS_DFORK_NEXTENTS(dip, XFS_DATA_FORK)) {
Please line up the XFS_DFORK_NEXTENTS(...) calls.
> + if (show_warnings)
> + print_warning("inode %llu has unexpected extents",
> + (unsigned long long)cur_ino);
> + success = 0;
> + }
> + else
} else
memset(...)
> + memset(XFS_DFORK_DPTR(dip), 0, mp->m_sb.sb_inodesize - (XFS_DFORK_DPTR(dip) - (char*)dip));
Please put this in a separate helper that isn't triple-indented and
overflows 80 columns.
--D
> }
> nametable_clear();
>
> --
> 2.14.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-04 20:57 ` [PATCH 1/1] xfs_metadump: Zap more stale data Stefan Ring
2018-10-04 22:23 ` Darrick J. Wong
@ 2018-10-05 20:35 ` Stefan Ring
2018-10-05 20:40 ` Darrick J. Wong
1 sibling, 1 reply; 11+ messages in thread
From: Stefan Ring @ 2018-10-05 20:35 UTC (permalink / raw)
To: linux-xfs
On Thu, Oct 4, 2018 at 10:57 PM Stefan Ring <stefanrin@gmail.com> wrote:
> diff --git a/db/metadump.c b/db/metadump.c
> index cc2ae9af..e7159cd1 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -1421,12 +1421,42 @@ process_sf_attr(
> memset(asfep, 0, XFS_DFORK_ASIZE(dip, mp) - ino_attr_size);
> }
>
> +static void
> +process_dir_free_block(
> + char *block)
> +{
> + struct xfs_dir2_free *free;
> + struct xfs_dir3_icfree_hdr freehdr;
> +
> + if (!zero_stale_data)
> + return;
> +
> + free = (struct xfs_dir2_free *)block;
> + M_DIROPS(mp)->free_hdr_from_disk(&freehdr, free);
> +
> + /* Zero out space from end of bests[] to end of block */
> + if (freehdr.magic == XFS_DIR2_FREE_MAGIC) {
> + __be16 *bests;
> + char *high;
> + int used;
> +
> + bests = M_DIROPS(mp)->free_bests_p(free);
> + high = (char *)&bests[freehdr.nvalid];
> + used = high - (char*)free;
> + memset(high, 0, mp->m_sb.sb_blocksize - used);
> + iocur_top->need_crc = 1;
> + }
> + else if (show_warnings)
> + print_warning("invalid magic in dir inode %llu free block",
> + (unsigned long long)cur_ino);
> +}
> +
> static void
> process_dir_leaf_block(
> char *block)
> {
> struct xfs_dir2_leaf *leaf;
> - struct xfs_dir3_icleaf_hdr leafhdr;
> + struct xfs_dir3_icleaf_hdr leafhdr;
>
> if (!zero_stale_data)
> return;
> @@ -1449,6 +1479,18 @@ process_dir_leaf_block(
> lbp = xfs_dir2_leaf_bests_p(ltp);
> memset(free, 0, (char *)lbp - free);
> iocur_top->need_crc = 1;
> + } else
> + if (leafhdr.magic == XFS_DIR2_LEAFN_MAGIC ||
> + leafhdr.magic == XFS_DIR3_LEAFN_MAGIC) {
> + struct xfs_dir2_leaf_entry *ents;
> + char *free;
> + int used;
> +
> + ents = M_DIROPS(mp)->leaf_ents_p(leaf);
> + free = (char *)&ents[leafhdr.count];
> + used = free - (char*)leaf;
> + memset(free, 0, mp->m_sb.sb_blocksize - used);
I forgot to mention this: sb_blocksize is used conspicuously rarely in
this file. Is this the right metric to lean on?
> + iocur_top->need_crc = 1;
> }
> }
>
> @@ -1499,7 +1541,7 @@ process_dir_data_block(
> if (show_warnings)
> print_warning(
> "invalid magic in dir inode %llu block %ld",
> - (long long)cur_ino, (long)offset);
> + (unsigned long long)cur_ino, (long)offset);
> return;
> }
>
> @@ -1813,8 +1855,7 @@ process_single_fsb_objects(
> switch (btype) {
> case TYP_DIR2:
> if (o >= mp->m_dir_geo->freeblk) {
> - /* TODO, zap any stale data */
> - break;
> + process_dir_free_block(dp);
> } else if (o >= mp->m_dir_geo->leafblk) {
> process_dir_leaf_block(dp);
> } else {
> @@ -2115,6 +2156,20 @@ process_btinode(
> }
>
> pp = XFS_BMDR_PTR_ADDR(dib, 1, maxrecs);
> +
> + if (zero_stale_data) {
> + /* Space before btree pointers */
> + char *top;
> + int used;
> + top = (char*)XFS_BMDR_PTR_ADDR(dib, 1, nrecs);
> + memset(top, 0, (char*)pp - top);
> +
> + /* Space after btree pointers */
> + top = (char*)&pp[nrecs];
> + used = top - (char*)dip;
> + memset(top, 0, mp->m_sb.sb_inodesize - used);
> + }
> +
> for (i = 0; i < nrecs; i++) {
> xfs_agnumber_t ag;
> xfs_agblock_t bno;
> @@ -2250,7 +2305,16 @@ process_inode(
> case S_IFREG:
> success = process_inode_data(dip, TYP_DATA);
> break;
> - default: ;
> + default:
> + if (XFS_DFORK_NEXTENTS(dip, XFS_ATTR_FORK) ||
> + XFS_DFORK_NEXTENTS(dip, XFS_DATA_FORK)) {
> + if (show_warnings)
> + print_warning("inode %llu has unexpected extents",
> + (unsigned long long)cur_ino);
> + success = 0;
> + }
> + else
> + memset(XFS_DFORK_DPTR(dip), 0, mp->m_sb.sb_inodesize - (XFS_DFORK_DPTR(dip) - (char*)dip));
> }
> nametable_clear();
>
> --
> 2.14.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-05 20:35 ` Stefan Ring
@ 2018-10-05 20:40 ` Darrick J. Wong
2018-10-07 9:43 ` Stefan Ring
0 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2018-10-05 20:40 UTC (permalink / raw)
To: Stefan Ring; +Cc: linux-xfs
On Fri, Oct 05, 2018 at 10:35:34PM +0200, Stefan Ring wrote:
> On Thu, Oct 4, 2018 at 10:57 PM Stefan Ring <stefanrin@gmail.com> wrote:
> > diff --git a/db/metadump.c b/db/metadump.c
> > index cc2ae9af..e7159cd1 100644
> > --- a/db/metadump.c
> > +++ b/db/metadump.c
> > @@ -1421,12 +1421,42 @@ process_sf_attr(
> > memset(asfep, 0, XFS_DFORK_ASIZE(dip, mp) - ino_attr_size);
> > }
> >
> > +static void
> > +process_dir_free_block(
> > + char *block)
> > +{
> > + struct xfs_dir2_free *free;
> > + struct xfs_dir3_icfree_hdr freehdr;
> > +
> > + if (!zero_stale_data)
> > + return;
> > +
> > + free = (struct xfs_dir2_free *)block;
> > + M_DIROPS(mp)->free_hdr_from_disk(&freehdr, free);
> > +
> > + /* Zero out space from end of bests[] to end of block */
> > + if (freehdr.magic == XFS_DIR2_FREE_MAGIC) {
> > + __be16 *bests;
> > + char *high;
> > + int used;
> > +
> > + bests = M_DIROPS(mp)->free_bests_p(free);
> > + high = (char *)&bests[freehdr.nvalid];
> > + used = high - (char*)free;
> > + memset(high, 0, mp->m_sb.sb_blocksize - used);
> > + iocur_top->need_crc = 1;
> > + }
> > + else if (show_warnings)
> > + print_warning("invalid magic in dir inode %llu free block",
> > + (unsigned long long)cur_ino);
> > +}
> > +
> > static void
> > process_dir_leaf_block(
> > char *block)
> > {
> > struct xfs_dir2_leaf *leaf;
> > - struct xfs_dir3_icleaf_hdr leafhdr;
> > + struct xfs_dir3_icleaf_hdr leafhdr;
> >
> > if (!zero_stale_data)
> > return;
> > @@ -1449,6 +1479,18 @@ process_dir_leaf_block(
> > lbp = xfs_dir2_leaf_bests_p(ltp);
> > memset(free, 0, (char *)lbp - free);
> > iocur_top->need_crc = 1;
> > + } else
> > + if (leafhdr.magic == XFS_DIR2_LEAFN_MAGIC ||
> > + leafhdr.magic == XFS_DIR3_LEAFN_MAGIC) {
> > + struct xfs_dir2_leaf_entry *ents;
> > + char *free;
> > + int used;
> > +
> > + ents = M_DIROPS(mp)->leaf_ents_p(leaf);
> > + free = (char *)&ents[leafhdr.count];
> > + used = free - (char*)leaf;
> > + memset(free, 0, mp->m_sb.sb_blocksize - used);
>
> I forgot to mention this: sb_blocksize is used conspicuously rarely in
> this file. Is this the right metric to lean on?
No, because directory blocks span multiple fs blocks (good self catch!).
You could look it up in the directory geometry structure in the
xfs_mount *.
--D
> > + iocur_top->need_crc = 1;
> > }
> > }
> >
> > @@ -1499,7 +1541,7 @@ process_dir_data_block(
> > if (show_warnings)
> > print_warning(
> > "invalid magic in dir inode %llu block %ld",
> > - (long long)cur_ino, (long)offset);
> > + (unsigned long long)cur_ino, (long)offset);
> > return;
> > }
> >
> > @@ -1813,8 +1855,7 @@ process_single_fsb_objects(
> > switch (btype) {
> > case TYP_DIR2:
> > if (o >= mp->m_dir_geo->freeblk) {
> > - /* TODO, zap any stale data */
> > - break;
> > + process_dir_free_block(dp);
> > } else if (o >= mp->m_dir_geo->leafblk) {
> > process_dir_leaf_block(dp);
> > } else {
> > @@ -2115,6 +2156,20 @@ process_btinode(
> > }
> >
> > pp = XFS_BMDR_PTR_ADDR(dib, 1, maxrecs);
> > +
> > + if (zero_stale_data) {
> > + /* Space before btree pointers */
> > + char *top;
> > + int used;
> > + top = (char*)XFS_BMDR_PTR_ADDR(dib, 1, nrecs);
> > + memset(top, 0, (char*)pp - top);
> > +
> > + /* Space after btree pointers */
> > + top = (char*)&pp[nrecs];
> > + used = top - (char*)dip;
> > + memset(top, 0, mp->m_sb.sb_inodesize - used);
> > + }
> > +
> > for (i = 0; i < nrecs; i++) {
> > xfs_agnumber_t ag;
> > xfs_agblock_t bno;
> > @@ -2250,7 +2305,16 @@ process_inode(
> > case S_IFREG:
> > success = process_inode_data(dip, TYP_DATA);
> > break;
> > - default: ;
> > + default:
> > + if (XFS_DFORK_NEXTENTS(dip, XFS_ATTR_FORK) ||
> > + XFS_DFORK_NEXTENTS(dip, XFS_DATA_FORK)) {
> > + if (show_warnings)
> > + print_warning("inode %llu has unexpected extents",
> > + (unsigned long long)cur_ino);
> > + success = 0;
> > + }
> > + else
> > + memset(XFS_DFORK_DPTR(dip), 0, mp->m_sb.sb_inodesize - (XFS_DFORK_DPTR(dip) - (char*)dip));
> > }
> > nametable_clear();
> >
> > --
> > 2.14.4
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-04 22:23 ` Darrick J. Wong
@ 2018-10-05 20:46 ` Stefan Ring
2018-10-05 20:57 ` Darrick J. Wong
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Ring @ 2018-10-05 20:46 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
On Fri, Oct 5, 2018 at 12:23 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Thu, Oct 04, 2018 at 10:57:49PM +0200, Stefan Ring wrote:
> > I have empirically found and tried to fix some places where stale data was not properly zeroed out.
> >
> > In the order of the code changes:
> >
> > The "freeindex" blocks in inode directories, from last entry to end of block.
> >
> > XFS_DIR2_LEAF1_MAGIC, from last entry to end of block.
> >
> > In btree format inodes before as well as after the btree pointers.
> >
> > In dev inodes, everything after the header.
>
> Mostly looks ok, but with some style issues:
>
> > ---
> > db/metadump.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 69 insertions(+), 5 deletions(-)
> >
> > diff --git a/db/metadump.c b/db/metadump.c
> > index cc2ae9af..e7159cd1 100644
> > --- a/db/metadump.c
> > +++ b/db/metadump.c
> > @@ -1421,12 +1421,42 @@ process_sf_attr(
> > memset(asfep, 0, XFS_DFORK_ASIZE(dip, mp) - ino_attr_size);
> > }
> >
> > +static void
> > +process_dir_free_block(
> > + char *block)
> > +{
> > + struct xfs_dir2_free *free;
> > + struct xfs_dir3_icfree_hdr freehdr;
> > +
> > + if (!zero_stale_data)
> > + return;
> > +
> > + free = (struct xfs_dir2_free *)block;
> > + M_DIROPS(mp)->free_hdr_from_disk(&freehdr, free);
> > +
> > + /* Zero out space from end of bests[] to end of block */
> > + if (freehdr.magic == XFS_DIR2_FREE_MAGIC) {
>
> How about XFS_DIR3_FREE_MAGIC ?
Is there any documentation on DIR3? I left it out on purpose and hoped
someone would fill in the blanks for me ;).
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-05 20:46 ` Stefan Ring
@ 2018-10-05 20:57 ` Darrick J. Wong
0 siblings, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2018-10-05 20:57 UTC (permalink / raw)
To: Stefan Ring; +Cc: linux-xfs
On Fri, Oct 05, 2018 at 10:46:28PM +0200, Stefan Ring wrote:
> On Fri, Oct 5, 2018 at 12:23 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> > On Thu, Oct 04, 2018 at 10:57:49PM +0200, Stefan Ring wrote:
> > > I have empirically found and tried to fix some places where stale data was not properly zeroed out.
> > >
> > > In the order of the code changes:
> > >
> > > The "freeindex" blocks in inode directories, from last entry to end of block.
> > >
> > > XFS_DIR2_LEAF1_MAGIC, from last entry to end of block.
> > >
> > > In btree format inodes before as well as after the btree pointers.
> > >
> > > In dev inodes, everything after the header.
> >
> > Mostly looks ok, but with some style issues:
> >
> > > ---
> > > db/metadump.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> > > 1 file changed, 69 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/db/metadump.c b/db/metadump.c
> > > index cc2ae9af..e7159cd1 100644
> > > --- a/db/metadump.c
> > > +++ b/db/metadump.c
> > > @@ -1421,12 +1421,42 @@ process_sf_attr(
> > > memset(asfep, 0, XFS_DFORK_ASIZE(dip, mp) - ino_attr_size);
> > > }
> > >
> > > +static void
> > > +process_dir_free_block(
> > > + char *block)
> > > +{
> > > + struct xfs_dir2_free *free;
> > > + struct xfs_dir3_icfree_hdr freehdr;
> > > +
> > > + if (!zero_stale_data)
> > > + return;
> > > +
> > > + free = (struct xfs_dir2_free *)block;
> > > + M_DIROPS(mp)->free_hdr_from_disk(&freehdr, free);
> > > +
> > > + /* Zero out space from end of bests[] to end of block */
> > > + if (freehdr.magic == XFS_DIR2_FREE_MAGIC) {
> >
> > How about XFS_DIR3_FREE_MAGIC ?
>
> Is there any documentation on DIR3? I left it out on purpose and hoped
> someone would fill in the blanks for me ;).
Fairly similar to DIR2, but with bigger headers, I think.
https://djwong.org/docs/kdoc/filesystems/xfs-data-structures/dynamic.html#leaf-directories
--D
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-05 20:40 ` Darrick J. Wong
@ 2018-10-07 9:43 ` Stefan Ring
2018-10-07 11:57 ` Stefan Ring
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Ring @ 2018-10-07 9:43 UTC (permalink / raw)
To: linux-xfs
On Fri, Oct 5, 2018 at 10:41 PM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > I forgot to mention this: sb_blocksize is used conspicuously rarely in
> > this file. Is this the right metric to lean on?
>
> No, because directory blocks span multiple fs blocks (good self catch!).
>
> You could look it up in the directory geometry structure in the
> xfs_mount *.
Interestingly my file system has dirblklog = 0. I guess I'll have to
start creating some toy file system images for experimentation.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-07 9:43 ` Stefan Ring
@ 2018-10-07 11:57 ` Stefan Ring
2018-10-07 16:21 ` Darrick J. Wong
2018-10-10 9:37 ` Stefan Ring
0 siblings, 2 replies; 11+ messages in thread
From: Stefan Ring @ 2018-10-07 11:57 UTC (permalink / raw)
To: linux-xfs
On Sun, Oct 7, 2018 at 11:43 AM Stefan Ring <stefanrin@gmail.com> wrote:
>
> Interestingly my file system has dirblklog = 0. I guess I'll have to
> start creating some toy file system images for experimentation.
Ok, I've found out how to create one with dirblklog > 0, but the
default seems to be, and have been for a very long time, zero
(directory blocks are the same size as file system blocks). I'm afraid
of uncovering all kinds of nastiness going this untrodden path…
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-07 11:57 ` Stefan Ring
@ 2018-10-07 16:21 ` Darrick J. Wong
2018-10-10 9:37 ` Stefan Ring
1 sibling, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2018-10-07 16:21 UTC (permalink / raw)
To: Stefan Ring; +Cc: linux-xfs
On Sun, Oct 07, 2018 at 01:57:10PM +0200, Stefan Ring wrote:
> On Sun, Oct 7, 2018 at 11:43 AM Stefan Ring <stefanrin@gmail.com> wrote:
> >
> > Interestingly my file system has dirblklog = 0. I guess I'll have to
> > start creating some toy file system images for experimentation.
>
> Ok, I've found out how to create one with dirblklog > 0, but the
> default seems to be, and have been for a very long time, zero
> (directory blocks are the same size as file system blocks). I'm afraid
> of uncovering all kinds of nastiness going this untrodden path…
Nothing is ever simple in xfs, sadly. :/
That said, I think the "recommended" ceph configuration still has
multiblock directories, so I think those code paths are somewhat
frequently used by some of the users.
(I guess you could temporarily change the patch to memset(buf, 'a', ...)
to make sure it only ever touches the empty parts of the dir block buffer.)
--D
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] xfs_metadump: Zap more stale data
2018-10-07 11:57 ` Stefan Ring
2018-10-07 16:21 ` Darrick J. Wong
@ 2018-10-10 9:37 ` Stefan Ring
1 sibling, 0 replies; 11+ messages in thread
From: Stefan Ring @ 2018-10-10 9:37 UTC (permalink / raw)
To: linux-xfs
On Sun, Oct 7, 2018 at 1:57 PM Stefan Ring <stefanrin@gmail.com> wrote:
>
> On Sun, Oct 7, 2018 at 11:43 AM Stefan Ring <stefanrin@gmail.com> wrote:
> >
> > Interestingly my file system has dirblklog = 0. I guess I'll have to
> > start creating some toy file system images for experimentation.
>
> Ok, I've found out how to create one with dirblklog > 0, but the
> default seems to be, and have been for a very long time, zero
> (directory blocks are the same size as file system blocks). I'm afraid
> of uncovering all kinds of nastiness going this untrodden path…
I think I'm fine now. I'll post an updated patch soon.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-10-10 16:59 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-04 20:57 [PATCH 0/1] Try to squash metadump data leaks Stefan Ring
2018-10-04 20:57 ` [PATCH 1/1] xfs_metadump: Zap more stale data Stefan Ring
2018-10-04 22:23 ` Darrick J. Wong
2018-10-05 20:46 ` Stefan Ring
2018-10-05 20:57 ` Darrick J. Wong
2018-10-05 20:35 ` Stefan Ring
2018-10-05 20:40 ` Darrick J. Wong
2018-10-07 9:43 ` Stefan Ring
2018-10-07 11:57 ` Stefan Ring
2018-10-07 16:21 ` Darrick J. Wong
2018-10-10 9:37 ` Stefan Ring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.