Tested-by: Tom Samstag <tom.samstag@xxxxxxxxxx> On Thu, Jan 2, 2025 at 3:20 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote: > > 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@xxxxxxxxxx> > 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@xxxxxxxxxx > Cc: <linux-xfs@xxxxxxxxxxxxxxx> # v6.12.0 > Fixes: b05a31722f5d4c ("xfs_db: access realtime file blocks") > Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx> > --- > 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);
