[PATCH 1/2] mkfs,xfs_repair: don't pass a daddr as the flags argument

[PATCH 1/2] mkfs,xfs_repair: don't pass a daddr as the flags argument

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

libxfs_buf_get_uncached doesn't take a daddr argument, so don't pass one
as the flags argument.  Also take the opportunity to use
xfs_buf_set_daddr to set the actual disk address.

Cc: <linux-xfs@vger.kernel.org> # v6.13.0
Fixes: 0d7c490474e5e5 ("mkfs: format realtime groups")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 mkfs/xfs_mkfs.c |    5 ++---
 repair/rt.c     |    4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f5556fcc4040ed..79ce68e96bd2a5 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -4989,15 +4989,14 @@ write_rtsb(
 	}
 
 	error = -libxfs_buf_get_uncached(mp->m_rtdev_targp,
-				XFS_FSB_TO_BB(mp, 1), XFS_RTSB_DADDR,
-				&rtsb_bp);
+				XFS_FSB_TO_BB(mp, 1), 0, &rtsb_bp);
 	if (error) {
 		fprintf(stderr,
  _("%s: couldn't grab realtime superblock buffer\n"), progname);
 			exit(1);
 	}
 
-	rtsb_bp->b_maps[0].bm_bn = XFS_RTSB_DADDR;
+	xfs_buf_set_daddr(rtsb_bp, XFS_RTSB_DADDR);
 	rtsb_bp->b_ops = &xfs_rtsb_buf_ops;
 
 	libxfs_update_rtsb(rtsb_bp, sb_bp);
diff --git a/repair/rt.c b/repair/rt.c
index 5ba04919bc3ccf..12cc9bb8a88aeb 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -616,12 +616,12 @@ rewrite_rtsb(
  _("couldn't grab primary sb to update realtime sb\n"));
 
 	error = -libxfs_buf_get_uncached(mp->m_rtdev_targp,
-			XFS_FSB_TO_BB(mp, 1), XFS_RTSB_DADDR, &rtsb_bp);
+			XFS_FSB_TO_BB(mp, 1), 0, &rtsb_bp);
 	if (error)
 		do_error(
  _("couldn't grab realtime superblock\n"));
 
-	rtsb_bp->b_maps[0].bm_bn = XFS_RTSB_DADDR;
+	xfs_buf_set_daddr(rtsb_bp, XFS_RTSB_DADDR);
 	rtsb_bp->b_ops = &xfs_rtsb_buf_ops;
 
 	libxfs_update_rtsb(rtsb_bp, sb_bp);

[PATCH 2/2] xfs_db: obfuscate rt superblock label when metadumping

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Metadump can obfuscate the filesystem label on all the superblocks on
the data device, so it must perform the same transformation on the
realtime device superblock to avoid leaking information and so that the
mdrestored filesystem is consistent.

Found by running xfs/503 with realtime turned on and a patch to set
labels on common/populated filesystem images.

Cc: <linux-xfs@vger.kernel.org> # v6.13.0
Fixes: 6bc20c5edbab51 ("xfs_db: metadump realtime devices")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 db/metadump.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/db/metadump.c b/db/metadump.c
index 4f4b4f8a39a551..4d090942bf29cd 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -3073,6 +3073,17 @@ copy_rtsb(void)
 		print_warning("cannot read realtime superblock");
 		return !metadump.stop_on_read_error;
 	}
+
+	/* Replace any filesystem label with "L's" */
+	if (metadump.obfuscate) {
+		struct xfs_rtsb	*rtsb = iocur_top->data;
+
+		memset(rtsb->rsb_fname, 'L',
+		       min(strlen((char *)rtsb->rsb_fname),
+				sizeof(rtsb->rsb_fname)));
+		iocur_top->need_crc = 1;
+	}
+
 	error = write_buf(iocur_top);
 	pop_cur();
 

Re: [PATCH 2/2] xfs_db: obfuscate rt superblock label when metadumping

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] mkfs,xfs_repair: don't pass a daddr as the flags argument

[Christoph Hellwig]

On Tue, Feb 18, 2025 at 08:08:13PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> libxfs_buf_get_uncached doesn't take a daddr argument, so don't pass one
> as the flags argument.  Also take the opportunity to use
> xfs_buf_set_daddr to set the actual disk address.

Should it take a daddr argument?  I've been wondering that a bit as the
interface that doesn't pass one seems a bit odd.

The patch itself looks fine, although I don't really see the point in
the xfsprogs-only xfs_buf_set_daddr (including the current two callers).

