All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Stian Halseth <stian@itx.no>
Cc: Carlos Maiolino <cem@kernel.org>,
	linux-xfs@vger.kernel.org, "Darrick J . Wong" <djwong@kernel.org>,
	sparclinux@vger.kernel.org, Jan Engelhardt <jengelh@inai.de>,
	Dave Chinner <dgc@kernel.org>,
	stable@vger.kernel.org
Subject: Re: [PATCH v3] xfs: fix unaligned access to rmapbt high keys in scrub
Date: Wed, 11 Nov 2026 07:19:45 -0800	[thread overview]
Message-ID: <avSHkadYwEjnKF87@infradead.org> (raw)
In-Reply-To: <20261111103047.604541-1-stian@itx.no>

On Wed, Nov 11, 2026 at 11:30:47AM +0100, Stian Halseth wrote:
> The high keys in a rmapbt node block aren't naturally aligned.
> xchk_rmapbt_check_unwritten_in_keyflags() reads rm_offset through
> union xfs_btree_key and the union is 8-byte aligned.
> 
> The compiler may use an 8-byte load, and on sparc64 each high key
> takes an unaligned access trap:
> 
>   Kernel unaligned access at TPC[105a54d8] xchk_rmapbt_check_unwritten_in_keyflags+0xb8/0x160 [xfs]
> 
> Read the keys through the packed struct xfs_rmap_key instead. This
> tells the compiler they may be unaligned. Decode rm_offset with
> be64_to_cpu() and drop the byte-swapped flag.
> 
> Fixes: 38384569a2a8 ("xfs: detect unwritten bit set in rmapbt node block keys")
> Reported-by: Jan Engelhardt <jengelh@inai.de>
> Closes: https://github.com/sparclinux/issues/issues/115
> Suggested-by: Dave Chinner <dgc@kernel.org>
> Cc: stable@vger.kernel.org # v6.4+
> Signed-off-by: Stian Halseth <stian@itx.no>
> ---
> Tested on sparc64 (T4, 7.3.4) by scrubbing the rmapbt of a loop-mounted
> image: alignment faults per scrub went from about 4000 to 0.
> 
> v3: separate key and rkey variables plus a comment (Dave), shorter
> changelog.
> v2: https://lore.kernel.org/all/20261110223927.558557-1-stian@itx.no/
> v1: https://lore.kernel.org/all/20261109134853.1659991-1-stian@itx.no/
> 
>  fs/xfs/scrub/rmap.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/scrub/rmap.c b/fs/xfs/scrub/rmap.c
> index 68e2847c962bb..7609c03cdc8ab 100644
> --- a/fs/xfs/scrub/rmap.c
> +++ b/fs/xfs/scrub/rmap.c
> @@ -149,8 +149,6 @@ xchk_rmapbt_check_unwritten_in_keyflags(
>  	struct xfs_scrub	*sc = bs->sc;
>  	struct xfs_btree_cur	*cur = bs->cur;
>  	struct xfs_btree_block	*keyblock;
> -	union xfs_btree_key	*lkey, *hkey;
> -	__be64			badflag = cpu_to_be64(XFS_RMAP_OFF_UNWRITTEN);
>  	unsigned int		level;
>  
>  	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_PREEN)
> @@ -159,6 +157,14 @@ xchk_rmapbt_check_unwritten_in_keyflags(
>  	for (level = 1; level < cur->bc_nlevels; level++) {
>  		struct xfs_buf	*bp;
>  		unsigned int	ptr;
> +		/*
> +		 * The on-disk high keys in an rmap node block are not
> +		 * naturally aligned, so decode them through the packed
> +		 * struct xfs_rmap_key to make the compiler treat them as
> +		 * unaligned.
> +		 */
> +		union xfs_btree_key	*key;
> +		const struct xfs_rmap_key *rkey;
>  
>  		/* Only check the first time we've seen this node block. */
>  		if (cur->bc_levels[level].ptr > 1)
> @@ -166,15 +172,16 @@ xchk_rmapbt_check_unwritten_in_keyflags(
>  
>  		keyblock = xfs_btree_get_block(cur, level, &bp);
>  		for (ptr = 1; ptr <= be16_to_cpu(keyblock->bb_numrecs); ptr++) {
> -			lkey = xfs_btree_key_addr(cur, ptr, keyblock);
> -
> -			if (lkey->rmap.rm_offset & badflag) {
> +			key = xfs_btree_key_addr(cur, ptr, keyblock);
> +			rkey = &key->rmap;
> +			if (be64_to_cpu(rkey->rm_offset) & XFS_RMAP_OFF_UNWRITTEN) {

Overly long line.

But I wonder if we should simply mark struct xfs_rmap_key as __packed
to force the compliler to deal with unaligned fields instead of jumping
through these hoops at every call site.


  reply	other threads:[~2026-11-11 15:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-11 10:30 [PATCH v3] xfs: fix unaligned access to rmapbt high keys in scrub Stian Halseth
2026-11-11 15:19 ` Christoph Hellwig [this message]
2026-11-11 19:01   ` Stian Halseth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=avSHkadYwEjnKF87@infradead.org \
    --to=hch@infradead.org \
    --cc=cem@kernel.org \
    --cc=dgc@kernel.org \
    --cc=djwong@kernel.org \
    --cc=jengelh@inai.de \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stian@itx.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.