Skip to contentSkip to conversation

Try to squash metadump data leaks

PATCH Superseded Stefan Ring 2 participants 11 messages Oct 5, 2018 – Oct 10, 2018

v1 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

cover Try to squash metadump data leaks

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(-)
Signature · 1 line
2.14.4
#1 lore raw

PATCH 1/1 xfs_metadump: Zap more stale data

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(-)
db/metadump.c +69 -5
index cc2ae9af..e7159cd1 100644
@@ -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();
 
Signature · 1 line
2.14.4
#2 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

Quoted 11 lines · Stefan Ring

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:

Quoted 17 lines
---
 db/metadump.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 5 deletions(-)
db/metadump.c +5 -0
index cc2ae9af..e7159cd1 100644
@@ -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;

db/metadump.c process_sf_attr( · line 1438

Quoted 10 lines
db/metadump.c +10 -0
+	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 ?

db/metadump.c process_sf_attr( · line 1449

Quoted 11 lines
db/metadump.c +11 -0
+		__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;
}
Quoted 15 lines
db/metadump.c +5 -1
+		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;

db/metadump.c process_dir_leaf_block( · line 1484

Quoted 6 lines
db/metadump.c +3 -0
@@ -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?

Quoted 13 lines
db/metadump.c +9 -0
+		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;
 	}
 }
 

db/metadump.c process_dir_data_block( · line 1544

Quoted 5 lines
db/metadump.c +1 -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.

Quoted 14 lines
db/metadump.c +1 -2
 		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 {

db/metadump.c process_btinode( · line 2164

Quoted 9 lines
db/metadump.c +6 -0
@@ -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.

Quoted 12 lines
db/metadump.c +8 -0
+		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;

db/metadump.c process_inode( · line 2310

Quoted 7 lines
db/metadump.c +3 -1
@@ -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.

db/metadump.c process_inode( · line 2316

Quoted 6 lines
db/metadump.c +6 -0
+				if (show_warnings)
+					print_warning("inode %llu has unexpected extents",
+							(unsigned long long)cur_ino);
+				success = 0;
+			}
+			else

} else

	memset(...)

db/metadump.c process_inode( · line 2317

Quoted 1 line
db/metadump.c +1 -0
+				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

Quoted 6 lines
db/metadump.c +0 -0
 	}
 	nametable_clear();
 
Signature · 1 line
2.14.4
#3 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

Quoted 37 lines · Darrick J. Wong
Quoted 11 lines · Stefan Ring

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:

Quoted 21 lines
---
 db/metadump.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 5 deletions(-)
db/metadump.c +9 -0
index cc2ae9af..e7159cd1 100644
@@ -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;
Quoted 8 lines

db/metadump.c process_sf_attr( · line 1438

Quoted 6 lines
db/metadump.c +6 -0
+
+     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 ;).

#4 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

Quoted 47 lines · Stefan Ring
Quoted 37 lines · Darrick J. Wong
Quoted 11 lines · Stefan Ring

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:

Quoted 21 lines
---
 db/metadump.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 5 deletions(-)
db/metadump.c +9 -0
index cc2ae9af..e7159cd1 100644
@@ -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;
Quoted 8 lines

db/metadump.c process_sf_attr( · line 1438

Quoted 6 lines
db/metadump.c +6 -0
+
+     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 ?

Quoted 2 lines

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

#5 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

In reply to Stefan Ring
Quoted 53 lines · Stefan Ring
db/metadump.c +32 -1
index cc2ae9af..e7159cd1 100644
@@ -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

db/metadump.c process_dir_leaf_block( · line 1492

Quoted 10 lines
db/metadump.c +10 -0
+       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?

Quoted 65 lines
db/metadump.c +27 -5
+               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

#6 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

Quoted 59 lines · Stefan Ring
Quoted 58 lines · Stefan Ring
db/metadump.c +37 -1
index cc2ae9af..e7159cd1 100644
@@ -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;
Quoted 8 lines

db/metadump.c process_dir_leaf_block( · line 1492

Quoted 5 lines
db/metadump.c +5 -0
+
+               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

Quoted 65 lines
Quoted 65 lines
db/metadump.c +27 -5
+               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

#7 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

Quoted 7 lines · Darrick J. Wong
Quoted 2 lines

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.

#8 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

Quoted 3 lines · Stefan Ring

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…

#9 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

Quoted 9 lines · Stefan Ring
Quoted 3 lines · Stefan Ring

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

#10 lore raw

Re: [PATCH 1/1] xfs_metadump: Zap more stale data

In reply to Stefan Ring
Quoted 10 lines · Stefan Ring
Quoted 3 lines · Stefan Ring

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.