Re: [PATCH 1/2] mkfs,xfs_repair: don't pass a daddr as the flags argument

[Darrick J. Wong]

On Wed, Feb 19, 2025 at 06:37:17AM +0100, Christoph Hellwig wrote:
> On Tue, Feb 18, 2025 at 08:08:13PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > libxfs_buf_get_uncached doesn't take a daddr argument, so don't pass one
> > as the flags argument.  Also take the opportunity to use
> > xfs_buf_set_daddr to set the actual disk address.
> 
> Should it take a daddr argument?  I've been wondering that a bit as the
> interface that doesn't pass one seems a bit odd.

We'd have to change the kernel first, there are libxfs callers with that
signature.

> The patch itself looks fine, although I don't really see the point in
> the xfsprogs-only xfs_buf_set_daddr (including the current two callers).

Eh, yeah.  Want me to resend with those bits cut out?

--D

Re: [PATCH 1/2] mkfs,xfs_repair: don't pass a daddr as the flags argument

[Christoph Hellwig]

On Tue, Feb 18, 2025 at 09:45:15PM -0800, Darrick J. Wong wrote:
> > The patch itself looks fine, although I don't really see the point in
> > the xfsprogs-only xfs_buf_set_daddr (including the current two callers).
> 
> Eh, yeah.  Want me to resend with those bits cut out?

As long as the helper is around there's probably no reason not to use
it.  Removing it would probably pair pretty well with passing a daddr
to xfs_get_buf_uncached.  Or maybe killing xfs_{get,read}_buf_uncached
entirely in favor of just using xfs_buf_oneshot more..

Re: [PATCH 1/2] mkfs,xfs_repair: don't pass a daddr as the flags argument

[Darrick J. Wong]

On Wed, Feb 19, 2025 at 06:48:51AM +0100, Christoph Hellwig wrote:
> On Tue, Feb 18, 2025 at 09:45:15PM -0800, Darrick J. Wong wrote:
> > > The patch itself looks fine, although I don't really see the point in
> > > the xfsprogs-only xfs_buf_set_daddr (including the current two callers).
> > 
> > Eh, yeah.  Want me to resend with those bits cut out?
> 
> As long as the helper is around there's probably no reason not to use
> it.  Removing it would probably pair pretty well with passing a daddr
> to xfs_get_buf_uncached.  Or maybe killing xfs_{get,read}_buf_uncached
> entirely in favor of just using xfs_buf_oneshot more..

<nod> I think I'd rather rid of it entirely and fix the _uncached API to
take a daddr.  It's not like we can't pass in DADDR_NULL if we *really*
don't know where it's going.

--D

[PATCH v1.1 1/2] mkfs,xfs_repair: don't pass a daddr as the flags argument

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

libxfs_buf_get_uncached doesn't take a daddr argument, so don't pass one
as the flags argument.

Cc: <linux-xfs@vger.kernel.org> # v6.13.0
Fixes: 0d7c490474e5e5 ("mkfs: format realtime groups")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
v1.1: drop the xfs_buf_set_addr change
---
 mkfs/xfs_mkfs.c |    3 +--
 repair/rt.c     |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f5556fcc4040ed..86bc9865b1071e 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -4989,8 +4989,7 @@ write_rtsb(
 	}
 
 	error = -libxfs_buf_get_uncached(mp->m_rtdev_targp,
-				XFS_FSB_TO_BB(mp, 1), XFS_RTSB_DADDR,
-				&rtsb_bp);
+				XFS_FSB_TO_BB(mp, 1), 0, &rtsb_bp);
 	if (error) {
 		fprintf(stderr,
  _("%s: couldn't grab realtime superblock buffer\n"), progname);
diff --git a/repair/rt.c b/repair/rt.c
index 5ba04919bc3ccf..e0a4943ee3b766 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -616,7 +616,7 @@ rewrite_rtsb(
  _("couldn't grab primary sb to update realtime sb\n"));
 
 	error = -libxfs_buf_get_uncached(mp->m_rtdev_targp,
-			XFS_FSB_TO_BB(mp, 1), XFS_RTSB_DADDR, &rtsb_bp);
+			XFS_FSB_TO_BB(mp, 1), 0, &rtsb_bp);
 	if (error)
 		do_error(
  _("couldn't grab realtime superblock\n"));

Re: [PATCH v1.1 1/2] mkfs,xfs_repair: don't pass a daddr as the flags argument

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>