* [PATCH v7 1/4] ntfs: replace raw ctx->al_entry with restartable attr-list locators
2026-08-12 1:18 [PATCH v7 0/4] ntfs: serialize attribute-list access Hyunchul Lee
@ 2026-08-12 1:18 ` Hyunchul Lee
2026-08-12 1:18 ` [PATCH v7 2/4] ntfs: protect attribute-list buffer replacement with attr_list_persist_lock Hyunchul Lee
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Hyunchul Lee @ 2026-08-12 1:18 UTC (permalink / raw)
To: Namjae Jeon; +Cc: Hyunchul Lee, ntfs, Cen Zhang, Copilot, stable
ntfs_attr_search_ctx kept a raw al_entry pointer into the in-memory
$ATTRIBUTE_LIST buffer. Any insertion or removal reallocates the
whole buffer, so that pointer becomes stale while the context is still
alive across extent mapping, record moves, and mapping-pair rebuilds.
Replace the raw pointer with three generation-checked locators:
- al_cursor: resume position for ntfs_external_attr_find()
- al_insert: insertion point captured on -ENOENT
- al_exact: identity snapshot used by writers to re-find an entry
Add attr_list_lock (rw_semaphore) and attr_list_gen to protect the
in-memory list and detect stale locators. ntfs_external_attr_find()
takes the lock for read. Writers re-locate under the lock for write
via ntfs_attrlist_find_exact_locked() before mutating.
Reported-by: Cen Zhang <zzzccc427@gmail.com>
Link: https://lore.kernel.org/all/20260702161710.2606853-1-zzzccc427@gmail.com/
Fixes: 495e90fa3348 ("ntfs: update attrib operations")
Cc: stable@vger.kernel.org
Tested-by: Cen Zhang <zzzccc427@gmail.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
---
fs/ntfs/attrib.c | 377 +++++++++++++++++++++++++++++++++++++++++++----------
fs/ntfs/attrib.h | 44 ++++++-
fs/ntfs/attrlist.c | 23 ++--
fs/ntfs/index.c | 3 +-
fs/ntfs/inode.c | 34 +++--
fs/ntfs/inode.h | 6 +
6 files changed, 391 insertions(+), 96 deletions(-)
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index c62c8ca8b987..0f48dd923b72 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -30,6 +30,97 @@
__le16 AT_UNNAMED[] = { cpu_to_le16('\0') };
+void ntfs_attrlist_reset_locators(struct ntfs_attr_search_ctx *ctx)
+{
+ ctx->al_cursor = (struct ntfs_attrlist_cursor){ 0 };
+ ctx->al_insert = (struct ntfs_attrlist_anchor){ 0 };
+ ctx->al_exact = (struct ntfs_attrlist_exact){ 0 };
+}
+
+void ntfs_attrlist_capture_exact(struct ntfs_attr_search_ctx *ctx,
+ struct ntfs_inode *base_ni,
+ struct attr_list_entry *ale, u8 *al_start)
+{
+ ctx->al_exact.off = (u8 *)ale - al_start;
+ ctx->al_exact.gen = base_ni->attr_list_gen;
+ ctx->al_exact.valid = true;
+ ntfs_attrlist_exact_key_from_ale(&ctx->al_exact.key, ale);
+}
+
+static void ntfs_attrlist_capture_insert(struct ntfs_attr_search_ctx *ctx,
+ struct ntfs_inode *base_ni,
+ struct attr_list_entry *ale,
+ u8 *al_start, u8 *al_end)
+{
+ ctx->al_insert.off = (u8 *)ale - al_start;
+ ctx->al_insert.gen = base_ni->attr_list_gen;
+ ctx->al_insert.valid = true;
+ ctx->al_insert.at_end = ((u8 *)ale == al_end);
+}
+
+void ntfs_attrlist_exact_key_from_ale(struct ntfs_attrlist_exact_key *key,
+ const struct attr_list_entry *ale)
+{
+ key->type = ale->type;
+ key->lowest_vcn = ale->lowest_vcn;
+ key->mft_reference = ale->mft_reference;
+ key->instance = ale->instance;
+ key->name_len = ale->name_length;
+}
+
+bool ntfs_attrlist_exact_key_eq(const struct attr_list_entry *ale,
+ const struct ntfs_attrlist_exact_key *key)
+{
+ if (ale->type != key->type)
+ return false;
+ if (ale->lowest_vcn != key->lowest_vcn)
+ return false;
+ if (ale->mft_reference != key->mft_reference)
+ return false;
+ if (ale->instance != key->instance)
+ return false;
+
+ return ale->name_length == key->name_len;
+}
+
+static struct attr_list_entry *ntfs_attrlist_find_exact_locked(struct ntfs_inode *base_ni,
+ struct ntfs_attrlist_exact *exact)
+{
+ struct attr_list_entry *ale;
+ u8 *al_end;
+
+ if (!exact->valid || !base_ni->attr_list)
+ return NULL;
+
+ al_end = base_ni->attr_list + base_ni->attr_list_size;
+ if (exact->gen == base_ni->attr_list_gen &&
+ exact->off < base_ni->attr_list_size) {
+ ale = (struct attr_list_entry *)(base_ni->attr_list + exact->off);
+
+ if (ntfs_attr_list_entry_is_valid(ale, al_end) &&
+ ntfs_attrlist_exact_key_eq(ale, &exact->key))
+ return ale;
+ }
+
+ for (ale = (struct attr_list_entry *)base_ni->attr_list;
+ ntfs_attr_list_entry_is_valid(ale, al_end);
+ ale = (struct attr_list_entry *)((u8 *)ale +
+ le16_to_cpu(ale->length))) {
+ if (ntfs_attrlist_exact_key_eq(ale, &exact->key))
+ return ale;
+ }
+
+ return NULL;
+}
+
+static struct ntfs_inode *ntfs_attr_ctx_base_ni(struct ntfs_attr_search_ctx *ctx)
+{
+ if (ctx->ntfs_ino->nr_extents == -1)
+ return ctx->base_ntfs_ino;
+
+ return ctx->ntfs_ino;
+}
+
/*
* Maximum size allowed for reading attributes by ntfs_attr_readall().
* Extended attribute, reparse point are not expected to be larger than this size.
@@ -1156,6 +1247,7 @@ static int ntfs_external_attr_find(const __le32 type,
u32 al_name_len;
u32 attr_len, mft_free_len;
bool is_first_search = false;
+ bool attr_list_locked = false;
int err = 0;
static const char *es = " Unmount and run chkdsk.";
@@ -1171,10 +1263,16 @@ static int ntfs_external_attr_find(const __le32 type,
if (type == AT_END)
goto not_found;
vol = base_ni->vol;
+ down_read(&base_ni->attr_list_lock);
+ attr_list_locked = true;
al_start = base_ni->attr_list;
al_end = al_start + base_ni->attr_list_size;
- if (!ctx->al_entry) {
- ctx->al_entry = (struct attr_list_entry *)al_start;
+ if (!ctx->al_cursor.valid ||
+ ctx->al_cursor.gen != base_ni->attr_list_gen ||
+ ctx->al_cursor.off >= base_ni->attr_list_size) {
+ ctx->al_cursor.off = 0;
+ ctx->al_cursor.gen = base_ni->attr_list_gen;
+ ctx->al_cursor.valid = true;
is_first_search = true;
}
/*
@@ -1182,7 +1280,7 @@ static int ntfs_external_attr_find(const __le32 type,
* or the entry following that, if @ctx->is_first is 'true'.
*/
if (ctx->is_first) {
- al_entry = ctx->al_entry;
+ al_entry = (struct attr_list_entry *)(al_start + ctx->al_cursor.off);
ctx->is_first = false;
/*
* If an enumeration and the first attribute is higher than
@@ -1195,14 +1293,14 @@ static int ntfs_external_attr_find(const __le32 type,
goto find_attr_list_attr;
} else {
/* Check for small entry */
- if (((al_end - (u8 *)ctx->al_entry) <
- (long)offsetof(struct attr_list_entry, name)) ||
- (le16_to_cpu(ctx->al_entry->length) & 7) ||
- (le16_to_cpu(ctx->al_entry->length) < offsetof(struct attr_list_entry, name)))
+ al_entry = (struct attr_list_entry *)(al_start + ctx->al_cursor.off);
+ if (((al_end - (u8 *)al_entry) < (long)offsetof(struct attr_list_entry, name)) ||
+ (le16_to_cpu(al_entry->length) & 7) ||
+ (le16_to_cpu(al_entry->length) < offsetof(struct attr_list_entry, name)))
goto corrupt;
- al_entry = (struct attr_list_entry *)((u8 *)ctx->al_entry +
- le16_to_cpu(ctx->al_entry->length));
+ al_entry = (struct attr_list_entry *)((u8 *)al_entry +
+ le16_to_cpu(al_entry->length));
if ((u8 *)al_entry == al_end)
goto not_found;
@@ -1218,15 +1316,18 @@ static int ntfs_external_attr_find(const __le32 type,
* attribute list attribute from the base mft record as it is
* not listed in the attribute list itself.
*/
- if ((type == AT_UNUSED) && le32_to_cpu(ctx->al_entry->type) <
- le32_to_cpu(AT_ATTRIBUTE_LIST) &&
- le32_to_cpu(al_entry->type) >
- le32_to_cpu(AT_ATTRIBUTE_LIST)) {
+ if ((type == AT_UNUSED) &&
+ le32_to_cpu(((struct attr_list_entry *)(al_start +
+ ctx->al_cursor.off))->type) <
+ le32_to_cpu(AT_ATTRIBUTE_LIST) &&
+ le32_to_cpu(al_entry->type) > le32_to_cpu(AT_ATTRIBUTE_LIST)) {
find_attr_list_attr:
/* Check for bogus calls. */
- if (name || name_len || val || val_len || lowest_vcn)
- return -EINVAL;
+ if (name || name_len || val || val_len || lowest_vcn) {
+ err = -EINVAL;
+ goto unlock_list_attr;
+ }
/* We want the base record. */
if (ctx->ntfs_ino != base_ni)
@@ -1248,28 +1349,33 @@ static int ntfs_external_attr_find(const __le32 type,
* Setup the search context so the correct
* attribute is returned next time round.
*/
- ctx->al_entry = al_entry;
+ ctx->al_cursor.off = (u8 *)al_entry - al_start;
+ ctx->al_cursor.gen = base_ni->attr_list_gen;
+ ctx->al_cursor.valid = true;
ctx->is_first = true;
- /* Got it. Done. */
if (!err)
- return 0;
+ goto unlock_list_attr;
/* Error! If other than not found return it. */
if (err != -ENOENT)
- return err;
+ goto unlock_list_attr;
/* Not found?!? Absurd! */
ntfs_error(ctx->ntfs_ino->vol->sb, "Attribute list wasn't found");
- return -EIO;
+ err = -EIO;
+ goto unlock_list_attr;
}
}
for (;; al_entry = next_al_entry) {
+scan_ale:
/* Out of bounds check. */
if ((u8 *)al_entry < base_ni->attr_list ||
(u8 *)al_entry > al_end)
break; /* Inode is corrupt. */
- ctx->al_entry = al_entry;
+ ctx->al_cursor.off = (u8 *)al_entry - al_start;
+ ctx->al_cursor.gen = base_ni->attr_list_gen;
+ ctx->al_cursor.valid = true;
/* Catch the end of the attribute list. */
if ((u8 *)al_entry == al_end)
goto not_found;
@@ -1333,6 +1439,9 @@ static int ntfs_external_attr_find(const __le32 type,
if (rc)
continue;
}
+ if (type != AT_UNUSED &&
+ le64_to_cpu(al_entry->lowest_vcn) > (u64)lowest_vcn)
+ goto not_found;
/*
* The names match or @name not present and attribute is
* unnamed. Now check @lowest_vcn. Continue search if the
@@ -1355,8 +1464,19 @@ static int ntfs_external_attr_find(const __le32 type,
continue;
is_enumeration:
- if (MREF_LE(al_entry->mft_reference) == ni->mft_no) {
- if (MSEQNO_LE(al_entry->mft_reference) != ni->seq_no) {
+ /*
+ * The extent mapping below takes extent_lock. Keep only a
+ * stable copy of this ALE while taking it, so the attr-list
+ * read lock never nests outside extent_lock.
+ */
+ ntfs_attrlist_capture_exact(ctx, base_ni, al_entry, al_start);
+ if (attr_list_locked)
+ up_read(&base_ni->attr_list_lock);
+ attr_list_locked = false;
+
+ if (MREF_LE(ctx->al_exact.key.mft_reference) == ni->mft_no) {
+ if (MSEQNO_LE(ctx->al_exact.key.mft_reference) !=
+ ni->seq_no) {
ntfs_error(vol->sb,
"Found stale mft reference in attribute list of base inode 0x%llx.%s",
base_ni->mft_no, es);
@@ -1368,21 +1488,22 @@ static int ntfs_external_attr_find(const __le32 type,
if (ni != base_ni)
unmap_extent_mft_record(ni);
/* Do we want the base record back? */
- if (MREF_LE(al_entry->mft_reference) ==
- base_ni->mft_no) {
+ if (MREF_LE(ctx->al_exact.key.mft_reference) ==
+ base_ni->mft_no) {
ni = ctx->ntfs_ino = base_ni;
ctx->mrec = ctx->base_mrec;
ctx->mapped_mrec = ctx->mapped_base_mrec;
} else {
/* We want an extent record. */
- ctx->mrec = map_extent_mft_record(base_ni,
- le64_to_cpu(
- al_entry->mft_reference), &ni);
+ ctx->mrec =
+ map_extent_mft_record(base_ni, le64_to_cpu(
+ ctx->al_exact.key.mft_reference),
+ &ni);
if (IS_ERR(ctx->mrec)) {
ntfs_error(vol->sb,
- "Failed to map extent mft record 0x%lx of base inode 0x%llx.%s",
- MREF_LE(al_entry->mft_reference),
- base_ni->mft_no, es);
+ "Failed to map extent mft record 0x%lx of base inode 0x%llx.%s",
+ MREF_LE(ctx->al_exact.key.mft_reference),
+ base_ni->mft_no, es);
err = PTR_ERR(ctx->mrec);
if (err == -ENOENT)
err = -EIO;
@@ -1392,7 +1513,6 @@ static int ntfs_external_attr_find(const __le32 type,
}
ctx->ntfs_ino = ni;
ctx->mapped_mrec = true;
-
}
}
a = ctx->attr = (struct attr_record *)((u8 *)ctx->mrec +
@@ -1424,8 +1544,19 @@ static int ntfs_external_attr_find(const __le32 type,
mft_free_len = le32_to_cpu(ctx->mrec->bytes_in_use) -
((u8 *)a - (u8 *)ctx->mrec);
- if (mft_free_len >= sizeof(a->type) && a->type == AT_END)
- continue;
+ if (mft_free_len >= sizeof(a->type) && a->type == AT_END) {
+ down_read(&base_ni->attr_list_lock);
+ attr_list_locked = true;
+ al_start = base_ni->attr_list;
+ al_end = al_start + base_ni->attr_list_size;
+ al_entry = ntfs_attrlist_find_exact_locked(base_ni,
+ &ctx->al_exact);
+ if (!al_entry)
+ goto corrupt;
+ al_entry = (struct attr_list_entry *)((u8 *)al_entry +
+ le16_to_cpu(al_entry->length));
+ goto scan_ale;
+ }
attr_len = le32_to_cpu(a->length);
if (!attr_len ||
@@ -1434,23 +1565,38 @@ static int ntfs_external_attr_find(const __le32 type,
attr_len > mft_free_len)
break;
- if (al_entry->instance != a->instance)
+ if (ctx->al_exact.key.instance != a->instance)
goto do_next_attr;
/*
* If the type and/or the name are mismatched between the
* attribute list entry and the attribute record, there is
* corruption so we break and return error EIO.
*/
- if (al_entry->type != a->type)
+ if (ctx->al_exact.key.type != a->type)
break;
if (a->name_length && ((le16_to_cpu(a->name_offset) +
a->name_length * sizeof(__le16)) > attr_len))
break;
- if (!ntfs_are_names_equal((__le16 *)((u8 *)a +
- le16_to_cpu(a->name_offset)), a->name_length,
- al_name, al_name_len, CASE_SENSITIVE,
- vol->upcase, vol->upcase_len))
+ /*
+ * al_entry's name isn't carried across the attr_list_lock
+ * drop above; re-take the lock just long enough to re-locate
+ * the same attr-list entry (same pattern as the AT_END case
+ * above) and compare @a's on-disk name against it. The
+ * corrupt/unlock_list_attr labels below drop the lock again
+ * via attr_list_locked on the not-found/mismatch paths.
+ */
+ down_read(&base_ni->attr_list_lock);
+ attr_list_locked = true;
+ al_entry = ntfs_attrlist_find_exact_locked(base_ni, &ctx->al_exact);
+ if (!al_entry)
+ goto corrupt;
+ if (!ntfs_are_names_equal((__le16 *)((u8 *)a + le16_to_cpu(a->name_offset)),
+ a->name_length, al_entry->name,
+ al_entry->name_length, CASE_SENSITIVE,
+ vol->upcase, vol->upcase_len))
break;
+ up_read(&base_ni->attr_list_lock);
+ attr_list_locked = false;
ctx->attr = a;
@@ -1480,6 +1626,11 @@ static int ntfs_external_attr_find(const __le32 type,
goto do_next_attr_loop;
}
+unlock_list_attr:
+ if (attr_list_locked)
+ up_read(&base_ni->attr_list_lock);
+ return err;
+
corrupt:
if (ni != base_ni) {
if (ni)
@@ -1491,20 +1642,31 @@ static int ntfs_external_attr_find(const __le32 type,
}
if (!err) {
- u64 mft_no = ctx->al_entry ? MREF_LE(ctx->al_entry->mft_reference) : 0;
- u32 type = ctx->al_entry ? le32_to_cpu(ctx->al_entry->type) : 0;
+ u64 mft_no = ctx->al_exact.valid ?
+ MREF_LE(ctx->al_exact.key.mft_reference) :
+ 0;
+ u32 ale_type = ctx->al_exact.valid ?
+ le32_to_cpu(ctx->al_exact.key.type) :
+ 0;
ntfs_error(vol->sb,
- "Base inode 0x%llx contains corrupt attribute, mft %#llx, type %#x. %s",
- (long long)base_ni->mft_no, (long long)mft_no, type,
- "Unmount and run chkdsk.");
+ "Base inode 0x%llx contains corrupt attribute, mft %#llx, type %#x. %s",
+ (long long)base_ni->mft_no, (long long)mft_no,
+ ale_type, "Unmount and run chkdsk.");
err = -EIO;
}
+ if (attr_list_locked)
+ up_read(&base_ni->attr_list_lock);
if (err != -ENOMEM)
NVolSetErrors(vol);
return err;
not_found:
+ if (attr_list_locked) {
+ ntfs_attrlist_capture_insert(ctx, base_ni, al_entry, al_start,
+ al_end);
+ up_read(&base_ni->attr_list_lock);
+ }
/*
* If we were looking for AT_END, we reset the search context @ctx and
* use ntfs_attr_find() to seek to the end of the base mft record.
@@ -1634,11 +1796,11 @@ static bool ntfs_attr_init_search_ctx(struct ntfs_attr_search_ctx *ctx,
ctx->attr = (struct attr_record *)((u8 *)mrec + le16_to_cpu(mrec->attrs_offset));
ctx->is_first = true;
ctx->ntfs_ino = ni;
- ctx->al_entry = NULL;
ctx->base_ntfs_ino = NULL;
ctx->base_mrec = NULL;
ctx->base_attr = NULL;
ctx->mapped_base_mrec = false;
+ ntfs_attrlist_reset_locators(ctx);
return true;
}
@@ -1662,11 +1824,7 @@ void ntfs_attr_reinit_search_ctx(struct ntfs_attr_search_ctx *ctx)
/* Sanity checks are performed elsewhere. */
ctx->attr = (struct attr_record *)((u8 *)ctx->mrec +
le16_to_cpu(ctx->mrec->attrs_offset));
- /*
- * This needs resetting due to ntfs_external_attr_find() which
- * can leave it set despite having zeroed ctx->base_ntfs_ino.
- */
- ctx->al_entry = NULL;
+ ntfs_attrlist_reset_locators(ctx);
return;
} /* Attribute list. */
if (ctx->ntfs_ino != ctx->base_ntfs_ino && ctx->ntfs_ino)
@@ -2626,13 +2784,12 @@ static int ntfs_non_resident_attr_record_add(struct ntfs_inode *ni, __le32 type,
* update of attribute list.
*/
ntfs_attr_reinit_search_ctx(ctx);
- err = ntfs_attr_lookup(type, name, name_len, CASE_SENSITIVE,
- lowest_vcn, NULL, 0, ctx);
+ err = ntfs_attr_lookup(type, name, name_len, CASE_SENSITIVE, lowest_vcn,
+ NULL, 0, ctx);
if (err) {
pr_err("%s: attribute lookup failed\n", __func__);
ntfs_attr_put_search_ctx(ctx);
return err;
-
}
offset = (u8 *)ctx->attr - (u8 *)ctx->mrec;
ntfs_attr_put_search_ctx(ctx);
@@ -3396,6 +3553,8 @@ int ntfs_attr_record_move_to(struct ntfs_attr_search_ctx *ctx, struct ntfs_inode
int err;
struct mft_record *ni_mrec;
struct super_block *sb;
+ struct ntfs_inode *base_ni;
+ struct attr_list_entry *ale;
if (!ctx || !ctx->attr || !ctx->ntfs_ino || !ni) {
ntfs_debug("Invalid arguments passed.\n");
@@ -3411,7 +3570,7 @@ int ntfs_attr_record_move_to(struct ntfs_attr_search_ctx *ctx, struct ntfs_inode
if (ctx->ntfs_ino == ni)
return 0;
- if (!ctx->al_entry) {
+ if (!ctx->al_exact.valid) {
ntfs_debug("Inode should contain attribute list to use this function.\n");
return -EINVAL;
}
@@ -3465,9 +3624,24 @@ int ntfs_attr_record_move_to(struct ntfs_attr_search_ctx *ctx, struct ntfs_inode
mark_mft_record_dirty(ni);
/* Update attribute list. */
- ctx->al_entry->mft_reference =
+ a = (struct attr_record *)nctx->attr;
+ base_ni = ntfs_attr_ctx_base_ni(ctx);
+
+ down_write(&base_ni->attr_list_lock);
+ ale = ntfs_attrlist_find_exact_locked(base_ni, &ctx->al_exact);
+ if (!ale) {
+ up_write(&base_ni->attr_list_lock);
+ unmap_mft_record(ni);
+ err = -EIO;
+ goto put_err_out;
+ }
+
+ ale->mft_reference =
MK_LE_MREF(ni->mft_no, le16_to_cpu(ni_mrec->sequence_number));
- ctx->al_entry->instance = nctx->attr->instance;
+ ale->instance = nctx->attr->instance;
+ base_ni->attr_list_gen++;
+ ntfs_attrlist_capture_exact(ctx, base_ni, ale, base_ni->attr_list);
+ up_write(&base_ni->attr_list_lock);
unmap_mft_record(ni);
put_err_out:
ntfs_attr_put_search_ctx(nctx);
@@ -3707,6 +3881,23 @@ static int ntfs_attr_update_meta(struct attr_record *a, struct ntfs_inode *ni,
* call to this function. Vice-versa @na->compressed_size will be calculated and
* set to correct value during this function.
*/
+static bool ntfs_attr_ctx_matches_ni(const struct ntfs_attr_search_ctx *ctx,
+ const struct ntfs_inode *ni)
+{
+ const struct attr_record *a = ctx->attr;
+ const __le16 *name;
+
+ if (a->type != ni->type || a->name_length != ni->name_len)
+ return false;
+ if (!a->name_length)
+ return true;
+
+ name = (const __le16 *)((const u8 *)a + le16_to_cpu(a->name_offset));
+ return ntfs_are_names_equal(name, a->name_length, ni->name,
+ ni->name_len, CASE_SENSITIVE,
+ ni->vol->upcase, ni->vol->upcase_len);
+}
+
int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
{
struct ntfs_attr_search_ctx *ctx;
@@ -3715,11 +3906,12 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
struct attr_record *a;
s64 stop_vcn;
int err = 0, mp_size, cur_max_mp_size, exp_max_mp_size;
- bool finished_build;
+ bool finished_build, attrlist_changed = false;
bool first_updated = false;
struct super_block *sb;
struct runlist_element *start_rl;
unsigned int de_cluster_count = 0;
+ bool first_lookup = true;
retry:
if (!ni || !ni->runlist.rl)
@@ -3748,11 +3940,24 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
/* Fill attribute records with new mapping pairs. */
stop_vcn = 0;
finished_build = false;
+ first_lookup = true;
start_rl = ni->runlist.rl;
- while (!(err = ntfs_attr_lookup(ni->type, ni->name, ni->name_len,
- CASE_SENSITIVE, from_vcn, NULL, 0, ctx))) {
+ while (1) {
unsigned int de_cnt = 0;
+ err = ntfs_attr_lookup(AT_UNUSED, NULL, 0, CASE_SENSITIVE, 0,
+ NULL, 0, ctx);
+ if (err)
+ break;
+ if (!ntfs_attr_ctx_matches_ni(ctx, ni))
+ continue;
+ if (first_lookup) {
+ if (le64_to_cpu(ctx->attr->data.non_resident.lowest_vcn) <
+ from_vcn)
+ continue;
+ first_lookup = false;
+ }
+
a = ctx->attr;
m = ctx->mrec;
if (!a->data.non_resident.lowest_vcn)
@@ -3878,15 +4083,40 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
}
}
- /* Update lowest vcn. */
- a->data.non_resident.lowest_vcn = cpu_to_le64(stop_vcn);
- mark_mft_record_dirty(ctx->ntfs_ino);
- if ((ctx->ntfs_ino->nr_extents == -1 || NInoAttrList(ctx->ntfs_ino)) &&
+ /*
+ * al_cursor.valid is true here exactly when this iteration's
+ * ntfs_attr_lookup() went through ntfs_external_attr_find()
+ */
+ if (ctx->al_cursor.valid &&
ctx->attr->type != AT_ATTRIBUTE_LIST) {
- ctx->al_entry->lowest_vcn = cpu_to_le64(stop_vcn);
- err = ntfs_attrlist_update(base_ni);
- if (err)
+ struct attr_list_entry *ale;
+
+ down_write(&base_ni->attr_list_lock);
+ ale = ntfs_attrlist_find_exact_locked(base_ni,
+ &ctx->al_exact);
+ if (!ale) {
+ up_write(&base_ni->attr_list_lock);
+ err = -EIO;
goto put_err_out;
+ }
+ ale->lowest_vcn = cpu_to_le64(stop_vcn);
+ base_ni->attr_list_gen++;
+ ntfs_attrlist_capture_exact(ctx, base_ni, ale,
+ base_ni->attr_list);
+ ctx->al_cursor.off = (u8 *)ale - base_ni->attr_list;
+ ctx->al_cursor.gen = base_ni->attr_list_gen;
+ ctx->al_cursor.valid = true;
+ attrlist_changed = true;
+ up_write(&base_ni->attr_list_lock);
+
+ /* Update lowest vcn in attr record after ALE is fixed. */
+ a->data.non_resident.lowest_vcn = cpu_to_le64(stop_vcn);
+ mark_mft_record_dirty(ctx->ntfs_ino);
+
+ } else {
+ /* Update lowest vcn. */
+ a->data.non_resident.lowest_vcn = cpu_to_le64(stop_vcn);
+ mark_mft_record_dirty(ctx->ntfs_ino);
}
/*
@@ -3937,12 +4167,21 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
}
}
+ if (attrlist_changed) {
+ err = ntfs_attrlist_update(base_ni);
+ if (err)
+ goto put_err_out;
+ }
+
/* Deallocate not used attribute extents and return with success. */
if (finished_build) {
ntfs_attr_reinit_search_ctx(ctx);
ntfs_debug("Deallocate marked extents.\n");
- while (!(err = ntfs_attr_lookup(ni->type, ni->name, ni->name_len,
- CASE_SENSITIVE, 0, NULL, 0, ctx))) {
+ while (!(err = ntfs_attr_lookup(AT_UNUSED, NULL, 0,
+ CASE_SENSITIVE, 0, NULL, 0,
+ ctx))) {
+ if (!ntfs_attr_ctx_matches_ni(ctx, ni))
+ continue;
if (le64_to_cpu(ctx->attr->data.non_resident.highest_vcn) !=
NTFS_VCN_DELETE_MARK)
continue;
diff --git a/fs/ntfs/attrib.h b/fs/ntfs/attrib.h
index e2224fbfaabe..a39719f75d06 100644
--- a/fs/ntfs/attrib.h
+++ b/fs/ntfs/attrib.h
@@ -15,6 +15,34 @@
extern __le16 AT_UNNAMED[];
+struct ntfs_attrlist_cursor {
+ u32 off;
+ u64 gen;
+ bool valid;
+};
+
+struct ntfs_attrlist_anchor {
+ u32 off;
+ u64 gen;
+ bool valid;
+ bool at_end;
+};
+
+struct ntfs_attrlist_exact_key {
+ __le32 type;
+ __le64 lowest_vcn;
+ __le64 mft_reference;
+ __le16 instance;
+ u8 name_len;
+};
+
+struct ntfs_attrlist_exact {
+ u32 off;
+ u64 gen;
+ bool valid;
+ struct ntfs_attrlist_exact_key key;
+};
+
/*
* ntfs_attr_search_ctx - used in attribute search functions
* @mrec: buffer containing mft record to search
@@ -22,10 +50,12 @@ extern __le16 AT_UNNAMED[];
* @attr: attribute record in @mrec where to begin/continue search
* @is_first: if true ntfs_attr_lookup() begins search with @attr, else after
* @ntfs_ino: Inode owning this attribute search
- * @al_entry: Current attribute list entry
* @base_ntfs_ino: Base inode
* @mapped_base_mrec: true if @base_mrec was mapped by the search
* @base_attr: Base attribute record pointer
+ * @al_cursor: Restartable attr-list enumeration cursor
+ * @al_insert: Insert-before anchor used on -ENOENT
+ * @al_exact: Exact ALE identity for writer-side updates
*
* Structure must be initialized to zero before the first call to one of the
* attribute search functions. Initialize @mrec to point to the mft record to
@@ -45,11 +75,13 @@ struct ntfs_attr_search_ctx {
struct attr_record *attr;
bool is_first;
struct ntfs_inode *ntfs_ino;
- struct attr_list_entry *al_entry;
struct ntfs_inode *base_ntfs_ino;
struct mft_record *base_mrec;
bool mapped_base_mrec;
struct attr_record *base_attr;
+ struct ntfs_attrlist_cursor al_cursor;
+ struct ntfs_attrlist_anchor al_insert;
+ struct ntfs_attrlist_exact al_exact;
};
enum { /* ways of processing holes when expanding */
@@ -89,6 +121,14 @@ void ntfs_attr_reinit_search_ctx(struct ntfs_attr_search_ctx *ctx);
struct ntfs_attr_search_ctx *ntfs_attr_get_search_ctx(struct ntfs_inode *ni,
struct mft_record *mrec);
void ntfs_attr_put_search_ctx(struct ntfs_attr_search_ctx *ctx);
+void ntfs_attrlist_reset_locators(struct ntfs_attr_search_ctx *ctx);
+void ntfs_attrlist_capture_exact(struct ntfs_attr_search_ctx *ctx,
+ struct ntfs_inode *base_ni,
+ struct attr_list_entry *ale, u8 *al_start);
+void ntfs_attrlist_exact_key_from_ale(struct ntfs_attrlist_exact_key *key,
+ const struct attr_list_entry *ale);
+bool ntfs_attrlist_exact_key_eq(const struct attr_list_entry *ale,
+ const struct ntfs_attrlist_exact_key *key);
int ntfs_attr_size_bounds_check(const struct ntfs_volume *vol,
const __le32 type, const s64 size);
int ntfs_attr_can_be_resident(const struct ntfs_volume *vol,
diff --git a/fs/ntfs/attrlist.c b/fs/ntfs/attrlist.c
index be3086d34338..b8594037df40 100644
--- a/fs/ntfs/attrlist.c
+++ b/fs/ntfs/attrlist.c
@@ -182,15 +182,16 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
0 : le32_to_cpu(attr->data.resident.value_length), ctx);
if (!err) {
/* Found some extent, check it to be before new extent. */
- if (ctx->al_entry->lowest_vcn == lowest_vcn) {
+ if (ctx->al_exact.key.lowest_vcn == lowest_vcn) {
err = -EEXIST;
ntfs_debug("Such attribute already present in the attribute list.\n");
ntfs_attr_put_search_ctx(ctx);
goto err_out;
}
/* Add new entry after this extent. */
- ale = (struct attr_list_entry *)((u8 *)ctx->al_entry +
- le16_to_cpu(ctx->al_entry->length));
+ entry_offset = ctx->al_exact.off +
+ le16_to_cpu(((struct attr_list_entry *)(ni->attr_list +
+ ctx->al_exact.off))->length);
} else {
/* Check for real errors. */
if (err != -ENOENT) {
@@ -199,13 +200,11 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
goto err_out;
}
/* No previous extents found. */
- ale = ctx->al_entry;
+ entry_offset = ctx->al_insert.off;
}
/* Don't need it anymore, @ctx->al_entry points to @ni->attr_list. */
ntfs_attr_put_search_ctx(ctx);
- /* Determine new entry offset. */
- entry_offset = ((u8 *)ale - ni->attr_list);
/* Set pointer to new entry. */
ale = (struct attr_list_entry *)(new_al + entry_offset);
memset(ale, 0, entry_len);
@@ -261,7 +260,7 @@ int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
struct ntfs_inode *base_ni;
struct attr_list_entry *ale;
- if (!ctx || !ctx->ntfs_ino || !ctx->al_entry) {
+ if (!ctx || !ctx->ntfs_ino || !ctx->al_exact.valid) {
ntfs_debug("Invalid arguments.\n");
return -EINVAL;
}
@@ -270,12 +269,14 @@ int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
base_ni = ctx->base_ntfs_ino;
else
base_ni = ctx->ntfs_ino;
- ale = ctx->al_entry;
+ if (ctx->al_exact.off >= base_ni->attr_list_size)
+ return -EIO;
+ ale = (struct attr_list_entry *)(base_ni->attr_list + ctx->al_exact.off);
ntfs_debug("Entering for inode 0x%llx, attr 0x%x, lowest_vcn %lld.\n",
- (long long)ctx->ntfs_ino->mft_no,
- (unsigned int)le32_to_cpu(ctx->al_entry->type),
- (long long)le64_to_cpu(ctx->al_entry->lowest_vcn));
+ (long long)ctx->ntfs_ino->mft_no,
+ (unsigned int)le32_to_cpu(ctx->al_exact.key.type),
+ (long long)le64_to_cpu(ctx->al_exact.key.lowest_vcn));
if (!NInoAttrList(base_ni)) {
ntfs_debug("Attribute list isn't present.\n");
diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index 409759eab55d..082fe9226c72 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -1346,7 +1346,8 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
* When there is no space to build a non-resident
* index, we may have to move the root to an extent
*/
- if ((ret == -ENOSPC) && (ctx->al_entry || !ntfs_inode_add_attrlist(icx->idx_ni))) {
+ if ((ret == -ENOSPC) &&
+ (ctx->al_cursor.valid || !ntfs_inode_add_attrlist(icx->idx_ni))) {
ntfs_attr_put_search_ctx(ctx);
ctx = NULL;
ir = ntfs_ir_lookup(icx->idx_ni, icx->name, icx->name_len, &ctx);
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 39c7fd8c1149..38206e2009e2 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -475,6 +475,8 @@ void __ntfs_init_inode(struct super_block *sb, struct ntfs_inode *ni)
ni->folio = NULL;
ni->folio_ofs = 0;
ni->mrec = NULL;
+ init_rwsem(&ni->attr_list_lock);
+ ni->attr_list_gen = 0;
ni->attr_list_size = 0;
ni->attr_list = NULL;
ni->itype.index.block_size = 0;
@@ -3327,19 +3329,25 @@ static int ntfs_attr_position(__le32 type, struct ntfs_attr_search_ctx *ctx)
if (atype == AT_END)
return -ENOSPC;
- /*
- * if ntfs_external_attr_lookup return -ENOENT, ctx->al_entry
- * could point to an attribute in an extent mft record, but
- * ctx->attr and ctx->ntfs_ino always points to an attibute in
- * a base mft record.
- */
- if (ctx->al_entry &&
- MREF_LE(ctx->al_entry->mft_reference) != ctx->ntfs_ino->mft_no) {
- ntfs_attr_reinit_search_ctx(ctx);
- err = ntfs_attr_lookup(atype, NULL, 0, CASE_SENSITIVE, 0, NULL,
- 0, ctx);
- if (err)
- return err;
+ if (ctx->al_insert.valid && !ctx->al_insert.at_end &&
+ ctx->al_insert.off < ntfs_inode_base(ctx->ntfs_ino)->attr_list_size) {
+ struct ntfs_inode *base_ni =
+ ntfs_inode_base(ctx->ntfs_ino);
+ struct attr_list_entry *ale =
+ (struct attr_list_entry *)(base_ni->attr_list +
+ ctx->al_insert.off);
+
+ if (MREF_LE(ale->mft_reference) != ctx->ntfs_ino->mft_no) {
+ ntfs_attr_reinit_search_ctx(ctx);
+ err = ntfs_attr_lookup(atype, NULL, 0,
+ CASE_SENSITIVE, 0, NULL,
+ 0, ctx);
+ if (err)
+ return err;
+ } else {
+ ntfs_attrlist_capture_exact(ctx, base_ni, ale,
+ base_ni->attr_list);
+ }
}
}
return 0;
diff --git a/fs/ntfs/inode.h b/fs/ntfs/inode.h
index c6d065aaecd5..ed00e06c5d14 100644
--- a/fs/ntfs/inode.h
+++ b/fs/ntfs/inode.h
@@ -10,6 +10,8 @@
#ifndef _LINUX_NTFS_INODE_H
#define _LINUX_NTFS_INODE_H
+#include <linux/rwsem.h>
+
#include "debug.h"
enum ntfs_inode_mutex_lock_class {
@@ -66,6 +68,8 @@ enum ntfs_inode_mutex_lock_class {
* Attribute list support (only for use by the attribute lookup
* functions). Setup during read_inode for all inodes with attribute
* lists. Only valid if NI_AttrList is set in state.
+ * @attr_list_lock: Protects in-memory attribute list state.
+ * @attr_list_gen: Generation of the in-memory attribute list state.
* @attr_list_size: Length of attribute list value in bytes.
* @attr_list: Attribute list value itself.
*
@@ -118,6 +122,8 @@ struct ntfs_inode {
int folio_ofs;
s64 mft_lcn[2];
unsigned int mft_lcn_count;
+ struct rw_semaphore attr_list_lock;
+ u64 attr_list_gen;
u32 attr_list_size;
u8 *attr_list;
union {
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 2/4] ntfs: protect attribute-list buffer replacement with attr_list_persist_lock
2026-08-12 1:18 [PATCH v7 0/4] ntfs: serialize attribute-list access Hyunchul Lee
2026-08-12 1:18 ` [PATCH v7 1/4] ntfs: replace raw ctx->al_entry with restartable attr-list locators Hyunchul Lee
@ 2026-08-12 1:18 ` Hyunchul Lee
2026-08-12 1:18 ` [PATCH v7 3/4] ntfs: protect attribute-list creation/teardown " Hyunchul Lee
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Hyunchul Lee @ 2026-08-12 1:18 UTC (permalink / raw)
To: Namjae Jeon; +Cc: Hyunchul Lee, ntfs, Cen Zhang, Copilot, stable
ntfs_attrlist_entry_add() and ntfs_attrlist_entry_rm() replace the
in-memory attribute-list buffer and then persist it to disk. The swap
is protected by attr_list_lock, but persisting can sleep and recurse,
so that lock cannot be held across it.
Add attr_list_persist_lock, a mutex that covers the entire
publish/swap/persist/rollback transaction for each caller.
Reported-by: Cen Zhang <zzzccc427@gmail.com>
Link: https://lore.kernel.org/all/20260702161710.2606853-1-zzzccc427@gmail.com/
Fixes: 495e90fa3348 ("ntfs: update attrib operations")
Cc: stable@vger.kernel.org
Tested-by: Cen Zhang <zzzccc427@gmail.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
---
fs/ntfs/attrib.c | 18 ++++--
fs/ntfs/attrib.h | 2 +
fs/ntfs/attrlist.c | 172 ++++++++++++++++++++++++++++++++++++++++++++---------
fs/ntfs/attrlist.h | 1 +
fs/ntfs/inode.c | 3 +-
fs/ntfs/inode.h | 3 +
6 files changed, 165 insertions(+), 34 deletions(-)
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 0f48dd923b72..ef918a73fb0c 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -83,8 +83,8 @@ bool ntfs_attrlist_exact_key_eq(const struct attr_list_entry *ale,
return ale->name_length == key->name_len;
}
-static struct attr_list_entry *ntfs_attrlist_find_exact_locked(struct ntfs_inode *base_ni,
- struct ntfs_attrlist_exact *exact)
+struct attr_list_entry *ntfs_attrlist_find_exact_locked(struct ntfs_inode *base_ni,
+ struct ntfs_attrlist_exact *exact)
{
struct attr_list_entry *ale;
u8 *al_end;
@@ -3792,7 +3792,7 @@ static int ntfs_attr_update_meta(struct attr_record *a, struct ntfs_inode *ni,
goto out;
}
- err = ntfs_attrlist_update(base_ni);
+ err = ntfs_attrlist_update_locked(base_ni);
if (err)
goto out;
err = -EAGAIN;
@@ -4168,7 +4168,7 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
}
if (attrlist_changed) {
- err = ntfs_attrlist_update(base_ni);
+ err = ntfs_attrlist_update_locked(base_ni);
if (err)
goto put_err_out;
}
@@ -5029,7 +5029,15 @@ static int ntfs_resident_attr_resize(struct ntfs_inode *attr_ni, const s64 newsi
"Couldn't free space in the MFT record to make attribute list non resident");
return err;
}
- err = ntfs_attrlist_update(base_ni);
+ /*
+ * Resizing the $ATTRIBUTE_LIST attribute itself only happens
+ * underneath ntfs_attrlist_update_locked() while already holding
+ * attr_list_persist_lock.
+ */
+ if (attr_ni->type == AT_ATTRIBUTE_LIST)
+ err = ntfs_attrlist_update_locked(base_ni);
+ else
+ err = ntfs_attrlist_update(base_ni);
if (err)
return err;
goto attr_resize_again;
diff --git a/fs/ntfs/attrib.h b/fs/ntfs/attrib.h
index a39719f75d06..2e5f87abd852 100644
--- a/fs/ntfs/attrib.h
+++ b/fs/ntfs/attrib.h
@@ -129,6 +129,8 @@ void ntfs_attrlist_exact_key_from_ale(struct ntfs_attrlist_exact_key *key,
const struct attr_list_entry *ale);
bool ntfs_attrlist_exact_key_eq(const struct attr_list_entry *ale,
const struct ntfs_attrlist_exact_key *key);
+struct attr_list_entry *ntfs_attrlist_find_exact_locked(struct ntfs_inode *base_ni,
+ struct ntfs_attrlist_exact *exact);
int ntfs_attr_size_bounds_check(const struct ntfs_volume *vol,
const __le32 type, const s64 size);
int ntfs_attr_can_be_resident(const struct ntfs_volume *vol,
diff --git a/fs/ntfs/attrlist.c b/fs/ntfs/attrlist.c
index b8594037df40..aa6d5f10bf96 100644
--- a/fs/ntfs/attrlist.c
+++ b/fs/ntfs/attrlist.c
@@ -51,7 +51,13 @@ int ntfs_attrlist_need(struct ntfs_inode *ni)
return 0;
}
-int ntfs_attrlist_update(struct ntfs_inode *base_ni)
+/*
+ * ntfs_attrlist_update_locked - persist the in-memory attribute list to disk
+ * @base_ni: base ntfs inode containing the attribute list
+ *
+ * Caller must hold @base_ni->attr_list_persist_lock.
+ */
+int ntfs_attrlist_update_locked(struct ntfs_inode *base_ni)
{
struct inode *attr_vi;
struct ntfs_inode *attr_ni;
@@ -111,6 +117,23 @@ int ntfs_attrlist_update(struct ntfs_inode *base_ni)
return 0;
}
+/*
+ * ntfs_attrlist_update - persist the in-memory attribute list to disk
+ * @base_ni: base ntfs inode containing the attribute list
+ *
+ * Serialize the persist against concurrent attribute-list replacement
+ * transactions.
+ */
+int ntfs_attrlist_update(struct ntfs_inode *base_ni)
+{
+ int err;
+
+ mutex_lock(&base_ni->attr_list_persist_lock);
+ err = ntfs_attrlist_update_locked(base_ni);
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+ return err;
+}
+
/*
* ntfs_attrlist_entry_add - add an attribute list attribute entry
* @ni: opened ntfs inode, which contains that attribute
@@ -122,12 +145,13 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
{
struct attr_list_entry *ale;
__le64 mref;
- struct ntfs_attr_search_ctx *ctx;
- u8 *new_al;
+ struct ntfs_attr_search_ctx *ctx = NULL;
+ u8 *new_al = NULL;
int entry_len, entry_offset, err;
struct mft_record *ni_mrec;
u8 *old_al;
__le64 lowest_vcn;
+ bool found, rollback;
if (!ni || !attr) {
ntfs_debug("Invalid arguments.\n");
@@ -154,12 +178,21 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
return -ENOENT;
}
- /* Determine size and allocate memory for new attribute list. */
+ mutex_lock(&ni->attr_list_persist_lock);
+
+ if (!NInoAttrList(ni) || !ni->attr_list) {
+ err = -ENOENT;
+ goto err_out;
+ }
+
+ /* Determine size of new attribute list entry. */
entry_len = (sizeof(struct attr_list_entry) + sizeof(__le16) *
attr->name_length + 7) & ~7;
- new_al = kvzalloc(ni->attr_list_size + entry_len, GFP_NOFS);
- if (!new_al)
- return -ENOMEM;
+
+retry_lookup:
+ new_al = NULL;
+ found = false;
+ rollback = false;
/* Find place for the new entry. */
ctx = ntfs_attr_get_search_ctx(ni, NULL);
@@ -181,29 +214,54 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
le16_to_cpu(attr->data.resident.value_offset)), (attr->non_resident) ?
0 : le32_to_cpu(attr->data.resident.value_length), ctx);
if (!err) {
+ found = true;
/* Found some extent, check it to be before new extent. */
if (ctx->al_exact.key.lowest_vcn == lowest_vcn) {
err = -EEXIST;
ntfs_debug("Such attribute already present in the attribute list.\n");
- ntfs_attr_put_search_ctx(ctx);
goto err_out;
}
- /* Add new entry after this extent. */
- entry_offset = ctx->al_exact.off +
- le16_to_cpu(((struct attr_list_entry *)(ni->attr_list +
- ctx->al_exact.off))->length);
} else {
/* Check for real errors. */
if (err != -ENOENT) {
ntfs_debug("Attribute lookup failed.\n");
- ntfs_attr_put_search_ctx(ctx);
goto err_out;
}
/* No previous extents found. */
+ }
+
+ down_write(&ni->attr_list_lock);
+ if (found) {
+ if (ctx->al_exact.gen != ni->attr_list_gen) {
+ up_write(&ni->attr_list_lock);
+ ntfs_attr_put_search_ctx(ctx);
+ ctx = NULL;
+ goto retry_lookup;
+ }
+ ale = ntfs_attrlist_find_exact_locked(ni, &ctx->al_exact);
+ if (!ale) {
+ up_write(&ni->attr_list_lock);
+ err = -EIO;
+ goto err_out;
+ }
+ entry_offset = (u8 *)ale - ni->attr_list + le16_to_cpu(ale->length);
+ } else {
+ if (!ctx->al_insert.valid ||
+ ctx->al_insert.gen != ni->attr_list_gen) {
+ up_write(&ni->attr_list_lock);
+ ntfs_attr_put_search_ctx(ctx);
+ ctx = NULL;
+ goto retry_lookup;
+ }
entry_offset = ctx->al_insert.off;
}
- /* Don't need it anymore, @ctx->al_entry points to @ni->attr_list. */
- ntfs_attr_put_search_ctx(ctx);
+
+ new_al = kvzalloc(ni->attr_list_size + entry_len, GFP_NOFS);
+ if (!new_al) {
+ up_write(&ni->attr_list_lock);
+ err = -ENOMEM;
+ goto err_out;
+ }
/* Set pointer to new entry. */
ale = (struct attr_list_entry *)(new_al + entry_offset);
@@ -231,17 +289,34 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
old_al = ni->attr_list;
ni->attr_list = new_al;
ni->attr_list_size = ni->attr_list_size + entry_len;
+ ni->attr_list_gen++;
+ up_write(&ni->attr_list_lock);
+ ntfs_attr_put_search_ctx(ctx);
+ ctx = NULL;
- err = ntfs_attrlist_update(ni);
+ err = ntfs_attrlist_update_locked(ni);
if (err) {
- ni->attr_list = old_al;
- ni->attr_list_size -= entry_len;
+ down_write(&ni->attr_list_lock);
+ if (ni->attr_list == new_al) {
+ ni->attr_list = old_al;
+ ni->attr_list_size -= entry_len;
+ ni->attr_list_gen++;
+ rollback = true;
+ }
+ up_write(&ni->attr_list_lock);
+ if (!rollback)
+ new_al = NULL;
goto err_out;
}
kvfree(old_al);
+ mutex_unlock(&ni->attr_list_persist_lock);
return 0;
err_out:
- kvfree(new_al);
+ if (ctx)
+ ntfs_attr_put_search_ctx(ctx);
+ if (new_al)
+ kvfree(new_al);
+ mutex_unlock(&ni->attr_list_persist_lock);
return err;
}
@@ -255,10 +330,12 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
*/
int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
{
- u8 *new_al;
- int new_al_len;
+ u8 *new_al = NULL;
+ int err, new_al_len;
+ bool rollback;
struct ntfs_inode *base_ni;
struct attr_list_entry *ale;
+ u8 *old_al;
if (!ctx || !ctx->ntfs_ino || !ctx->al_exact.valid) {
ntfs_debug("Invalid arguments.\n");
@@ -269,9 +346,6 @@ int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
base_ni = ctx->base_ntfs_ino;
else
base_ni = ctx->ntfs_ino;
- if (ctx->al_exact.off >= base_ni->attr_list_size)
- return -EIO;
- ale = (struct attr_list_entry *)(base_ni->attr_list + ctx->al_exact.off);
ntfs_debug("Entering for inode 0x%llx, attr 0x%x, lowest_vcn %lld.\n",
(long long)ctx->ntfs_ino->mft_no,
@@ -282,12 +356,33 @@ int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
ntfs_debug("Attribute list isn't present.\n");
return -ENOENT;
}
+ mutex_lock(&base_ni->attr_list_persist_lock);
+
+ /*
+ * Another thread may have removed the attribute list while we were
+ * waiting for the mutex.
+ */
+ if (!NInoAttrList(base_ni) || !base_ni->attr_list) {
+ err = -ENOENT;
+ goto out_unlock;
+ }
+
+ down_write(&base_ni->attr_list_lock);
+ ale = ntfs_attrlist_find_exact_locked(base_ni, &ctx->al_exact);
+ if (!ale) {
+ up_write(&base_ni->attr_list_lock);
+ err = -EIO;
+ goto out_unlock;
+ }
/* Allocate memory for new attribute list. */
new_al_len = base_ni->attr_list_size - le16_to_cpu(ale->length);
new_al = kvzalloc(new_al_len, GFP_NOFS);
- if (!new_al)
- return -ENOMEM;
+ if (!new_al) {
+ up_write(&base_ni->attr_list_lock);
+ err = -ENOMEM;
+ goto out_unlock;
+ }
/* Copy entries from old attribute list to new. */
memcpy(new_al, base_ni->attr_list, (u8 *)ale - base_ni->attr_list);
@@ -295,9 +390,30 @@ int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
ale->length), new_al_len - ((u8 *)ale - base_ni->attr_list));
/* Set new runlist. */
- kvfree(base_ni->attr_list);
+ old_al = base_ni->attr_list;
base_ni->attr_list = new_al;
base_ni->attr_list_size = new_al_len;
+ base_ni->attr_list_gen++;
+ up_write(&base_ni->attr_list_lock);
- return ntfs_attrlist_update(base_ni);
+ err = ntfs_attrlist_update_locked(base_ni);
+ if (err) {
+ rollback = false;
+ down_write(&base_ni->attr_list_lock);
+ if (base_ni->attr_list == new_al) {
+ base_ni->attr_list = old_al;
+ base_ni->attr_list_size += le16_to_cpu(ale->length);
+ base_ni->attr_list_gen++;
+ rollback = true;
+ }
+ up_write(&base_ni->attr_list_lock);
+ if (rollback)
+ kvfree(new_al);
+ goto out_unlock;
+ }
+ kvfree(old_al);
+ err = 0;
+out_unlock:
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+ return err;
}
diff --git a/fs/ntfs/attrlist.h b/fs/ntfs/attrlist.h
index 1892a3934d3a..ecb8a8c957fa 100644
--- a/fs/ntfs/attrlist.h
+++ b/fs/ntfs/attrlist.h
@@ -16,5 +16,6 @@ int ntfs_attrlist_need(struct ntfs_inode *ni);
int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr);
int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx);
int ntfs_attrlist_update(struct ntfs_inode *base_ni);
+int ntfs_attrlist_update_locked(struct ntfs_inode *base_ni);
#endif /* defined _NTFS_ATTRLIST_H */
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 38206e2009e2..ede16196ed0c 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -476,6 +476,7 @@ void __ntfs_init_inode(struct super_block *sb, struct ntfs_inode *ni)
ni->folio_ofs = 0;
ni->mrec = NULL;
init_rwsem(&ni->attr_list_lock);
+ mutex_init(&ni->attr_list_persist_lock);
ni->attr_list_gen = 0;
ni->attr_list_size = 0;
ni->attr_list = NULL;
@@ -3147,7 +3148,7 @@ int ntfs_inode_add_attrlist(struct ntfs_inode *ni)
goto rollback;
}
- err = ntfs_attrlist_update(ni);
+ err = ntfs_attrlist_update_locked(ni);
if (err < 0)
goto remove_attrlist_record;
diff --git a/fs/ntfs/inode.h b/fs/ntfs/inode.h
index ed00e06c5d14..a6c5b15d648d 100644
--- a/fs/ntfs/inode.h
+++ b/fs/ntfs/inode.h
@@ -69,6 +69,7 @@ enum ntfs_inode_mutex_lock_class {
* functions). Setup during read_inode for all inodes with attribute
* lists. Only valid if NI_AttrList is set in state.
* @attr_list_lock: Protects in-memory attribute list state.
+ * @attr_list_persist_lock: Serializes attribute list replacement and persist.
* @attr_list_gen: Generation of the in-memory attribute list state.
* @attr_list_size: Length of attribute list value in bytes.
* @attr_list: Attribute list value itself.
@@ -123,6 +124,8 @@ struct ntfs_inode {
s64 mft_lcn[2];
unsigned int mft_lcn_count;
struct rw_semaphore attr_list_lock;
+ /* Serializes attribute list replacement and persist. */
+ struct mutex attr_list_persist_lock;
u64 attr_list_gen;
u32 attr_list_size;
u8 *attr_list;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 3/4] ntfs: protect attribute-list creation/teardown with attr_list_persist_lock
2026-08-12 1:18 [PATCH v7 0/4] ntfs: serialize attribute-list access Hyunchul Lee
2026-08-12 1:18 ` [PATCH v7 1/4] ntfs: replace raw ctx->al_entry with restartable attr-list locators Hyunchul Lee
2026-08-12 1:18 ` [PATCH v7 2/4] ntfs: protect attribute-list buffer replacement with attr_list_persist_lock Hyunchul Lee
@ 2026-08-12 1:18 ` Hyunchul Lee
2026-08-12 1:18 ` [PATCH v7 4/4] ntfs: protect mapping-pairs attr-list updates " Hyunchul Lee
2026-10-17 2:54 ` [PATCH v7 0/4] ntfs: serialize attribute-list access Cen Zhang
4 siblings, 0 replies; 7+ messages in thread
From: Hyunchul Lee @ 2026-08-12 1:18 UTC (permalink / raw)
To: Namjae Jeon; +Cc: Hyunchul Lee, ntfs, Cen Zhang, Copilot, stable
ntfs_attr_record_rm() and ntfs_inode_add_attrlist() update the
in-memory attribute list without any lock, so a concurrent reader can
observe an inconsistent trio. Teardown can also free a buffer that a
concurrent ntfs_attrlist_entry_add()/rm() transaction still owns while
it sleeps in ntfs_attrlist_update().
Protect both functions with attr_list_persist_lock for their whole
publish/persist or teardown sequence, and with attr_list_lock (write)
around the actual buffer pointer/size/gen updates.
Take attr_list_persist_lock for every non-$ATTRIBUTE_LIST removal even
when no list is currently present, so creation cannot publish a stale
entry after the initial state check. Initialize the shared removal
result for successful paths, and route all attribute-list creation
failures through a common unlock path.
Reported-by: Cen Zhang <zzzccc427@gmail.com>
Link: https://lore.kernel.org/all/20260702161710.2606853-1-zzzccc427@gmail.com/
Fixes: 495e90fa3348 ("ntfs: update attrib operations")
Cc: stable@vger.kernel.org
Tested-by: Cen Zhang <zzzccc427@gmail.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
fs/ntfs/attrib.c | 141 ++++++++++++++++++++++++++++++++++++++++++-----------
fs/ntfs/attrib.h | 2 +-
fs/ntfs/attrlist.c | 34 ++++++-------
fs/ntfs/attrlist.h | 2 +-
fs/ntfs/index.c | 23 +++++++--
fs/ntfs/inode.c | 33 ++++++++++---
fs/ntfs/namei.c | 2 +-
fs/ntfs/super.c | 2 +-
8 files changed, 179 insertions(+), 60 deletions(-)
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index ef918a73fb0c..e52acbced9c5 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -2801,16 +2801,20 @@ static int ntfs_non_resident_attr_record_add(struct ntfs_inode *ni, __le32 type,
/*
* ntfs_attr_record_rm - remove attribute extent
- * @ctx: search context describing the attribute which should be removed
+ * @ctx: search context describing the attribute which should be removed
+ * @persist_locked: true if the caller already holds
+ * base_ni->attr_list_persist_lock for the in-flight
+ * transaction this removal is part of
*
* If this function succeed, user should reinit search context if he/she wants
* use it anymore.
*/
-int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx)
+int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx, bool persist_locked)
{
struct ntfs_inode *base_ni, *ni;
__le32 type;
- int err;
+ int err = 0;
+ bool attrlist_locked = persist_locked;
if (!ctx || !ctx->ntfs_ino || !ctx->mrec || !ctx->attr)
return -EINVAL;
@@ -2825,10 +2829,17 @@ int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx)
else
base_ni = ctx->ntfs_ino;
+ /* Keep ALE removal and the follow-up need check in one transaction. */
+ if (!attrlist_locked && type != AT_ATTRIBUTE_LIST) {
+ mutex_lock(&base_ni->attr_list_persist_lock);
+ attrlist_locked = true;
+ }
+
/* Remove attribute itself. */
if (ntfs_attr_record_resize(ctx->mrec, ctx->attr, 0)) {
ntfs_debug("Couldn't remove attribute record. Bug or damaged MFT record.\n");
- return -EIO;
+ err = -EIO;
+ goto out_unlock;
}
mark_mft_record_dirty(ni);
@@ -2837,19 +2848,42 @@ int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx)
* delete $ATTRIBUTE_LIST itself.
*/
if (NInoAttrList(base_ni) && type != AT_ATTRIBUTE_LIST) {
- err = ntfs_attrlist_entry_rm(ctx);
+ err = ntfs_attrlist_entry_rm_locked(ctx);
if (err) {
ntfs_debug("Couldn't delete record from $ATTRIBUTE_LIST.\n");
- return err;
+ goto out_unlock;
}
}
/* Post $ATTRIBUTE_LIST delete setup. */
if (type == AT_ATTRIBUTE_LIST) {
+ /*
+ * attr_list_persist_lock serializes this in-memory attr_list
+ * teardown against a concurrent ntfs_attrlist_entry_add()/rm()
+ * transaction on the same inode.
+ *
+ * @persist_locked is true when we got here from
+ * ntfs_attr_update_mapping_pairs() rebuilding the mapping
+ * pairs of the $ATTRIBUTE_LIST attribute itself: that call
+ * only happens underneath ntfs_attrlist_update_locked(), which
+ * is always invoked while already holding this same mutex for
+ * the in-flight transaction. Taking it again here would
+ * deadlock the caller against itself, so skip the
+ * (re-)acquisition in that case and rely on the lock already
+ * held further up the stack.
+ */
+ if (!persist_locked)
+ mutex_lock(&base_ni->attr_list_persist_lock);
+ down_write(&base_ni->attr_list_lock);
if (NInoAttrList(base_ni) && base_ni->attr_list)
kvfree(base_ni->attr_list);
base_ni->attr_list = NULL;
+ base_ni->attr_list_size = 0;
+ base_ni->attr_list_gen++;
NInoClearAttrList(base_ni);
+ up_write(&base_ni->attr_list_lock);
+ if (!persist_locked)
+ mutex_unlock(&base_ni->attr_list_persist_lock);
}
/* Free MFT record, if it doesn't contain attributes. */
@@ -2857,17 +2891,18 @@ int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx)
le16_to_cpu(ctx->mrec->attrs_offset) == 8) {
if (ntfs_mft_record_free(ni->vol, ni)) {
ntfs_debug("Couldn't free MFT record.\n");
- return -EIO;
+ err = -EIO;
+ goto out_unlock;
}
/* Remove done if we freed base inode. */
if (ni == base_ni)
- return 0;
+ goto out_unlock;
ntfs_inode_close(ni);
ctx->ntfs_ino = ni = NULL;
}
if (type == AT_ATTRIBUTE_LIST || !NInoAttrList(base_ni))
- return 0;
+ goto out_unlock;
/* Remove attribute list if we don't need it any more. */
if (!ntfs_attrlist_need(base_ni)) {
@@ -2878,7 +2913,7 @@ int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx)
if (ntfs_attr_lookup(AT_ATTRIBUTE_LIST, NULL, 0, CASE_SENSITIVE,
0, NULL, 0, ctx)) {
ntfs_debug("Couldn't find attribute list. Succeed anyway.\n");
- return 0;
+ goto out_unlock;
}
/* Deallocate clusters. */
if (ctx->attr->non_resident) {
@@ -2889,16 +2924,16 @@ int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx)
ctx->attr, NULL, &new_rl_count);
if (IS_ERR(al_rl)) {
ntfs_debug("Couldn't decompress attribute list runlist. Succeed anyway.\n");
- return 0;
+ goto out_unlock;
}
if (ntfs_cluster_free_from_rl(base_ni->vol, al_rl))
ntfs_debug("Leaking clusters! Run chkdsk. Couldn't free clusters from attribute list runlist.\n");
kvfree(al_rl);
}
/* Remove attribute record itself. */
- if (ntfs_attr_record_rm(ctx)) {
+ if (ntfs_attr_record_rm(ctx, true)) {
ntfs_debug("Couldn't remove attribute list. Succeed anyway.\n");
- return 0;
+ goto out_unlock;
}
na.mft_no = VFS_I(base_ni)->i_ino;
@@ -2914,7 +2949,10 @@ int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx)
}
}
- return 0;
+out_unlock:
+ if (attrlist_locked && !persist_locked)
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+ return err;
}
/*
@@ -3545,6 +3583,12 @@ int ntfs_attr_map_whole_runlist(struct ntfs_inode *ni)
* ntfs_attr_record_move_to - move attribute record to target inode
* @ctx: attribute search context describing the attribute record
* @ni: opened ntfs inode to which move attribute record
+ *
+ * Caller must hold the base inode's attr_list_persist_lock: this rewrites
+ * the moved attribute's ALE in place, and that mutation has to be part of
+ * the same transaction as the persist that follows it. Otherwise a
+ * concurrent ntfs_attrlist_update_locked() can copy the attribute list to
+ * disk while only one of @mft_reference/@instance has been updated.
*/
int ntfs_attr_record_move_to(struct ntfs_attr_search_ctx *ctx, struct ntfs_inode *ni)
{
@@ -3627,6 +3671,8 @@ int ntfs_attr_record_move_to(struct ntfs_attr_search_ctx *ctx, struct ntfs_inode
a = (struct attr_record *)nctx->attr;
base_ni = ntfs_attr_ctx_base_ni(ctx);
+ lockdep_assert_held(&base_ni->attr_list_persist_lock);
+
down_write(&base_ni->attr_list_lock);
ale = ntfs_attrlist_find_exact_locked(base_ni, &ctx->al_exact);
if (!ale) {
@@ -4049,7 +4095,14 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
ntfs_debug("Attribute list is too big. Defragment the volume\n");
return -ENOSPC;
}
- if (ntfs_attrlist_update(base_ni))
+ /*
+ * This call is only ever reached while
+ * rebuilding the $ATTRIBUTE_LIST attribute's
+ * own mapping pairs, which happens underneath
+ * ntfs_attrlist_update_locked() while holding
+ * persist lock.
+ */
+ if (ntfs_attrlist_update_locked(base_ni))
return -EIO;
goto retry;
}
@@ -4185,8 +4238,16 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
if (le64_to_cpu(ctx->attr->data.non_resident.highest_vcn) !=
NTFS_VCN_DELETE_MARK)
continue;
- /* Remove unused attribute record. */
- err = ntfs_attr_record_rm(ctx);
+ /*
+ * Remove unused attribute record. When @ni is the
+ * $ATTRIBUTE_LIST attribute itself, we only get here
+ * underneath ntfs_attrlist_update_locked(), whose
+ * caller already holds
+ * base_ni->attr_list_persist_lock for this
+ * transaction, so tell ntfs_attr_record_rm() not to
+ * recurse into it.
+ */
+ err = ntfs_attr_record_rm(ctx, ni->type == AT_ATTRIBUTE_LIST);
if (err) {
ntfs_error(sb, "Could not remove unused attr");
goto put_err_out;
@@ -5014,30 +5075,42 @@ static int ntfs_resident_attr_resize(struct ntfs_inode *attr_ni, const s64 newsi
*/
if (attr_ni->type == AT_STANDARD_INFORMATION ||
attr_ni->type == AT_ATTRIBUTE_LIST) {
+ /*
+ * Resizing the $ATTRIBUTE_LIST attribute itself only happens
+ * underneath ntfs_attrlist_update_locked() while already holding
+ * attr_list_persist_lock.
+ */
+ bool persist_locked = attr_ni->type == AT_ATTRIBUTE_LIST;
+
ntfs_attr_put_search_ctx(ctx);
if (!NInoAttrList(base_ni)) {
+ /* This takes attr_list_persist_lock on its own. */
err = ntfs_inode_add_attrlist(base_ni);
if (err)
return err;
}
+ /*
+ * ntfs_inode_free_space() moves attributes out of the base MFT
+ * record, rewriting their ALEs in place. Keep those mutations
+ * and the persist below inside one transaction.
+ */
+ if (!persist_locked)
+ mutex_lock(&base_ni->attr_list_persist_lock);
+
err = ntfs_inode_free_space(base_ni, sizeof(struct attr_record));
if (err) {
+ if (!persist_locked)
+ mutex_unlock(&base_ni->attr_list_persist_lock);
err = -ENOSPC;
ntfs_error(sb,
"Couldn't free space in the MFT record to make attribute list non resident");
return err;
}
- /*
- * Resizing the $ATTRIBUTE_LIST attribute itself only happens
- * underneath ntfs_attrlist_update_locked() while already holding
- * attr_list_persist_lock.
- */
- if (attr_ni->type == AT_ATTRIBUTE_LIST)
- err = ntfs_attrlist_update_locked(base_ni);
- else
- err = ntfs_attrlist_update(base_ni);
+ err = ntfs_attrlist_update_locked(base_ni);
+ if (!persist_locked)
+ mutex_unlock(&base_ni->attr_list_persist_lock);
if (err)
return err;
goto attr_resize_again;
@@ -5085,15 +5158,27 @@ static int ntfs_resident_attr_resize(struct ntfs_inode *attr_ni, const s64 newsi
}
unmap_mft_record(ext_ni);
+ /*
+ * Move the attribute and persist the resulting attribute list as one
+ * transaction, so the ALE rewritten by ntfs_attr_record_move_to()
+ * cannot be copied to disk half-updated. attr_ni->type is neither
+ * $STANDARD_INFORMATION nor $ATTRIBUTE_LIST here (both are handled
+ * above), so we cannot already hold the lock via
+ * ntfs_attrlist_update_locked().
+ */
+ mutex_lock(&base_ni->attr_list_persist_lock);
+
/* Move attribute to it. */
err = ntfs_attr_record_move_to(ctx, ext_ni);
if (err) {
+ mutex_unlock(&base_ni->attr_list_persist_lock);
ntfs_error(sb, "Couldn't move attribute to new MFT record");
err = -ENOMEM;
goto put_err_out;
}
- err = ntfs_attrlist_update(base_ni);
+ err = ntfs_attrlist_update_locked(base_ni);
+ mutex_unlock(&base_ni->attr_list_persist_lock);
if (err < 0)
goto put_err_out;
@@ -5454,7 +5539,7 @@ int ntfs_attr_rm(struct ntfs_inode *ni)
}
while (!(err = ntfs_attr_lookup(ni->type, ni->name, ni->name_len,
CASE_SENSITIVE, 0, NULL, 0, ctx))) {
- err = ntfs_attr_record_rm(ctx);
+ err = ntfs_attr_record_rm(ctx, false);
if (err) {
ntfs_error(sb,
"Failed to remove attribute extent. Leaving inconstant metadata.\n");
diff --git a/fs/ntfs/attrib.h b/fs/ntfs/attrib.h
index 2e5f87abd852..ec0c846febf1 100644
--- a/fs/ntfs/attrib.h
+++ b/fs/ntfs/attrib.h
@@ -161,7 +161,7 @@ int ntfs_attr_exist(struct ntfs_inode *ni, const __le32 type, __le16 *name,
u32 name_len);
int ntfs_attr_remove(struct ntfs_inode *ni, const __le32 type, __le16 *name,
u32 name_len);
-int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx);
+int ntfs_attr_record_rm(struct ntfs_attr_search_ctx *ctx, bool persist_locked);
int ntfs_attr_record_move_to(struct ntfs_attr_search_ctx *ctx, struct ntfs_inode *ni);
int ntfs_attr_add(struct ntfs_inode *ni, __le32 type,
__le16 *name, u8 name_len, u8 *val, s64 size);
diff --git a/fs/ntfs/attrlist.c b/fs/ntfs/attrlist.c
index aa6d5f10bf96..bdea09efe284 100644
--- a/fs/ntfs/attrlist.c
+++ b/fs/ntfs/attrlist.c
@@ -117,23 +117,6 @@ int ntfs_attrlist_update_locked(struct ntfs_inode *base_ni)
return 0;
}
-/*
- * ntfs_attrlist_update - persist the in-memory attribute list to disk
- * @base_ni: base ntfs inode containing the attribute list
- *
- * Serialize the persist against concurrent attribute-list replacement
- * transactions.
- */
-int ntfs_attrlist_update(struct ntfs_inode *base_ni)
-{
- int err;
-
- mutex_lock(&base_ni->attr_list_persist_lock);
- err = ntfs_attrlist_update_locked(base_ni);
- mutex_unlock(&base_ni->attr_list_persist_lock);
- return err;
-}
-
/*
* ntfs_attrlist_entry_add - add an attribute list attribute entry
* @ni: opened ntfs inode, which contains that attribute
@@ -328,7 +311,7 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
*
* Return 0 on success and -errno on error.
*/
-int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
+int ntfs_attrlist_entry_rm_locked(struct ntfs_attr_search_ctx *ctx)
{
u8 *new_al = NULL;
int err, new_al_len;
@@ -356,7 +339,7 @@ int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
ntfs_debug("Attribute list isn't present.\n");
return -ENOENT;
}
- mutex_lock(&base_ni->attr_list_persist_lock);
+ lockdep_assert_held(&base_ni->attr_list_persist_lock);
/*
* Another thread may have removed the attribute list while we were
@@ -414,6 +397,19 @@ int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
kvfree(old_al);
err = 0;
out_unlock:
+ return err;
+}
+
+int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx)
+{
+ struct ntfs_inode *base_ni;
+ int err;
+
+ if (!ctx || !ctx->ntfs_ino)
+ return -EINVAL;
+ base_ni = ctx->base_ntfs_ino ? ctx->base_ntfs_ino : ctx->ntfs_ino;
+ mutex_lock(&base_ni->attr_list_persist_lock);
+ err = ntfs_attrlist_entry_rm_locked(ctx);
mutex_unlock(&base_ni->attr_list_persist_lock);
return err;
}
diff --git a/fs/ntfs/attrlist.h b/fs/ntfs/attrlist.h
index ecb8a8c957fa..ccf61971c01e 100644
--- a/fs/ntfs/attrlist.h
+++ b/fs/ntfs/attrlist.h
@@ -15,7 +15,7 @@
int ntfs_attrlist_need(struct ntfs_inode *ni);
int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr);
int ntfs_attrlist_entry_rm(struct ntfs_attr_search_ctx *ctx);
-int ntfs_attrlist_update(struct ntfs_inode *base_ni);
+int ntfs_attrlist_entry_rm_locked(struct ntfs_attr_search_ctx *ctx);
int ntfs_attrlist_update_locked(struct ntfs_inode *base_ni);
#endif /* defined _NTFS_ATTRLIST_H */
diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index 082fe9226c72..8c44ec25c2a2 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -1348,18 +1348,35 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
*/
if ((ret == -ENOSPC) &&
(ctx->al_cursor.valid || !ntfs_inode_add_attrlist(icx->idx_ni))) {
+ struct ntfs_inode *base_ni;
+
ntfs_attr_put_search_ctx(ctx);
ctx = NULL;
ir = ntfs_ir_lookup(icx->idx_ni, icx->name, icx->name_len, &ctx);
- if (ir && !ntfs_attr_record_move_away(ctx, ix_root_size -
+ if (!ir)
+ goto clear_bmp;
+
+ base_ni = ctx->base_ntfs_ino ? ctx->base_ntfs_ino : ctx->ntfs_ino;
+ /*
+ * ntfs_attr_record_move_away() rewrites the moved attribute's
+ * ALE in place, so it and the persist below have to form one
+ * transaction. Nothing up this call chain holds the lock:
+ * ntfs_inode_add_attrlist() above takes it on its own, and
+ * ntfs_attrlist_update_locked() never reaches index code.
+ */
+ mutex_lock(&base_ni->attr_list_persist_lock);
+ if (!ntfs_attr_record_move_away(ctx, ix_root_size -
le32_to_cpu(ctx->attr->data.resident.value_length))) {
- if (ntfs_attrlist_update(ctx->base_ntfs_ino ?
- ctx->base_ntfs_ino : ctx->ntfs_ino))
+ if (ntfs_attrlist_update_locked(base_ni)) {
+ mutex_unlock(&base_ni->attr_list_persist_lock);
goto clear_bmp;
+ }
+ mutex_unlock(&base_ni->attr_list_persist_lock);
ntfs_attr_put_search_ctx(ctx);
ctx = NULL;
goto retry;
}
+ mutex_unlock(&base_ni->attr_list_persist_lock);
}
clear_bmp:
ntfs_ibm_clear(icx, new_ib_vcn);
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index ede16196ed0c..a304a2ca22d8 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -3054,14 +3054,20 @@ int ntfs_inode_add_attrlist(struct ntfs_inode *ni)
ntfs_debug("inode %llu\n", ni->mft_no);
+ /* Serialize the initial state check and attribute-list publication. */
+ mutex_lock(&ni->attr_list_persist_lock);
+
if (NInoAttrList(ni) || ni->nr_extents) {
ntfs_error(ni->vol->sb, "Inode already has attribute list");
- return -EEXIST;
+ err = -EEXIST;
+ goto out_unlock;
}
ni_mrec = map_mft_record(ni);
- if (IS_ERR(ni_mrec))
- return -EIO;
+ if (IS_ERR(ni_mrec)) {
+ err = -EIO;
+ goto out_unlock;
+ }
/* Form attribute list. */
ctx = ntfs_attr_get_search_ctx(ni, ni_mrec);
@@ -3123,9 +3129,12 @@ int ntfs_inode_add_attrlist(struct ntfs_inode *ni)
}
/* Set in-memory attribute list. */
+ down_write(&ni->attr_list_lock);
ni->attr_list = al;
ni->attr_list_size = al_len;
+ ni->attr_list_gen++;
NInoSetAttrList(ni);
+ up_write(&ni->attr_list_lock);
attr_al_len = offsetof(struct attr_record, data.resident.reserved) + 1 +
((al_len + 7) & ~7);
@@ -3151,29 +3160,35 @@ int ntfs_inode_add_attrlist(struct ntfs_inode *ni)
err = ntfs_attrlist_update_locked(ni);
if (err < 0)
goto remove_attrlist_record;
-
+ mutex_unlock(&ni->attr_list_persist_lock);
ntfs_attr_put_search_ctx(ctx);
unmap_mft_record(ni);
return 0;
remove_attrlist_record:
- /* Prevent ntfs_attr_recorm_rm from freeing attribute list. */
+ down_write(&ni->attr_list_lock);
ni->attr_list = NULL;
+ ni->attr_list_gen++;
NInoClearAttrList(ni);
+ up_write(&ni->attr_list_lock);
+
/* Remove $ATTRIBUTE_LIST record. */
ntfs_attr_reinit_search_ctx(ctx);
if (!ntfs_attr_lookup(AT_ATTRIBUTE_LIST, NULL, 0,
CASE_SENSITIVE, 0, NULL, 0, ctx)) {
- if (ntfs_attr_record_rm(ctx))
+ if (ntfs_attr_record_rm(ctx, true))
ntfs_error(ni->vol->sb, "Rollback failed to remove attrlist");
} else {
ntfs_error(ni->vol->sb, "Rollback failed to find attrlist");
}
/* Setup back in-memory runlist. */
+ down_write(&ni->attr_list_lock);
ni->attr_list = al;
ni->attr_list_size = al_len;
+ ni->attr_list_gen++;
NInoSetAttrList(ni);
+ up_write(&ni->attr_list_lock);
rollback:
/*
* Scan attribute list for attributes that placed not in the base MFT
@@ -3200,15 +3215,21 @@ int ntfs_inode_add_attrlist(struct ntfs_inode *ni)
}
/* Remove in-memory attribute list. */
+ down_write(&ni->attr_list_lock);
ni->attr_list = NULL;
ni->attr_list_size = 0;
+ ni->attr_list_gen++;
NInoClearAttrList(ni);
NInoClearAttrListDirty(ni);
+ up_write(&ni->attr_list_lock);
+
put_err_out:
ntfs_attr_put_search_ctx(ctx);
err_out:
kvfree(al);
unmap_mft_record(ni);
+out_unlock:
+ mutex_unlock(&ni->attr_list_persist_lock);
return err;
}
diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index 96045face63f..09deb96533d1 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -938,7 +938,7 @@ static int ntfs_delete(struct ntfs_inode *ni, struct ntfs_inode *dir_ni,
if (err)
goto err_out;
- err = ntfs_attr_record_rm(actx);
+ err = ntfs_attr_record_rm(actx, false);
if (err)
goto err_out;
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 8abe7bee4c0d..6d28c4c25a05 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -485,7 +485,7 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
ret = ntfs_attr_lookup(AT_VOLUME_NAME, NULL, 0, 0, 0, NULL, 0,
ctx);
if (!ret)
- ret = ntfs_attr_record_rm(ctx);
+ ret = ntfs_attr_record_rm(ctx, false);
else if (ret == -ENOENT)
ret = 0;
ntfs_attr_put_search_ctx(ctx);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 4/4] ntfs: protect mapping-pairs attr-list updates with attr_list_persist_lock
2026-08-12 1:18 [PATCH v7 0/4] ntfs: serialize attribute-list access Hyunchul Lee
` (2 preceding siblings ...)
2026-08-12 1:18 ` [PATCH v7 3/4] ntfs: protect attribute-list creation/teardown " Hyunchul Lee
@ 2026-08-12 1:18 ` Hyunchul Lee
2026-10-17 2:54 ` [PATCH v7 0/4] ntfs: serialize attribute-list access Cen Zhang
4 siblings, 0 replies; 7+ messages in thread
From: Hyunchul Lee @ 2026-08-12 1:18 UTC (permalink / raw)
To: Namjae Jeon; +Cc: Hyunchul Lee, ntfs, Cen Zhang, Copilot, stable
ntfs_attr_update_mapping_pairs() updates several ALEs while rebuilding
a fragmented attribute's mapping pairs and persists the attribute
list once after the loop. Each ALE update is covered by
attr_list_lock, but the overall publish/persist sequence is not
serialized against concurrent ntfs_attrlist_entry_add()/rm() on the
same base inode.
Acquire attr_list_persist_lock once after resolving base_ni whenever
the call can touch the attribute list, and hold it across every ALE
update and the deferred ntfs_attrlist_update() call.
Reported-by: Cen Zhang <zzzccc427@gmail.com>
Link: https://lore.kernel.org/all/20260702161710.2606853-1-zzzccc427@gmail.com/
Fixes: 495e90fa3348 ("ntfs: update attrib operations")
Cc: stable@vger.kernel.org
Tested-by: Cen Zhang <zzzccc427@gmail.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
---
fs/ntfs/attrib.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
fs/ntfs/inode.c | 46 ++++++++++++++++++++++++++++++++++++++++----
fs/ntfs/inode.h | 1 +
3 files changed, 97 insertions(+), 8 deletions(-)
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index e52acbced9c5..d3621e5c24df 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -3717,6 +3717,7 @@ int ntfs_attr_record_move_away(struct ntfs_attr_search_ctx *ctx, int extra)
base_ni = ctx->base_ntfs_ino;
else
base_ni = ctx->ntfs_ino;
+ lockdep_assert_held(&base_ni->attr_list_persist_lock);
sb = ctx->ntfs_ino->vol->sb;
if (!NInoAttrList(base_ni)) {
@@ -3725,7 +3726,7 @@ int ntfs_attr_record_move_away(struct ntfs_attr_search_ctx *ctx, int extra)
return -EINVAL;
}
- err = ntfs_inode_attach_all_extents(ctx->ntfs_ino);
+ err = ntfs_inode_attach_all_extents_locked(ctx->ntfs_ino);
if (err) {
ntfs_error(sb, "Couldn't attach extents, inode=%llu",
(unsigned long long)base_ni->mft_no);
@@ -3789,7 +3790,8 @@ int ntfs_attr_record_move_away(struct ntfs_attr_search_ctx *ctx, int extra)
* update allocated and compressed size.
*/
static int ntfs_attr_update_meta(struct attr_record *a, struct ntfs_inode *ni,
- struct mft_record *m, struct ntfs_attr_search_ctx *ctx)
+ struct mft_record *m, struct ntfs_attr_search_ctx *ctx,
+ bool *attrlist_locked)
{
int sparse, err = 0;
struct ntfs_inode *base_ni;
@@ -3826,6 +3828,19 @@ static int ntfs_attr_update_meta(struct attr_record *a, struct ntfs_inode *ni,
!(le32_to_cpu(m->bytes_allocated) - le32_to_cpu(m->bytes_in_use))) {
if (!NInoAttrList(base_ni)) {
+ /*
+ * ntfs_inode_add_attrlist() acquires
+ * attr_list_persist_lock itself, so drop it
+ * first if our caller took it for us. Nothing
+ * is in flight that needs protecting: without
+ * an attribute list no ALE has been touched
+ * yet. The -EAGAIN below makes the caller
+ * restart and re-acquire the lock.
+ */
+ if (*attrlist_locked) {
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+ *attrlist_locked = false;
+ }
err = ntfs_inode_add_attrlist(base_ni);
if (err)
goto out;
@@ -3952,7 +3967,7 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
struct attr_record *a;
s64 stop_vcn;
int err = 0, mp_size, cur_max_mp_size, exp_max_mp_size;
- bool finished_build, attrlist_changed = false;
+ bool finished_build, attrlist_changed = false, attrlist_locked = false;
bool first_updated = false;
struct super_block *sb;
struct runlist_element *start_rl;
@@ -3977,9 +3992,16 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
else
base_ni = ni;
+ if (ni->type != AT_ATTRIBUTE_LIST) {
+ mutex_lock(&base_ni->attr_list_persist_lock);
+ attrlist_locked = true;
+ }
+
ctx = ntfs_attr_get_search_ctx(base_ni, NULL);
if (!ctx) {
ntfs_error(sb, "%s: Failed to get search context", __func__);
+ if (attrlist_locked)
+ mutex_unlock(&base_ni->attr_list_persist_lock);
return -ENOMEM;
}
@@ -4050,9 +4072,13 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
continue;
}
- err = ntfs_attr_update_meta(a, ni, m, ctx);
+ err = ntfs_attr_update_meta(a, ni, m, ctx, &attrlist_locked);
if (err < 0) {
if (err == -EAGAIN) {
+ if (attrlist_locked) {
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+ attrlist_locked = false;
+ }
ntfs_attr_put_search_ctx(ctx);
goto retry;
}
@@ -4089,6 +4115,10 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
* attributes and try again.
*/
if (ni->type == AT_ATTRIBUTE_LIST) {
+ if (WARN_ON_ONCE(attrlist_locked)) {
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+ attrlist_locked = false;
+ }
ntfs_attr_put_search_ctx(ctx);
if (ntfs_inode_free_space(base_ni, mp_size -
cur_max_mp_size)) {
@@ -4109,6 +4139,18 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
/* Add attribute list if it isn't present, and retry. */
if (!NInoAttrList(base_ni)) {
+ /*
+ * We hold attr_list_persist_lock even when the
+ * inode has no attribute list yet, since one
+ * can appear under us. ntfs_inode_add_attrlist()
+ * takes the same lock, so drop it here; no ALE
+ * has been touched, so there is no transaction
+ * to break, and the retry re-acquires it.
+ */
+ if (attrlist_locked) {
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+ attrlist_locked = false;
+ }
ntfs_attr_put_search_ctx(ctx);
if (ntfs_inode_add_attrlist(base_ni)) {
ntfs_error(sb, "Can not add attrlist");
@@ -4225,6 +4267,12 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
if (err)
goto put_err_out;
}
+ if (attrlist_locked) {
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+ attrlist_locked = false;
+ }
+ if (attrlist_changed && err)
+ goto put_err_out;
/* Deallocate not used attribute extents and return with success. */
if (finished_build) {
@@ -4351,6 +4399,8 @@ int ntfs_attr_update_mapping_pairs(struct ntfs_inode *ni, s64 from_vcn)
return 0;
put_err_out:
+ if (attrlist_locked)
+ mutex_unlock(&base_ni->attr_list_persist_lock);
if (ctx)
ntfs_attr_put_search_ctx(ctx);
return err;
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index a304a2ca22d8..60a1b690dbea 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -2921,6 +2921,7 @@ static struct ntfs_inode *ntfs_extent_inode_open(struct ntfs_inode *base_ni,
if (!base_ni)
return NULL;
+ lockdep_assert_held(&base_ni->extent_lock);
sb = base_ni->vol->sb;
ntfs_debug("Opening extent inode %llu (base mft record %llu).\n",
@@ -2988,15 +2989,20 @@ static struct ntfs_inode *ntfs_extent_inode_open(struct ntfs_inode *base_ni,
}
/*
- * ntfs_inode_attach_all_extents - attach all extents for target inode
+ * ntfs_inode_attach_all_extents_locked - attach all extents for target inode
* @ni: opened ntfs inode for which perform attach
*
+ * Caller must hold the base inode's attr_list_persist_lock. The attribute
+ * list buffer is replaced and freed by transactions serialized by this lock,
+ * so walking it without the lock can dereference a stale buffer.
+ *
* Return 0 on success and error.
*/
-int ntfs_inode_attach_all_extents(struct ntfs_inode *ni)
+int ntfs_inode_attach_all_extents_locked(struct ntfs_inode *ni)
{
struct attr_list_entry *ale;
u64 prev_attached = 0;
+ int err = 0;
if (!ni) {
ntfs_debug("Invalid arguments.\n");
@@ -3005,6 +3011,7 @@ int ntfs_inode_attach_all_extents(struct ntfs_inode *ni)
if (NInoAttr(ni))
ni = ni->ext.base_ntfs_ino;
+ lockdep_assert_held(&ni->attr_list_persist_lock);
ntfs_debug("Entering for inode 0x%llx.\n", ni->mft_no);
@@ -3018,19 +3025,50 @@ int ntfs_inode_attach_all_extents(struct ntfs_inode *ni)
}
/* Walk through attribute list and attach all extents. */
+ mutex_lock(&ni->extent_lock);
ale = (struct attr_list_entry *)ni->attr_list;
while ((u8 *)ale < ni->attr_list + ni->attr_list_size) {
if (ni->mft_no != MREF_LE(ale->mft_reference) &&
prev_attached != MREF_LE(ale->mft_reference)) {
if (!ntfs_extent_inode_open(ni, ale->mft_reference)) {
ntfs_debug("Couldn't attach extent inode.\n");
- return -1;
+ err = -1;
+ break;
}
prev_attached = MREF_LE(ale->mft_reference);
}
ale = (struct attr_list_entry *)((u8 *)ale + le16_to_cpu(ale->length));
}
- return 0;
+ mutex_unlock(&ni->extent_lock);
+ return err;
+}
+
+/*
+ * ntfs_inode_attach_all_extents - attach all extents for target inode
+ * @ni: opened ntfs inode for which perform attach
+ *
+ * Return 0 on success and error.
+ */
+int ntfs_inode_attach_all_extents(struct ntfs_inode *ni)
+{
+ struct ntfs_inode *base_ni;
+ int err;
+
+ if (!ni) {
+ ntfs_debug("Invalid arguments.\n");
+ return -EINVAL;
+ }
+
+ if (NInoAttr(ni))
+ base_ni = ni->ext.base_ntfs_ino;
+ else
+ base_ni = ni;
+
+ mutex_lock(&base_ni->attr_list_persist_lock);
+ err = ntfs_inode_attach_all_extents_locked(base_ni);
+ mutex_unlock(&base_ni->attr_list_persist_lock);
+
+ return err;
}
/*
diff --git a/fs/ntfs/inode.h b/fs/ntfs/inode.h
index a6c5b15d648d..a02ef1e59044 100644
--- a/fs/ntfs/inode.h
+++ b/fs/ntfs/inode.h
@@ -346,6 +346,7 @@ int ntfs_getattr(struct mnt_idmap *idmap, const struct path *path,
int ntfs_get_block_mft_record(struct ntfs_inode *mft_ni, struct ntfs_inode *ni);
int __ntfs_write_inode(struct inode *vi, int sync);
int ntfs_inode_attach_all_extents(struct ntfs_inode *ni);
+int ntfs_inode_attach_all_extents_locked(struct ntfs_inode *ni);
int ntfs_inode_add_attrlist(struct ntfs_inode *ni);
void ntfs_destroy_ext_inode(struct ntfs_inode *ni);
int ntfs_inode_free_space(struct ntfs_inode *ni, int size);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v7 0/4] ntfs: serialize attribute-list access
2026-08-12 1:18 [PATCH v7 0/4] ntfs: serialize attribute-list access Hyunchul Lee
` (3 preceding siblings ...)
2026-08-12 1:18 ` [PATCH v7 4/4] ntfs: protect mapping-pairs attr-list updates " Hyunchul Lee
@ 2026-10-17 2:54 ` Cen Zhang
2026-10-19 6:23 ` Hyunchul Lee
4 siblings, 1 reply; 7+ messages in thread
From: Cen Zhang @ 2026-10-17 2:54 UTC (permalink / raw)
To: Hyunchul Lee; +Cc: Namjae Jeon, ntfs, Copilot, stable
Hi Hyunchul,
Hyunchul Lee <hyc.lee@gmail.com> 于2026年8月12日周三 09:18写道：
>
> ntfs_external_attr_find() walks base_ni->attr_list using raw
> attr_list_entry pointers stored in ctx->al_entry. Because other paths
> can replace base_ni->attr_list and free the old buffer while a lookup
> still holds one of those cursors, a concurrent attribute-list mutation
> can turn a saved cursor into a stale or freed pointer.
>
> Reported in:
> https://lore.kernel.org/all/20260702161710.2606853-1-zzzccc427@gmail.com/
>
> This series fixes that hazard by replacing the raw ctx->al_entry
> pointer with restartable, generation-checked locators, and by adding
> locks that serialize attribute-list buffer replacement and on-disk
> persistence so concurrent mutations cannot corrupt state or double-free
> memory.
>
> The four patches are:
>
> 1. ntfs: replace raw ctx->al_entry with restartable attr-list locators
> 2. ntfs: protect attribute-list buffer replacement with
> attr_list_persist_lock
> 3. ntfs: protect attribute-list creation/teardown with
> attr_list_persist_lock
> 4. ntfs: protect mapping-pairs attr-list updates with
> attr_list_persist_lock
>
> What changes
> ------------
>
> Instead of a raw struct attr_list_entry *, the search context now
> carries three value-only locators:
>
> - al_cursor -- where ntfs_external_attr_find() resumes enumeration;
> invalidated on generation mismatch.
> - al_insert -- insertion point captured on -ENOENT lookups.
> - al_exact -- immutable identity snapshot (type, lowest_vcn,
> mft_reference, instance, name length) used by writers
> to re-locate the same ALE after a buffer replacement.
>
> The inode gains attr_list_lock, an rwsem that protects the live buffer,
> its size and generation, and in-place ALE changes. Readers hold it
> only while dereferencing the buffer; writers hold it while replacing the
> buffer or changing an ALE, then increment the generation. Pointers into
> the buffer never survive an unlock, and the rwsem is dropped before
> persistence or an operation that can take runlist.lock or extent_lock.
>
> attr_list_persist_lock is a non-recursive mutex that serializes a
> mutation through persistence and rollback, including list creation and
> teardown. ntfs_attrlist_update_locked() writes the live buffer without
> attr_list_lock, so the caller holds this mutex from mutation through the
> write. It also stabilizes the buffer while attaching extents. When both
> locks are needed, take attr_list_persist_lock first. Creation owns the
> mutex itself, so callers must create first and then restart; re-entrant
> teardown uses persist_locked instead of recursively acquiring it.
>
> ---
> * Changes in v7:
> * Patch 3: serialize non-$ATTRIBUTE_LIST removal with concurrent
> attribute-list creation.
> * Patch 3: initialize successful removal results and release
> attr_list_persist_lock on every early creation error.
> * Changes in v6:
> * Patch 2: initialize new_al before an early error path can reach its
> cleanup, fixing an x86_64 allmodconfig Clang build warning.
> * Changes in v5:
> * Patch 3: keep each in-place ALE update and its persist in one
> transaction. This fixes the Patch 2 review: a record move could
> update an ALE while persistence copied the same buffer.
> * Patch 3: serialize attribute-list creation, teardown, and extent
> attachment.
> * Patch 4: acquire the mutex unconditionally for mapping-pairs updates to
> close the attribute-list creation TOCTOU; drop it before creating a
> missing list and restart afterward to avoid self-deadlock.
> * Changes in v4:
> * Patch 1: fix attr-list lock/unlock and uninitialized locator on
> the AT_END path
> * Patch 2: hold attr_list_persist_lock around all
> ntfs_attrlist_update() callers and recheck the attribute-list
> state after locking
> * Patch 4: avoid self-deadlock rebuilding $ATTRIBUTE_LIST's own
> mapping pairs
> * Changes in v3:
> * Patch 1: fix $ATTRIBUTE_LIST extent double-delivery during
> mapping-pairs rebuild (xfstests generic/040)
> * Patch 3: fix self-deadlock on attr_list_persist_lock during
> attr-list rebuild (xfstests generic/040)
> * Changes in v2:
> * add Fixes tags
> * add Reported-by and Tested-by tags
>
> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
>
> ---
> Hyunchul Lee (4):
> ntfs: replace raw ctx->al_entry with restartable attr-list locators
> ntfs: protect attribute-list buffer replacement with attr_list_persist_lock
> ntfs: protect attribute-list creation/teardown with attr_list_persist_lock
> ntfs: protect mapping-pairs attr-list updates with attr_list_persist_lock
>
> fs/ntfs/attrib.c | 568 ++++++++++++++++++++++++++++++++++++++++++++---------
> fs/ntfs/attrib.h | 48 ++++-
> fs/ntfs/attrlist.c | 181 +++++++++++++----
> fs/ntfs/attrlist.h | 3 +-
> fs/ntfs/index.c | 26 ++-
> fs/ntfs/inode.c | 116 ++++++++---
> fs/ntfs/inode.h | 10 +
> fs/ntfs/namei.c | 2 +-
> fs/ntfs/super.c | 2 +-
> 9 files changed, 795 insertions(+), 161 deletions(-)
> ---
> base-commit: 5c244a0e9ca95468f4c2087b2c9269825429a13f
> change-id: 20260727-topic-attr-list-lock-v6-f1a909e3c859
>
> Best regards,
> --
> Thanks,
> Hyunchul
>
Just checking if there is any update on the v7 series, or if the issue has
been resolved otherwise.
Thanks again.
Best regards,
Cen Zhang
^ permalink raw reply [flat|nested] 7+ messages in thread