[PATCH] xfs_logprint: fix pointer bug

[PATCH] xfs_logprint: fix pointer bug

[Darrick J. Wong]

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

generic/055 captures a crash in xfs_logprint due to an incorrect
refactoring trying to increment a pointer-to-pointer whereas before it
incremented a pointer.

Fixes: 5a9b7e95140893 ("logprint: factor out a xlog_print_op helper")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 logprint/log_misc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index 16353ebd728f35..8e0589c161b871 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -992,7 +992,7 @@ xlog_print_op(
 			printf("0x%02x ", (unsigned int)**ptr);
 			if (n % 16 == 15)
 				printf("\n");
-			ptr++;
+			(*ptr)++;
 		}
 		printf("\n");
 		return true;

Re: [PATCH] xfs_logprint: fix pointer bug

[Christoph Hellwig]

On Tue, Dec 09, 2025 at 12:57:38PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> generic/055 captures a crash in xfs_logprint due to an incorrect
> refactoring trying to increment a pointer-to-pointer whereas before it
> incremented a pointer.

Oops, sorry.  This somehow did not show up in xfstests for me, where
did you run into it?

Looks good:

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

Re: [PATCH] xfs_logprint: fix pointer bug

[Darrick J. Wong]

On Tue, Dec 09, 2025 at 09:25:51PM -0800, Christoph Hellwig wrote:
> On Tue, Dec 09, 2025 at 12:57:38PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > generic/055 captures a crash in xfs_logprint due to an incorrect
> > refactoring trying to increment a pointer-to-pointer whereas before it
> > incremented a pointer.
> 
> Oops, sorry.  This somehow did not show up in xfstests for me, where
> did you run into it?

FSTYP         -- xfs (debug)
PLATFORM      -- Linux/x86_64 ca-nfsdev5-mtr00 6.18.0-xfsx #6 SMP PREEMPT_DYNAMIC Tue Dec  9 10:29:14 PST 2025
MKFS_OPTIONS  -- -f -m metadir=1,autofsck=1,uquota,gquota,pquota, -n size=8k, /dev/sdf
MOUNT_OPTIONS -- /dev/sdf /opt

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

Full log here
https://djwong.org/fstests/output/ca-nfsdev5-mtr00/ca-nfsdev5-mtr00_2025-12-09_18-36-11/generic_055.full.br

user aibots
pass suck

--D

Re: [PATCH] xfs_logprint: fix pointer bug

[Christoph Hellwig]

On Tue, Dec 09, 2025 at 10:26:20PM -0800, Darrick J. Wong wrote:
> On Tue, Dec 09, 2025 at 09:25:51PM -0800, Christoph Hellwig wrote:
> > On Tue, Dec 09, 2025 at 12:57:38PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > generic/055 captures a crash in xfs_logprint due to an incorrect
> > > refactoring trying to increment a pointer-to-pointer whereas before it
> > > incremented a pointer.
> > 
> > Oops, sorry.  This somehow did not show up in xfstests for me, where
> > did you run into it?
> 
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 ca-nfsdev5-mtr00 6.18.0-xfsx #6 SMP PREEMPT_DYNAMIC Tue Dec  9 10:29:14 PST 2025
> MKFS_OPTIONS  -- -f -m metadir=1,autofsck=1,uquota,gquota,pquota, -n size=8k, /dev/sdf
> MOUNT_OPTIONS -- /dev/sdf /opt
> 
> > Looks good:
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Full log here
> https://djwong.org/fstests/output/ca-nfsdev5-mtr00/ca-nfsdev5-mtr00_2025-12-09_18-36-11/generic_055.full.br

Hmm, generic/044...

> user aibots
> pass suck

?

Re: [PATCH] xfs_logprint: fix pointer bug

[Andrey Albershteyn]

On 2025-12-09 12:57:38, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> generic/055 captures a crash in xfs_logprint due to an incorrect
> refactoring trying to increment a pointer-to-pointer whereas before it
> incremented a pointer.
> 
> Fixes: 5a9b7e95140893 ("logprint: factor out a xlog_print_op helper")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  logprint/log_misc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/logprint/log_misc.c b/logprint/log_misc.c
> index 16353ebd728f35..8e0589c161b871 100644
> --- a/logprint/log_misc.c
> +++ b/logprint/log_misc.c
> @@ -992,7 +992,7 @@ xlog_print_op(
>  			printf("0x%02x ", (unsigned int)**ptr);
>  			if (n % 16 == 15)
>  				printf("\n");
> -			ptr++;
> +			(*ptr)++;
>  		}
>  		printf("\n");
>  		return true;
> 

Hmm, checking the results I also see the segfauls in 055.full but
test is passing. Is there any xfstests setting to make it fail on
coredumps/segfaults?

-- 
- Andrey

Re: [PATCH] xfs_logprint: fix pointer bug

[Darrick J. Wong]

On Wed, Dec 10, 2025 at 12:10:08PM +0100, Andrey Albershteyn wrote:
> On 2025-12-09 12:57:38, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > generic/055 captures a crash in xfs_logprint due to an incorrect
> > refactoring trying to increment a pointer-to-pointer whereas before it
> > incremented a pointer.
> > 
> > Fixes: 5a9b7e95140893 ("logprint: factor out a xlog_print_op helper")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  logprint/log_misc.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/logprint/log_misc.c b/logprint/log_misc.c
> > index 16353ebd728f35..8e0589c161b871 100644
> > --- a/logprint/log_misc.c
> > +++ b/logprint/log_misc.c
> > @@ -992,7 +992,7 @@ xlog_print_op(
> >  			printf("0x%02x ", (unsigned int)**ptr);
> >  			if (n % 16 == 15)
> >  				printf("\n");
> > -			ptr++;
> > +			(*ptr)++;
> >  		}
> >  		printf("\n");
> >  		return true;
> > 
> 
> Hmm, checking the results I also see the segfauls in 055.full but
> test is passing. Is there any xfstests setting to make it fail on
> coredumps/segfaults?

Well in theory it already captures coredumps ... if they're named "core"
and get written to $here.  But then coredumpctl intervenes and whisks
them all away so fstests never sees them.

Starting Nov 2025, fstests can query coredumps from systemd:
https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=9886baabdec372387b5e874fdcaf59390a75f4a9

though obviously this isn't compatible with check-parallel because
there's no good way to tag a process tree for coredumpctl so that you
can query dumps only from that tree later.

--D

> -- 
> - Andrey
> 
>