[PATCH] xfs_db: add blockget -L option

[PATCH] xfs_db: add blockget -L option

[hal]

From: Hal Pomeranz <hal@deer-run.com>

Allow blockget on a mounted file system or "dirty" file system image
with pending log entries-- by simply ignoring the log.  This makes
xfs_db more useful for forensics where we are often dealing with these
types of images.  Flushing log entries to disk by mounting/unmounting
the file system would allow us to use blockget, but would make changes
to the file system state which are not desirable in forensics contexts.

Signed-off-by: Hal Pomeranz <hal@deer-run.com>
---
 db/check.c        | 74 +++++++++++++++++++++++++++++++------------------------
 db/sb.c           | 12 ++++-----
 man/man8/xfs_db.8 |  8 +++++-
 3 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/db/check.c b/db/check.c
index 2f8dee5..034676a 100644
--- a/db/check.c
+++ b/db/check.c
@@ -378,7 +378,7 @@ static const cmdinfo_t	blockfree_cmd =
 	  NULL, N_("free block usage information"), NULL };
 static const cmdinfo_t	blockget_cmd =
 	{ "blockget", "check", blockget_f, 0, -1, 0,
-	  N_("[-s|-v] [-n] [-t] [-b bno]... [-i ino] ..."),
+	  N_("[-s|-v] [-L] [-n] [-t] [-b bno]... [-i ino] ..."),
 	  N_("get block usage and check consistency"), NULL };
 static const cmdinfo_t	blocktrash_cmd =
 	{ "blocktrash", NULL, blocktrash_f, 0, -1, 0,
@@ -1850,38 +1850,11 @@ init(
 	int		c;
 	xfs_ino_t	ino;
 	int		rt;
+	int		ignore_log;
+	int		lc;
 
-	serious_error = 0;
-	if (mp->m_sb.sb_magicnum != XFS_SB_MAGIC) {
-		dbprintf(_("bad superblock magic number %x, giving up\n"),
-			mp->m_sb.sb_magicnum);
-		serious_error = 1;
-		return 0;
-	}
-	if (!sb_logcheck())
-		return 0;
-	rt = mp->m_sb.sb_rextents != 0;
-	dbmap = xmalloc((mp->m_sb.sb_agcount + rt) * sizeof(*dbmap));
-	inomap = xmalloc((mp->m_sb.sb_agcount + rt) * sizeof(*inomap));
-	inodata = xmalloc(mp->m_sb.sb_agcount * sizeof(*inodata));
-	inodata_hash_size =
-		(int)MAX(MIN(mp->m_sb.sb_icount /
-				(INODATA_AVG_HASH_LENGTH * mp->m_sb.sb_agcount),
-			     MAX_INODATA_HASH_SIZE),
-			 MIN_INODATA_HASH_SIZE);
-	for (c = 0; c < mp->m_sb.sb_agcount; c++) {
-		dbmap[c] = xcalloc(mp->m_sb.sb_agblocks, sizeof(**dbmap));
-		inomap[c] = xcalloc(mp->m_sb.sb_agblocks, sizeof(**inomap));
-		inodata[c] = xcalloc(inodata_hash_size, sizeof(**inodata));
-	}
-	if (rt) {
-		dbmap[c] = xcalloc(mp->m_sb.sb_rblocks, sizeof(**dbmap));
-		inomap[c] = xcalloc(mp->m_sb.sb_rblocks, sizeof(**inomap));
-		sumfile = xcalloc(mp->m_rsumsize, 1);
-		sumcompute = xcalloc(mp->m_rsumsize, 1);
-	}
-	nflag = sflag = tflag = verbose = optind = 0;
-	while ((c = getopt(argc, argv, "b:i:npstv")) != EOF) {
+	ignore_log = nflag = sflag = tflag = verbose = optind = 0;
+	while ((c = getopt(argc, argv, "b:i:Lnpstv")) != EOF) {
 		switch (c) {
 		case 'b':
 			bno = strtoll(optarg, NULL, 10);
@@ -1891,6 +1864,9 @@ init(
 			ino = strtoll(optarg, NULL, 10);
 			add_ilist(ino);
 			break;
+		case 'L':
+			ignore_log = 1;
+			break;
 		case 'n':
 			nflag = 1;
 			break;
@@ -1911,6 +1887,40 @@ init(
 			return 0;
 		}
 	}
+
+	serious_error = 0;
+	if (mp->m_sb.sb_magicnum != XFS_SB_MAGIC) {
+		dbprintf(_("bad superblock magic number %x, giving up\n"),
+			mp->m_sb.sb_magicnum);
+		serious_error = 1;
+		return 0;
+	}
+
+	lc = sb_logcheck();
+	if (lc < 0 || (lc == 0 && ignore_log == 0))
+		return 0;
+
+	rt = mp->m_sb.sb_rextents != 0;
+	dbmap = xmalloc((mp->m_sb.sb_agcount + rt) * sizeof(*dbmap));
+	inomap = xmalloc((mp->m_sb.sb_agcount + rt) * sizeof(*inomap));
+	inodata = xmalloc(mp->m_sb.sb_agcount * sizeof(*inodata));
+	inodata_hash_size =
+		(int)MAX(MIN(mp->m_sb.sb_icount /
+				(INODATA_AVG_HASH_LENGTH * mp->m_sb.sb_agcount),
+			     MAX_INODATA_HASH_SIZE),
+			 MIN_INODATA_HASH_SIZE);
+	for (c = 0; c < mp->m_sb.sb_agcount; c++) {
+		dbmap[c] = xcalloc(mp->m_sb.sb_agblocks, sizeof(**dbmap));
+		inomap[c] = xcalloc(mp->m_sb.sb_agblocks, sizeof(**inomap));
+		inodata[c] = xcalloc(inodata_hash_size, sizeof(**inodata));
+	}
+	if (rt) {
+		dbmap[c] = xcalloc(mp->m_sb.sb_rblocks, sizeof(**dbmap));
+		inomap[c] = xcalloc(mp->m_sb.sb_rblocks, sizeof(**inomap));
+		sumfile = xcalloc(mp->m_rsumsize, 1);
+		sumcompute = xcalloc(mp->m_rsumsize, 1);
+	}
+
 	error = sbver_err = serious_error = 0;
 	fdblocks = frextents = icount = ifree = 0;
 	sbversion = XFS_SB_VERSION_4;
diff --git a/db/sb.c b/db/sb.c
index c7fbfd6..ba51910 100644
--- a/db/sb.c
+++ b/db/sb.c
@@ -235,13 +235,13 @@ sb_logcheck(void)
 		if (x.logdev && x.logdev != x.ddev) {
 			dbprintf(_("aborting - external log specified for FS "
 				 "with an internal log\n"));
-			return 0;
+			return -1;
 		}
 	} else {
 		if (!x.logdev || (x.logdev == x.ddev)) {
 			dbprintf(_("aborting - no external log specified for FS "
 				 "with an external log\n"));
-			return 0;
+			return -1;
 		}
 	}
 
@@ -250,14 +250,14 @@ sb_logcheck(void)
 	dirty = xlog_is_dirty(mp, mp->m_log, &x, 0);
 	if (dirty == -1) {
 		dbprintf(_("ERROR: cannot find log head/tail, run xfs_repair\n"));
-		return 0;
+		return -1;
 	} else if (dirty == 1) {
 		dbprintf(_(
 "ERROR: The filesystem has valuable metadata changes in a log which needs to\n"
 "be replayed.  Mount the filesystem to replay the log, and unmount it before\n"
 "re-running %s.  If you are unable to mount the filesystem, then use\n"
-"the xfs_repair -L option to destroy the log and attempt a repair.\n"
-"Note that destroying the log may cause corruption -- please attempt a mount\n"
+"the -L option to ignore the log. Note that ignoring the log may cause\n"
+"the program to crash or produce erroneous output. Please attempt a mount\n"
 "of the filesystem before doing this.\n"), progname);
 		return 0;
 	}
@@ -271,7 +271,7 @@ sb_logzero(uuid_t *uuidp)
 	int	cycle = XLOG_INIT_CYCLE;
 	int	error;
 
-	if (!sb_logcheck())
+	if (sb_logcheck() < 1)
 		return 0;
 
 	/*
diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
index 524b1ef..ec6a5ed 100644
--- a/man/man8/xfs_db.8
+++ b/man/man8/xfs_db.8
@@ -194,7 +194,7 @@ command. This must be done before another
 .B blockget
 command can be given, presumably with different arguments than the previous one.
 .TP
-.BI "blockget [\-npvs] [\-b " bno "] ... [\-i " ino "] ..."
+.BI "blockget [\-Lnpvs] [\-b " bno "] ... [\-i " ino "] ..."
 Get block usage and check filesystem consistency.
 The information is saved for use by a subsequent
 .BR blockuse ", " ncheck ", or " blocktrash
@@ -209,6 +209,12 @@ information should be printed.
 is used to specify inode numbers about which verbose information
 should be printed.
 .TP
+.B \-L
+is used to ignore unplayed log entries and attempt to run
+.B blockget
+anyway. Note that using this option may cause the program to crash
+or produce erroneous output.
+.TP
 .B \-n
 is used to save pathnames for inodes visited, this is used to support the
 .BR xfs_ncheck (8)
-- 
1.8.3.1

Re: [PATCH] xfs_db: add blockget -L option

[Eric Sandeen]

On 5/10/18 4:36 PM, hal@deer-run.com wrote:
> From: Hal Pomeranz <hal@deer-run.com>
> 
> Allow blockget on a mounted file system or "dirty" file system image
> with pending log entries-- by simply ignoring the log.  This makes
> xfs_db more useful for forensics where we are often dealing with these
> types of images.  Flushing log entries to disk by mounting/unmounting
> the file system would allow us to use blockget, but would make changes
> to the file system state which are not desirable in forensics contexts.
> 
> Signed-off-by: Hal Pomeranz <hal@deer-run.com>

Well, I'm back on the fence about this one; I know I steered you in
this direction, but after talking to dchinner a bit, he raised some valid
concerns about adding an option which essentially makes the tool behave
in unpredictable ways (by trying to traverse an inconsistent filesystem).

I had jumped to the conclusion that the usecase was for examining an
offline filesystem image without needing to perturb it by replaying the
log; Dave pointed out that you're talking about using this on a live
filesystem, and even baking this behavior into other higher level tools.
That kind of opened my eyes to all the ways this can go wrong.  :(

Thus spake Dave:

> IMO, the only thing worse than not having a forensic tool for a
> specific job is having a forensic tool provided by a trusted toolkit
> whose results are unreliable and cannot be trusted....

...

> I'd suggest, in that case, that you limit it's use to off-line or
> read-only snapshots of online filesystems? This would mean that the
> results of xfs_db operations (while eceedingly slow) will be
> reliable.

And in both of those cases, you won't need to ignore a dirty log.
Especially with teh express stated purpose of examining live
filesystems, I really hate to give you this much rope.

Stuff like parent pointers and fsmap are (eventually) going to give
you a much better way to achieve what (I think) you want, here.

-Eric

Re: [PATCH] xfs_db: add blockget -L option

[Hal Pomeranz]

I get the argument Dave is making. But the reality of forensics is that many of our tools are not reliable in all cases. We do a lot of cross-validation for crucial results. 

Having multiple tools helps. Right now, we basically have nothing for XFS. So adding this option to xfs_db makes things better. 

We work with underplayed file systems constantly, and it’s less of a worry than you might think. Try my patched version of xfs_db on a live file system of your choice. See if you can make it blow up. When it doesn’t, please consider folding in my patch.

--Hal

> On May 14, 2018, at 11:14 PM, Eric Sandeen <sandeen@sandeen.net> wrote:
> 
> 
> 
>> On 5/10/18 4:36 PM, hal@deer-run.com wrote:
>> From: Hal Pomeranz <hal@deer-run.com>
>> Allow blockget on a mounted file system or "dirty" file system image
>> with pending log entries-- by simply ignoring the log.  This makes
>> xfs_db more useful for forensics where we are often dealing with these
>> types of images.  Flushing log entries to disk by mounting/unmounting
>> the file system would allow us to use blockget, but would make changes
>> to the file system state which are not desirable in forensics contexts.
>> Signed-off-by: Hal Pomeranz <hal@deer-run.com>
> 
> Well, I'm back on the fence about this one; I know I steered you in
> this direction, but after talking to dchinner a bit, he raised some valid
> concerns about adding an option which essentially makes the tool behave
> in unpredictable ways (by trying to traverse an inconsistent filesystem).
> 
> I had jumped to the conclusion that the usecase was for examining an
> offline filesystem image without needing to perturb it by replaying the
> log; Dave pointed out that you're talking about using this on a live
> filesystem, and even baking this behavior into other higher level tools.
> That kind of opened my eyes to all the ways this can go wrong.  :(
> 
> Thus spake Dave:
> 
>> IMO, the only thing worse than not having a forensic tool for a
>> specific job is having a forensic tool provided by a trusted toolkit
>> whose results are unreliable and cannot be trusted....
> 
> ...
> 
>> I'd suggest, in that case, that you limit it's use to off-line or
>> read-only snapshots of online filesystems? This would mean that the
>> results of xfs_db operations (while eceedingly slow) will be
>> reliable.
> 
> And in both of those cases, you won't need to ignore a dirty log.
> Especially with teh express stated purpose of examining live
> filesystems, I really hate to give you this much rope.
> 
> Stuff like parent pointers and fsmap are (eventually) going to give
> you a much better way to achieve what (I think) you want, here.
> 
> -Eric

Re: [PATCH] xfs_db: add blockget -L option

[Dave Chinner]

On Tue, May 15, 2018 at 12:27:38AM -0400, Hal Pomeranz wrote:
> I get the argument Dave is making. But the reality of forensics is
> that many of our tools are not reliable in all cases. We do a lot
> of cross-validation for crucial results. 
> 
> Having multiple tools helps. Right now, we basically have nothing
> for XFS. So adding this option to xfs_db makes things better. 
> 
> We work with underplayed file systems constantly, and it’s
> less of a worry than you might think. Try my patched version of
> xfs_db on a live file system of your choice. See if you can make
> it blow up. When it doesn’t, please consider folding in my
> patch.

And the source code to your new and improved xfs_db can be found
where? I'm interested to know how you've solved the problem of
detecting stale metadata from userspace on a live filesystem...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs_db: add blockget -L option

[Eric Sandeen]

On 5/15/18 12:27 AM, Dave Chinner wrote:
> On Tue, May 15, 2018 at 12:27:38AM -0400, Hal Pomeranz wrote:
>> I get the argument Dave is making. But the reality of forensics is
>> that many of our tools are not reliable in all cases. We do a lot
>> of cross-validation for crucial results.
>>
>> Having multiple tools helps. Right now, we basically have nothing
>> for XFS. So adding this option to xfs_db makes things better.
>>
>> We work with underplayed file systems constantly, and it’s
>> less of a worry than you might think. Try my patched version of
>> xfs_db on a live file system of your choice. See if you can make
>> it blow up. When it doesn’t, please consider folding in my
>> patch.
> 
> And the source code to your new and improved xfs_db can be found
> where? I'm interested to know how you've solved the problem of
> detecting stale metadata from userspace on a live filesystem...

I believe he's referring to the patch he just sent, Dave.

-Eric