Re: xfs_db bug

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jan 02, 2025 at 02:14:45PM -0800, Tom Samstag wrote:
> Hi!
> 
> I'm encountering what I believe to be a bug in xfs_db with some code
> that worked previously for me. I have some code that uses xfs_db to
> copy some specific data out of an XFS disk image based on an inode
> number. Basically, it does similar to:
> 
> inode [inodenum]
> dblock 0
> p
> dblock 1
> p
> dblock 2
> p
> [etc]
> 
> This worked on older versions of xfs_db but is now resulting in
> corrupted output. I believe I've traced the issue to the code
> introduced in https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/commit/?id=b05a31722f5d4c5e071238cbedf491d5b109f278
> to support realtime files.
> 
> Specifically, the use of a `dblock` command when the previous command
> was not an `inode` command looks to lead to the data in
> iocur_top->data to no longer contain inode data as expected in the
> line
> if (is_rtfile(iocur_top->data))
> 
> I don't know the code well enough to submit a fix, but some quick
> experimentation suggested it may be sufficient to move the check for
> an rtfile to inside the push/pop context above after the call to
> set_cur_inode(iocur_top->ino).
> 
> Hopefully this describes the issue sufficiently but please let me know
> if you need anything else from me.

Oh, yeah, that's definitely a bug.  Thank you for isolating the cause!
Does the following patch fix it for you?

--D

From: Darrick J. Wong <djwong@kernel.org>
Subject: [PATCH] xfs_db: fix multiple dblock commands

Tom Samstag reported that running the following sequence of commands no
longer works quite right:

> inode [inodenum]
> dblock 0
> p
> dblock 1
> p
> dblock 2
> p
> [etc]

Mr. Samstag looked into the source code and discovered that the
dblock_f is incorrectly accessing iocur_top->data outside of the
push_cur -> set_cur_inode -> pop_cur sequence that this function uses to
compute the type of the file data.  In other words, it's using
whatever's on top of the stack at the start of the function.  For the
"dblock 0" case above this is the inode, but for the "dblock 1" case
this is the contents of file data block 0, not an inode.

Fix this by relocating the check to the correct place.

Reported-by: tom.samstag@netrise.io
Cc: <linux-xfs@vger.kernel.org> # v6.12.0
Fixes: b05a31722f5d4c ("xfs_db: access realtime file blocks")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 db/block.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/db/block.c b/db/block.c
index 00830a3d57e1df..2f1978c41f3094 100644
--- a/db/block.c
+++ b/db/block.c
@@ -246,6 +246,7 @@ dblock_f(
 	int		nb;
 	xfs_extnum_t	nex;
 	char		*p;
+	bool		isrt;
 	typnm_t		type;
 
 	bno = (xfs_fileoff_t)strtoull(argv[1], &p, 0);
@@ -255,6 +256,7 @@ dblock_f(
 	}
 	push_cur();
 	set_cur_inode(iocur_top->ino);
+	isrt = is_rtfile(iocur_top->data);
 	type = inode_next_type();
 	pop_cur();
 	if (type == TYP_NONE) {
@@ -273,7 +275,7 @@ dblock_f(
 	ASSERT(typtab[type].typnm == type);
 	if (nex > 1)
 		make_bbmap(&bbmap, nex, bmp);
-	if (is_rtfile(iocur_top->data))
+	if (isrt)
 		set_rt_cur(&typtab[type], xfs_rtb_to_daddr(mp, dfsbno),
 				nb * blkbb, DB_RING_ADD,
 				nex > 1 ? &bbmap : NULL);