Re: [PATCH] ntfs: serialize the resident read iomap path with mrec_lock
From: Namjae Jeon
Date: Fri Jul 31 2026 - 11:06:49 EST
> diff --git a/fs/ntfs/iomap.c b/fs/ntfs/iomap.c
> index 52eecf5cb2..f9fdceeeb3 100644
> --- a/fs/ntfs/iomap.c
> +++ b/fs/ntfs/iomap.c
> @@ -95,6 +95,8 @@ static int ntfs_read_iomap_begin_resident(struct inode *inode, loff_t offset, lo
> else
> base_ni = ni;
>
> + mutex_lock(&base_ni->mrec_lock);
> +
> ctx = ntfs_attr_get_search_ctx(base_ni, NULL);
> if (!ctx) {
> err = -ENOMEM;
> @@ -138,6 +140,8 @@ static int ntfs_read_iomap_begin_resident(struct inode *inode, loff_t offset, lo
> if (ctx)
> ntfs_attr_put_search_ctx(ctx);
>
> + mutex_unlock(&base_ni->mrec_lock);
We need to hold the lock until iomap_end() because iomap_begin() only
returns an inline_data pointer into the MFT record. The actual data
copy happens later in the iomap core. So can you check if the attached
patch fixes this issue ?
From a33fee1b75a933bfdc42d59fb9e9c7be1b84a06f Mon Sep 17 00:00:00 2001
From: Hyeontae Lee <wonju345@xxxxxxxxx>
Date: Fri, 31 Jul 2026 18:10:35 +0900
Subject: [PATCH] ntfs: serialize resident iomap reads with mrec_lock
ntfs_read_iomap_begin_resident() walks the MFT record through
ntfs_attr_lookup() -> ntfs_attr_find() without taking ni->mrec_lock,
while ntfs_attr_record_resize(), ntfs_make_room_for_attr() and
ntfs_resident_attr_record_add() memmove() the same base_ni->mrec buffer
under that lock. map_mft_record() only takes a reference and does not
serialize, so the reader can observe torn attribute length and offset
fields while a writer is relocating the records.
KCSAN reports the race between the mmap read fault path and both link()
and unlink():
BUG: KCSAN: data-race in ntfs_attr_find / ntfs_attr_record_resize
write to 0xffff888100af1018 of 4 bytes by task 96 on cpu 1:
ntfs_attr_record_resize+0xd2/0x130
ntfs_attr_record_rm+0xad/0x530
ntfs_delete+0x224/0x640
ntfs_unlink+0x14d/0x280
vfs_unlink+0x157/0x520
read to 0xffff888100af1018 of 4 bytes by task 95 on cpu 0:
ntfs_attr_find+0x104/0x5b0
ntfs_attr_lookup+0x39c/0x10c0
ntfs_read_iomap_begin_resident+0xc6/0x230
ntfs_read_iomap_begin+0x5d/0xa0
iomap_iter+0x2e2/0x6e0
iomap_read_folio+0x147/0x2a0
ntfs_read_folio+0x108/0x170
filemap_read_folio+0x35/0x100
filemap_fault+0x993/0x1000
value changed: 0x00000250 -> 0x000001f0
The address is mrec + 0x18, i.e. mft_record.bytes_in_use, and the change
is the 96 bytes of one $FILE_NAME attribute being removed.
Keep base_ni->mrec_lock from the resident read iomap lookup through
iomap_end(). This protects both the attribute walk and the subsequent copy
from iomap->inline_data, which points into the MFT record. The non-resident
path is left alone: ntfs_lookup() already holds the directory inode's
mrec_lock when it reads an index folio through read_mapping_folio(), and
taking the lock in the shared wrapper deadlocks there with recursive locking
on mrec_lock. The comment above the read_mapping_folio() call in
fs/ntfs/dir.c notes the same hazard.
The seek path uses the same lookup helper but does not dereference
iomap->inline_data. Release the lock before returning from that path,
whereas the regular read path records base_ni in iomap->private and releases
the lock from its iomap_end() callback.
Tested with a reproducer that faults in a 16-byte resident file while
another thread runs link()/unlink() on it. Before: 40 KCSAN reports in
about one second. After: no reports in 180 seconds over 206,090 read
iterations and 423,540 link/unlink cycles. A PROVE_LOCKING build shows no
lockdep splat with the same reproducer running for 60 seconds.
Fixes: b041ca562526 ("ntfs: update iomap and address space operations")
Link: https://lore.kernel.org/all/20260725042421.109599-1-wonju345@xxxxxxxxx/
Signed-off-by: Hyeontae Lee <wonju345@xxxxxxxxx>
---
fs/ntfs/iomap.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/fs/ntfs/iomap.c b/fs/ntfs/iomap.c
index 26a1831a2c18..73c50171285a 100644
--- a/fs/ntfs/iomap.c
+++ b/fs/ntfs/iomap.c
@@ -81,7 +81,7 @@ const struct iomap_write_ops ntfs_iomap_folio_ops = {
};
static int ntfs_read_iomap_begin_resident(struct inode *inode, loff_t offset, loff_t length,
- unsigned int flags, struct iomap *iomap)
+ unsigned int flags, struct iomap *iomap, bool keep_mrec_lock)
{
struct ntfs_inode *base_ni, *ni = NTFS_I(inode);
struct ntfs_attr_search_ctx *ctx;
@@ -95,6 +95,8 @@ static int ntfs_read_iomap_begin_resident(struct inode *inode, loff_t offset, lo
else
base_ni = ni;
+ mutex_lock(&base_ni->mrec_lock);
+
ctx = ntfs_attr_get_search_ctx(base_ni, NULL);
if (!ctx) {
err = -ENOMEM;
@@ -138,6 +140,13 @@ static int ntfs_read_iomap_begin_resident(struct inode *inode, loff_t offset, lo
if (ctx)
ntfs_attr_put_search_ctx(ctx);
+ if (!err && keep_mrec_lock && iomap->type == IOMAP_INLINE) {
+ iomap->private = base_ni;
+ return 0;
+ }
+
+ mutex_unlock(&base_ni->mrec_lock);
+
return err;
}
@@ -261,24 +270,35 @@ static int ntfs_read_iomap_begin_non_resident(struct inode *inode, loff_t offset
static int __ntfs_read_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
unsigned int flags, struct iomap *iomap, struct iomap *srcmap,
- bool need_unwritten)
+ bool need_unwritten, bool keep_mrec_lock)
{
if (NInoNonResident(NTFS_I(inode)))
return ntfs_read_iomap_begin_non_resident(inode, offset, length,
flags, iomap, need_unwritten);
return ntfs_read_iomap_begin_resident(inode, offset, length,
- flags, iomap);
+ flags, iomap, keep_mrec_lock);
}
static int ntfs_read_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
{
return __ntfs_read_iomap_begin(inode, offset, length, flags, iomap,
- srcmap, true);
+ srcmap, true, true);
+}
+
+static int ntfs_read_iomap_end(struct inode *inode, loff_t pos, loff_t length,
+ ssize_t written, unsigned int flags, struct iomap *iomap)
+{
+ struct ntfs_inode *base_ni = iomap->private;
+
+ if (base_ni)
+ mutex_unlock(&base_ni->mrec_lock);
+ return written;
}
const struct iomap_ops ntfs_read_iomap_ops = {
.iomap_begin = ntfs_read_iomap_begin,
+ .iomap_end = ntfs_read_iomap_end,
};
/*
@@ -318,7 +338,7 @@ static int ntfs_seek_iomap_begin(struct inode *inode, loff_t offset, loff_t leng
unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
{
return __ntfs_read_iomap_begin(inode, offset, length, flags, iomap,
- srcmap, false);
+ srcmap, false, false);
}
static int ntfs_zero_read_iomap_end(struct inode *inode, loff_t pos, loff_t length,
--
2.25.1