All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: cen zhang <zzzccc427@gmail.com>
Cc: cem@kernel.org, linux-xfs@vger.kernel.org,
	linux-kernel@vger.kernel.org, baijiaju1990@gmail.com,
	zhenghaoran154@gmail.com
Subject: Re: Subject: [BUG] Five data races in in XFS Filesystem,one potentially harmful
Date: Wed, 14 May 2025 06:17:15 -0700	[thread overview]
Message-ID: <aCSX29o_xGAUWtIl@infradead.org> (raw)
In-Reply-To: <CAFRLqsVtQ0CY-6gGCafMBJ1ORyrZtRiPUzsfwA2uNjOdfLHPLw@mail.gmail.com>

> 1. Race in `xfs_bmapi_reserve_delalloc()` and  `xfs_vn_getattr()`
> ----------------------------------------------------------------
> 
> A data race on `ip->i_delayed_blks`.

This is indeed a case for data_race as getattr is just reporting without any
locks.  Can you send a patch?

> 2. Race on `xfs_trans_ail_update_bulk` in `xfs_inode_item_format`
> -------------------------------------.
> 
> We observed unsynchronized access to `lip->li_lsn`, which may exhibit
> store/load tearing. However, we did not observe any symptoms
> indicating harmful behavior.

I think we'll need READ_ONCE/WRITE_ONCE here to be safe on 64-bit
systems to avoid tearing/reordering.  But that still won't help
with 32-bit systems.  Other lsn fields use an atomic64_t for, which
is pretty heavy-handed.

> Function: xfs_alloc_longest_free_extent+0x164/0x580

> Function: xfs_alloc_update_counters+0x238/0x720 fs/xfs/libxfs/xfs_alloc.c:908

Both of these should be called with b_sema held.  Does your tool
treat a semaphore with an initial count of 1 as a lock?  That is still
a pattern in Linux as mutexes don't allow non-owner unlocks.

> 4. Race on `pag->pagf_longest`
> ------------------------------
> 
> Similar to the previous race, this field appears to be safely used
> under current access patterns.

Same here I think.
> 
> Possibly Harmful Race
> ======================
> 
> 1. Race on `bp->b_addr` between `xfs_buf_map_pages()` and `xfs_buf_offset()`
> ----------------------------------------------------------------------------

And I think this is another case of b_sema synchronization unless I'm
missing something.


  parent reply	other threads:[~2025-05-14 13:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-13 12:25 Subject: [BUG] Five data races in in XFS Filesystem,one potentially harmful cen zhang
2025-05-13 21:56 ` Dave Chinner
2025-05-14 13:17 ` Christoph Hellwig [this message]
2025-05-14 22:16   ` Dave Chinner

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=aCSX29o_xGAUWtIl@infradead.org \
    --to=hch@infradead.org \
    --cc=baijiaju1990@gmail.com \
    --cc=cem@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=zhenghaoran154@gmail.com \
    --cc=zzzccc427@gmail.com \
    /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.