* fix cross-platform log CRC validation
@ 2025-09-15 13:20 Christoph Hellwig
2025-09-15 13:20 ` [PATCH 1/2] xfs: rename the old_crc variable in xlog_recover_process Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Christoph Hellwig @ 2025-09-15 13:20 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: Darrick J. Wong, linux-xfs
Hi all,
this series fixes recovery of logs written on i386 on other
architectures or vice versa.
Diffstat:
libxfs/xfs_log_format.h | 30 +++++++++++++++++++++++++++++-
libxfs/xfs_ondisk.h | 2 ++
xfs_log.c | 8 ++++----
xfs_log_priv.h | 4 ++--
xfs_log_recover.c | 34 ++++++++++++++++++++++++----------
5 files changed, 61 insertions(+), 17 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] xfs: rename the old_crc variable in xlog_recover_process 2025-09-15 13:20 fix cross-platform log CRC validation Christoph Hellwig @ 2025-09-15 13:20 ` Christoph Hellwig 2025-09-15 18:25 ` Darrick J. Wong 2025-09-15 13:20 ` [PATCH 2/2] xfs: fix log CRC mismatches between i386 and other architectures Christoph Hellwig 2025-09-16 11:34 ` fix cross-platform log CRC validation Carlos Maiolino 2 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2025-09-15 13:20 UTC (permalink / raw) To: Carlos Maiolino; +Cc: Darrick J. Wong, linux-xfs old_crc is a very misleading name. Rename it to expected_crc as that described the usage much better. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_log_recover.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index e6ed9e09c027..0a4db8efd903 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2894,20 +2894,19 @@ xlog_recover_process( int pass, struct list_head *buffer_list) { - __le32 old_crc = rhead->h_crc; - __le32 crc; + __le32 expected_crc = rhead->h_crc, crc; crc = xlog_cksum(log, rhead, dp, be32_to_cpu(rhead->h_len)); /* * Nothing else to do if this is a CRC verification pass. Just return * if this a record with a non-zero crc. Unfortunately, mkfs always - * sets old_crc to 0 so we must consider this valid even on v5 supers. - * Otherwise, return EFSBADCRC on failure so the callers up the stack - * know precisely what failed. + * sets expected_crc to 0 so we must consider this valid even on v5 + * supers. Otherwise, return EFSBADCRC on failure so the callers up the + * stack know precisely what failed. */ if (pass == XLOG_RECOVER_CRCPASS) { - if (old_crc && crc != old_crc) + if (expected_crc && crc != expected_crc) return -EFSBADCRC; return 0; } @@ -2918,11 +2917,11 @@ xlog_recover_process( * zero CRC check prevents warnings from being emitted when upgrading * the kernel from one that does not add CRCs by default. */ - if (crc != old_crc) { - if (old_crc || xfs_has_crc(log->l_mp)) { + if (crc != expected_crc) { + if (expected_crc || xfs_has_crc(log->l_mp)) { xfs_alert(log->l_mp, "log record CRC mismatch: found 0x%x, expected 0x%x.", - le32_to_cpu(old_crc), + le32_to_cpu(expected_crc), le32_to_cpu(crc)); xfs_hex_dump(dp, 32); } -- 2.47.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] xfs: rename the old_crc variable in xlog_recover_process 2025-09-15 13:20 ` [PATCH 1/2] xfs: rename the old_crc variable in xlog_recover_process Christoph Hellwig @ 2025-09-15 18:25 ` Darrick J. Wong 0 siblings, 0 replies; 8+ messages in thread From: Darrick J. Wong @ 2025-09-15 18:25 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs On Mon, Sep 15, 2025 at 06:20:29AM -0700, Christoph Hellwig wrote: > old_crc is a very misleading name. Rename it to expected_crc as that > described the usage much better. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Pretty straightforward so Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > fs/xfs/xfs_log_recover.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index e6ed9e09c027..0a4db8efd903 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -2894,20 +2894,19 @@ xlog_recover_process( > int pass, > struct list_head *buffer_list) > { > - __le32 old_crc = rhead->h_crc; > - __le32 crc; > + __le32 expected_crc = rhead->h_crc, crc; > > crc = xlog_cksum(log, rhead, dp, be32_to_cpu(rhead->h_len)); > > /* > * Nothing else to do if this is a CRC verification pass. Just return > * if this a record with a non-zero crc. Unfortunately, mkfs always > - * sets old_crc to 0 so we must consider this valid even on v5 supers. > - * Otherwise, return EFSBADCRC on failure so the callers up the stack > - * know precisely what failed. > + * sets expected_crc to 0 so we must consider this valid even on v5 > + * supers. Otherwise, return EFSBADCRC on failure so the callers up the > + * stack know precisely what failed. > */ > if (pass == XLOG_RECOVER_CRCPASS) { > - if (old_crc && crc != old_crc) > + if (expected_crc && crc != expected_crc) > return -EFSBADCRC; > return 0; > } > @@ -2918,11 +2917,11 @@ xlog_recover_process( > * zero CRC check prevents warnings from being emitted when upgrading > * the kernel from one that does not add CRCs by default. > */ > - if (crc != old_crc) { > - if (old_crc || xfs_has_crc(log->l_mp)) { > + if (crc != expected_crc) { > + if (expected_crc || xfs_has_crc(log->l_mp)) { > xfs_alert(log->l_mp, > "log record CRC mismatch: found 0x%x, expected 0x%x.", > - le32_to_cpu(old_crc), > + le32_to_cpu(expected_crc), > le32_to_cpu(crc)); > xfs_hex_dump(dp, 32); > } > -- > 2.47.2 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] xfs: fix log CRC mismatches between i386 and other architectures 2025-09-15 13:20 fix cross-platform log CRC validation Christoph Hellwig 2025-09-15 13:20 ` [PATCH 1/2] xfs: rename the old_crc variable in xlog_recover_process Christoph Hellwig @ 2025-09-15 13:20 ` Christoph Hellwig 2025-09-15 18:25 ` Darrick J. Wong 2025-09-16 11:34 ` fix cross-platform log CRC validation Carlos Maiolino 2 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2025-09-15 13:20 UTC (permalink / raw) To: Carlos Maiolino; +Cc: Darrick J. Wong, linux-xfs When mounting file systems with a log that was dirtied on i386 on other architectures or vice versa, log recovery is unhappy: [ 11.068052] XFS (vdb): Torn write (CRC failure) detected at log block 0x2. Truncating head block from 0xc. This is because the CRCs generated by i386 and other architectures always diff. The reason for that is that sizeof(struct xlog_rec_header) returns different values for i386 vs the rest (324 vs 328), because the struct is not sizeof(uint64_t) aligned, and i386 has odd struct size alignment rules. This issue goes back to commit 13cdc853c519 ("Add log versioning, and new super block field for the log stripe") in the xfs-import tree, which adds log v2 support and the h_size field that causes the unaligned size. At that time it only mattered for the crude debug only log header checksum, but with commit 0e446be44806 ("xfs: add CRC checks to the log") it became a real issue for v5 file system, because now there is a proper CRC, and regular builds actually expect it match. Fix this by allowing checksums with and without the padding. Fixes: 0e446be44806 ("xfs: add CRC checks to the log") Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/libxfs/xfs_log_format.h | 30 +++++++++++++++++++++++++++++- fs/xfs/libxfs/xfs_ondisk.h | 2 ++ fs/xfs/xfs_log.c | 8 ++++---- fs/xfs/xfs_log_priv.h | 4 ++-- fs/xfs/xfs_log_recover.c | 19 +++++++++++++++++-- 5 files changed, 54 insertions(+), 9 deletions(-) diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h index 0d637c276db0..942c490f23e4 100644 --- a/fs/xfs/libxfs/xfs_log_format.h +++ b/fs/xfs/libxfs/xfs_log_format.h @@ -174,12 +174,40 @@ typedef struct xlog_rec_header { __be32 h_prev_block; /* block number to previous LR : 4 */ __be32 h_num_logops; /* number of log operations in this LR : 4 */ __be32 h_cycle_data[XLOG_HEADER_CYCLE_SIZE / BBSIZE]; - /* new fields */ + + /* fields added by the Linux port: */ __be32 h_fmt; /* format of log record : 4 */ uuid_t h_fs_uuid; /* uuid of FS : 16 */ + + /* fields added for log v2: */ __be32 h_size; /* iclog size : 4 */ + + /* + * When h_size added for log v2 support, it caused structure to have + * a different size on i386 vs all other architectures because the + * sum of the size ofthe member is not aligned by that of the largest + * __be64-sized member, and i386 has really odd struct alignment rules. + * + * Due to the way the log headers are placed out on-disk that alone is + * not a problem becaue the xlog_rec_header always sits alone in a + * BBSIZEs area, and the rest of that area is padded with zeroes. + * But xlog_cksum used to calculate the checksum based on the structure + * size, and thus gives different checksums for i386 vs the rest. + * We now do two checksum validation passes for both sizes to allow + * moving v5 file systems with unclean logs between i386 and other + * (little-endian) architectures. + */ + __u32 h_pad0; } xlog_rec_header_t; +#ifdef __i386__ +#define XLOG_REC_SIZE offsetofend(struct xlog_rec_header, h_size) +#define XLOG_REC_SIZE_OTHER sizeof(struct xlog_rec_header) +#else +#define XLOG_REC_SIZE sizeof(struct xlog_rec_header) +#define XLOG_REC_SIZE_OTHER offsetofend(struct xlog_rec_header, h_size) +#endif /* __i386__ */ + typedef struct xlog_rec_ext_header { __be32 xh_cycle; /* write cycle of log : 4 */ __be32 xh_cycle_data[XLOG_HEADER_CYCLE_SIZE / BBSIZE]; /* : 256 */ diff --git a/fs/xfs/libxfs/xfs_ondisk.h b/fs/xfs/libxfs/xfs_ondisk.h index 5ed44fdf7491..7bfa3242e2c5 100644 --- a/fs/xfs/libxfs/xfs_ondisk.h +++ b/fs/xfs/libxfs/xfs_ondisk.h @@ -174,6 +174,8 @@ xfs_check_ondisk_structs(void) XFS_CHECK_STRUCT_SIZE(struct xfs_rud_log_format, 16); XFS_CHECK_STRUCT_SIZE(struct xfs_map_extent, 32); XFS_CHECK_STRUCT_SIZE(struct xfs_phys_extent, 16); + XFS_CHECK_STRUCT_SIZE(struct xlog_rec_header, 328); + XFS_CHECK_STRUCT_SIZE(struct xlog_rec_ext_header, 260); XFS_CHECK_OFFSET(struct xfs_bui_log_format, bui_extents, 16); XFS_CHECK_OFFSET(struct xfs_cui_log_format, cui_extents, 16); diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index c8a57e21a1d3..69703dc3ef94 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -1568,13 +1568,13 @@ xlog_cksum( struct xlog *log, struct xlog_rec_header *rhead, char *dp, - int size) + unsigned int hdrsize, + unsigned int size) { uint32_t crc; /* first generate the crc for the record header ... */ - crc = xfs_start_cksum_update((char *)rhead, - sizeof(struct xlog_rec_header), + crc = xfs_start_cksum_update((char *)rhead, hdrsize, offsetof(struct xlog_rec_header, h_crc)); /* ... then for additional cycle data for v2 logs ... */ @@ -1818,7 +1818,7 @@ xlog_sync( /* calculcate the checksum */ iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header, - iclog->ic_datap, size); + iclog->ic_datap, XLOG_REC_SIZE, size); /* * Intentionally corrupt the log record CRC based on the error injection * frequency, if defined. This facilitates testing log recovery in the diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h index a9a7a271c15b..0cfc654d8e87 100644 --- a/fs/xfs/xfs_log_priv.h +++ b/fs/xfs/xfs_log_priv.h @@ -499,8 +499,8 @@ xlog_recover_finish( extern void xlog_recover_cancel(struct xlog *); -extern __le32 xlog_cksum(struct xlog *log, struct xlog_rec_header *rhead, - char *dp, int size); +__le32 xlog_cksum(struct xlog *log, struct xlog_rec_header *rhead, + char *dp, unsigned int hdrsize, unsigned int size); extern struct kmem_cache *xfs_log_ticket_cache; struct xlog_ticket *xlog_ticket_alloc(struct xlog *log, int unit_bytes, diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 0a4db8efd903..549d60959aee 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2894,9 +2894,24 @@ xlog_recover_process( int pass, struct list_head *buffer_list) { - __le32 expected_crc = rhead->h_crc, crc; + __le32 expected_crc = rhead->h_crc, crc, other_crc; - crc = xlog_cksum(log, rhead, dp, be32_to_cpu(rhead->h_len)); + crc = xlog_cksum(log, rhead, dp, XLOG_REC_SIZE, + be32_to_cpu(rhead->h_len)); + + /* + * Look at the end of the struct xlog_rec_header definition in + * xfs_log_format.h for the glory details. + */ + if (expected_crc && crc != expected_crc) { + other_crc = xlog_cksum(log, rhead, dp, XLOG_REC_SIZE_OTHER, + be32_to_cpu(rhead->h_len)); + if (other_crc == expected_crc) { + xfs_notice_once(log->l_mp, + "Fixing up incorrect CRC due to padding."); + crc = other_crc; + } + } /* * Nothing else to do if this is a CRC verification pass. Just return -- 2.47.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] xfs: fix log CRC mismatches between i386 and other architectures 2025-09-15 13:20 ` [PATCH 2/2] xfs: fix log CRC mismatches between i386 and other architectures Christoph Hellwig @ 2025-09-15 18:25 ` Darrick J. Wong 2025-09-15 20:50 ` Christoph Hellwig 0 siblings, 1 reply; 8+ messages in thread From: Darrick J. Wong @ 2025-09-15 18:25 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs On Mon, Sep 15, 2025 at 06:20:30AM -0700, Christoph Hellwig wrote: > When mounting file systems with a log that was dirtied on i386 on > other architectures or vice versa, log recovery is unhappy: > > [ 11.068052] XFS (vdb): Torn write (CRC failure) detected at log block 0x2. Truncating head block from 0xc. > > This is because the CRCs generated by i386 and other architectures > always diff. The reason for that is that sizeof(struct xlog_rec_header) > returns different values for i386 vs the rest (324 vs 328), because the > struct is not sizeof(uint64_t) aligned, and i386 has odd struct size > alignment rules. ...and let me guess, the checksum function samples data all the way out to byte 324/328 too? > This issue goes back to commit 13cdc853c519 ("Add log versioning, and new > super block field for the log stripe") in the xfs-import tree, which > adds log v2 support and the h_size field that causes the unaligned size. > At that time it only mattered for the crude debug only log header > checksum, but with commit 0e446be44806 ("xfs: add CRC checks to the log") > it became a real issue for v5 file system, because now there is a proper > CRC, and regular builds actually expect it match. > > Fix this by allowing checksums with and without the padding. > > Fixes: 0e446be44806 ("xfs: add CRC checks to the log") Cc: <stable@vger.kernel.org> # v3.8 Perhaps? This seems like a serious tripping point for old kernels. > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/libxfs/xfs_log_format.h | 30 +++++++++++++++++++++++++++++- > fs/xfs/libxfs/xfs_ondisk.h | 2 ++ > fs/xfs/xfs_log.c | 8 ++++---- > fs/xfs/xfs_log_priv.h | 4 ++-- > fs/xfs/xfs_log_recover.c | 19 +++++++++++++++++-- > 5 files changed, 54 insertions(+), 9 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h > index 0d637c276db0..942c490f23e4 100644 > --- a/fs/xfs/libxfs/xfs_log_format.h > +++ b/fs/xfs/libxfs/xfs_log_format.h > @@ -174,12 +174,40 @@ typedef struct xlog_rec_header { > __be32 h_prev_block; /* block number to previous LR : 4 */ > __be32 h_num_logops; /* number of log operations in this LR : 4 */ > __be32 h_cycle_data[XLOG_HEADER_CYCLE_SIZE / BBSIZE]; > - /* new fields */ > + > + /* fields added by the Linux port: */ > __be32 h_fmt; /* format of log record : 4 */ > uuid_t h_fs_uuid; /* uuid of FS : 16 */ > + > + /* fields added for log v2: */ > __be32 h_size; /* iclog size : 4 */ > + > + /* > + * When h_size added for log v2 support, it caused structure to have > + * a different size on i386 vs all other architectures because the > + * sum of the size ofthe member is not aligned by that of the largest > + * __be64-sized member, and i386 has really odd struct alignment rules. > + * > + * Due to the way the log headers are placed out on-disk that alone is > + * not a problem becaue the xlog_rec_header always sits alone in a > + * BBSIZEs area, and the rest of that area is padded with zeroes. > + * But xlog_cksum used to calculate the checksum based on the structure > + * size, and thus gives different checksums for i386 vs the rest. > + * We now do two checksum validation passes for both sizes to allow > + * moving v5 file systems with unclean logs between i386 and other > + * (little-endian) architectures. Is this a problem on other 32-bit platforms? Or just i386? > + */ > + __u32 h_pad0; > } xlog_rec_header_t; > > +#ifdef __i386__ > +#define XLOG_REC_SIZE offsetofend(struct xlog_rec_header, h_size) > +#define XLOG_REC_SIZE_OTHER sizeof(struct xlog_rec_header) > +#else > +#define XLOG_REC_SIZE sizeof(struct xlog_rec_header) > +#define XLOG_REC_SIZE_OTHER offsetofend(struct xlog_rec_header, h_size) > +#endif /* __i386__ */ > + > typedef struct xlog_rec_ext_header { > __be32 xh_cycle; /* write cycle of log : 4 */ > __be32 xh_cycle_data[XLOG_HEADER_CYCLE_SIZE / BBSIZE]; /* : 256 */ > diff --git a/fs/xfs/libxfs/xfs_ondisk.h b/fs/xfs/libxfs/xfs_ondisk.h > index 5ed44fdf7491..7bfa3242e2c5 100644 > --- a/fs/xfs/libxfs/xfs_ondisk.h > +++ b/fs/xfs/libxfs/xfs_ondisk.h > @@ -174,6 +174,8 @@ xfs_check_ondisk_structs(void) > XFS_CHECK_STRUCT_SIZE(struct xfs_rud_log_format, 16); > XFS_CHECK_STRUCT_SIZE(struct xfs_map_extent, 32); > XFS_CHECK_STRUCT_SIZE(struct xfs_phys_extent, 16); > + XFS_CHECK_STRUCT_SIZE(struct xlog_rec_header, 328); > + XFS_CHECK_STRUCT_SIZE(struct xlog_rec_ext_header, 260); I guess we'll find out from the build bots. ;) The code changes looks ok modulo my various questions. --D > > XFS_CHECK_OFFSET(struct xfs_bui_log_format, bui_extents, 16); > XFS_CHECK_OFFSET(struct xfs_cui_log_format, cui_extents, 16); > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index c8a57e21a1d3..69703dc3ef94 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -1568,13 +1568,13 @@ xlog_cksum( > struct xlog *log, > struct xlog_rec_header *rhead, > char *dp, > - int size) > + unsigned int hdrsize, > + unsigned int size) > { > uint32_t crc; > > /* first generate the crc for the record header ... */ > - crc = xfs_start_cksum_update((char *)rhead, > - sizeof(struct xlog_rec_header), > + crc = xfs_start_cksum_update((char *)rhead, hdrsize, > offsetof(struct xlog_rec_header, h_crc)); > > /* ... then for additional cycle data for v2 logs ... */ > @@ -1818,7 +1818,7 @@ xlog_sync( > > /* calculcate the checksum */ > iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header, > - iclog->ic_datap, size); > + iclog->ic_datap, XLOG_REC_SIZE, size); > /* > * Intentionally corrupt the log record CRC based on the error injection > * frequency, if defined. This facilitates testing log recovery in the > diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h > index a9a7a271c15b..0cfc654d8e87 100644 > --- a/fs/xfs/xfs_log_priv.h > +++ b/fs/xfs/xfs_log_priv.h > @@ -499,8 +499,8 @@ xlog_recover_finish( > extern void > xlog_recover_cancel(struct xlog *); > > -extern __le32 xlog_cksum(struct xlog *log, struct xlog_rec_header *rhead, > - char *dp, int size); > +__le32 xlog_cksum(struct xlog *log, struct xlog_rec_header *rhead, > + char *dp, unsigned int hdrsize, unsigned int size); > > extern struct kmem_cache *xfs_log_ticket_cache; > struct xlog_ticket *xlog_ticket_alloc(struct xlog *log, int unit_bytes, > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index 0a4db8efd903..549d60959aee 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -2894,9 +2894,24 @@ xlog_recover_process( > int pass, > struct list_head *buffer_list) > { > - __le32 expected_crc = rhead->h_crc, crc; > + __le32 expected_crc = rhead->h_crc, crc, other_crc; > > - crc = xlog_cksum(log, rhead, dp, be32_to_cpu(rhead->h_len)); > + crc = xlog_cksum(log, rhead, dp, XLOG_REC_SIZE, > + be32_to_cpu(rhead->h_len)); > + > + /* > + * Look at the end of the struct xlog_rec_header definition in > + * xfs_log_format.h for the glory details. > + */ > + if (expected_crc && crc != expected_crc) { > + other_crc = xlog_cksum(log, rhead, dp, XLOG_REC_SIZE_OTHER, > + be32_to_cpu(rhead->h_len)); > + if (other_crc == expected_crc) { > + xfs_notice_once(log->l_mp, > + "Fixing up incorrect CRC due to padding."); > + crc = other_crc; > + } > + } > > /* > * Nothing else to do if this is a CRC verification pass. Just return > -- > 2.47.2 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] xfs: fix log CRC mismatches between i386 and other architectures 2025-09-15 18:25 ` Darrick J. Wong @ 2025-09-15 20:50 ` Christoph Hellwig 2025-09-16 10:26 ` Carlos Maiolino 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2025-09-15 20:50 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Christoph Hellwig, Carlos Maiolino, linux-xfs On Mon, Sep 15, 2025 at 11:25:13AM -0700, Darrick J. Wong wrote: > ...and let me guess, the checksum function samples data all the way out > to byte 324/328 too? Yes. > > Fixes: 0e446be44806 ("xfs: add CRC checks to the log") > > Cc: <stable@vger.kernel.org> # v3.8 Still not a fan of the explicit stable tag vs implying it by fixes, but yes, this should be backported. > > + * We now do two checksum validation passes for both sizes to allow > > + * moving v5 file systems with unclean logs between i386 and other > > + * (little-endian) architectures. > > Is this a problem on other 32-bit platforms? Or just i386? The alignment is an i386-specific quirk. We have similar workarounds for the extent structure in the EFI/EFD items and some 32-bit ioctls, except that this one can be a bit simpler. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] xfs: fix log CRC mismatches between i386 and other architectures 2025-09-15 20:50 ` Christoph Hellwig @ 2025-09-16 10:26 ` Carlos Maiolino 0 siblings, 0 replies; 8+ messages in thread From: Carlos Maiolino @ 2025-09-16 10:26 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Darrick J. Wong, linux-xfs On Mon, Sep 15, 2025 at 10:50:49PM +0200, Christoph Hellwig wrote: > On Mon, Sep 15, 2025 at 11:25:13AM -0700, Darrick J. Wong wrote: > > ...and let me guess, the checksum function samples data all the way out > > to byte 324/328 too? > > Yes. > > > > Fixes: 0e446be44806 ("xfs: add CRC checks to the log") > > > > Cc: <stable@vger.kernel.org> # v3.8 > > Still not a fan of the explicit stable tag vs implying it by fixes, > but yes, this should be backported. I'll add it here... > > > > + * We now do two checksum validation passes for both sizes to allow > > > + * moving v5 file systems with unclean logs between i386 and other > > > + * (little-endian) architectures. > > > > Is this a problem on other 32-bit platforms? Or just i386? > > The alignment is an i386-specific quirk. We have similar workarounds > for the extent structure in the EFI/EFD items and some 32-bit ioctls, > except that this one can be a bit simpler. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: fix cross-platform log CRC validation 2025-09-15 13:20 fix cross-platform log CRC validation Christoph Hellwig 2025-09-15 13:20 ` [PATCH 1/2] xfs: rename the old_crc variable in xlog_recover_process Christoph Hellwig 2025-09-15 13:20 ` [PATCH 2/2] xfs: fix log CRC mismatches between i386 and other architectures Christoph Hellwig @ 2025-09-16 11:34 ` Carlos Maiolino 2 siblings, 0 replies; 8+ messages in thread From: Carlos Maiolino @ 2025-09-16 11:34 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Darrick J. Wong, linux-xfs On Mon, 15 Sep 2025 06:20:28 -0700, Christoph Hellwig wrote: > this series fixes recovery of logs written on i386 on other > architectures or vice versa. > > Diffstat: > libxfs/xfs_log_format.h | 30 +++++++++++++++++++++++++++++- > libxfs/xfs_ondisk.h | 2 ++ > xfs_log.c | 8 ++++---- > xfs_log_priv.h | 4 ++-- > xfs_log_recover.c | 34 ++++++++++++++++++++++++---------- > 5 files changed, 61 insertions(+), 17 deletions(-) > > [...] Applied to for-next, thanks! [1/2] xfs: rename the old_crc variable in xlog_recover_process commit: 0b737f4ac1d3ec093347241df74bbf5f54a7e16c [2/2] xfs: fix log CRC mismatches between i386 and other architectures commit: e747883c7d7306acb4d683038d881528fbfbe749 Best regards, -- Carlos Maiolino <cem@kernel.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-16 11:34 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-15 13:20 fix cross-platform log CRC validation Christoph Hellwig 2025-09-15 13:20 ` [PATCH 1/2] xfs: rename the old_crc variable in xlog_recover_process Christoph Hellwig 2025-09-15 18:25 ` Darrick J. Wong 2025-09-15 13:20 ` [PATCH 2/2] xfs: fix log CRC mismatches between i386 and other architectures Christoph Hellwig 2025-09-15 18:25 ` Darrick J. Wong 2025-09-15 20:50 ` Christoph Hellwig 2025-09-16 10:26 ` Carlos Maiolino 2025-09-16 11:34 ` fix cross-platform log CRC validation Carlos Maiolino
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox