Linux filesystem development
 help / color / mirror / Atom feed
* [PATCH 0/3] ntfs: harden MFT record and attribute parsing
@ 2026-05-08 15:34 DaeMyung Kang
  2026-05-08 15:34 ` [PATCH 1/3] ntfs: validate MFT attrs_offset against bytes_in_use DaeMyung Kang
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: DaeMyung Kang @ 2026-05-08 15:34 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

This series tightens fs/ntfs against malformed on-disk metadata and
fixes one off-by-one in the MFT bitmap scan.

Patches 1/3 and 3/3 are complementary: 1/3 rejects MFT records whose
attrs_offset points past bytes_in_use at record entry, and 3/3 moves
the per-attribute name bounds check earlier so it covers the AT_UNUSED
enumeration path that hands the name pointer back to callers.  Without
3/3, two enumeration paths can read past an attribute record: one in
fs/ntfs/attrib.c passes the returned name pointer to ntfs_attr_iget(),
and another in fs/ntfs/inode.c copies the name while building an
attribute list.

Patch 2/3 is independent: ntfs_mft_record_layout() rejects
mft_no >= 2^32, but the bitmap scan in
ntfs_mft_bitmap_find_and_alloc_free_rec_nolock() used '>'.  Bring it in
line with the other 2^32 boundary checks in fs/ntfs/mft.c.

All three carry the same Fixes tag (d3ad708fecaa) since the issues
date from the initial fs/ntfs commit in this tree.

DaeMyung Kang (3):
  ntfs: validate MFT attrs_offset against bytes_in_use
  ntfs: fix MFT bitmap scan 2^32 boundary check
  ntfs: validate attribute name bounds before returning it

 fs/ntfs/attrib.c | 25 +++++++++++++++++--------
 fs/ntfs/mft.c    | 16 ++++++++++++++--
 2 files changed, 31 insertions(+), 10 deletions(-)

--
2.34.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] ntfs: validate MFT attrs_offset against bytes_in_use
  2026-05-08 15:34 [PATCH 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
@ 2026-05-08 15:34 ` DaeMyung Kang
  2026-05-08 15:34 ` [PATCH 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check DaeMyung Kang
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: DaeMyung Kang @ 2026-05-08 15:34 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

ntfs_mft_record_check() verifies that attrs_offset is aligned and that
the resulting pointer stays within the allocated MFT record buffer, but
it does not check that the first attribute header starts within the
bytes_in_use area.

A malformed record with attrs_offset greater than bytes_in_use can pass
this check as long as attrs_offset is still within bytes_allocated.  The
attribute parser then computes the remaining record space by subtracting
the attribute pointer from bytes_in_use.  Because that value is unsigned,
the subtraction can underflow and allow bytes after bytes_in_use to be
interpreted as an attribute.

Reject records where attrs_offset is outside bytes_in_use or where the
used area does not even contain the four-byte attribute type/AT_END
terminator at attrs_offset.

A small userspace model with attrs_offset=128 and bytes_in_use=64 shows
the current check accepts the record and the parser space calculation
underflows to 0xffffffc0.  With this change the same malformed record is
rejected before the attribute walker is entered.

Fixes: d3ad708fecaa ("ntfs: Initial commit")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/mft.c | 14 ++++++++++++--
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index c04462fe049e..70c1aa76181b 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -30,6 +30,8 @@ int ntfs_mft_record_check(const struct ntfs_volume *vol, struct mft_record *m,
 {
 	struct attr_record *a;
 	struct super_block *sb = vol->sb;
+	u16 attrs_offset;
+	u32 bytes_in_use;

 	if (!ntfs_is_file_record(m->magic)) {
 		ntfs_error(sb, "Record %llu has no FILE magic (0x%x)\n",
@@ -65,7 +67,17 @@ int ntfs_mft_record_check(const struct ntfs_volume *vol, struct mft_record *m,
 		goto err_out;
 	}

-	a = (struct attr_record *)((char *)m + le16_to_cpu(m->attrs_offset));
+	attrs_offset = le16_to_cpu(m->attrs_offset);
+	bytes_in_use = le32_to_cpu(m->bytes_in_use);
+
+	if (attrs_offset > bytes_in_use ||
+	    bytes_in_use - attrs_offset < sizeof_field(struct attr_record, type)) {
+		ntfs_error(sb, "Record %llu has corrupt attribute offset\n",
+				mft_no);
+		goto err_out;
+	}
+
+	a = (struct attr_record *)((char *)m + attrs_offset);
 	if ((char *)a < (char *)m || (char *)a > (char *)m + vol->mft_record_size) {
 		ntfs_error(sb, "Record %llu is corrupt\n", mft_no);
 		goto err_out;
--
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check
  2026-05-08 15:34 [PATCH 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
  2026-05-08 15:34 ` [PATCH 1/3] ntfs: validate MFT attrs_offset against bytes_in_use DaeMyung Kang
@ 2026-05-08 15:34 ` DaeMyung Kang
  2026-05-09  4:03   ` Namjae Jeon
  2026-05-08 15:34 ` [PATCH 3/3] ntfs: validate attribute name bounds before returning it DaeMyung Kang
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: DaeMyung Kang @ 2026-05-08 15:34 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

NTFS MFT record numbers are limited to the 32-bit range, and
ntfs_mft_record_layout() rejects mft_no >= 2^32.  The free-MFT-record
bitmap scan in ntfs_mft_bitmap_find_and_alloc_free_rec_nolock() also
guards against this overflow but uses a strict greater than comparison,
allowing record number 2^32 itself through this earlier check.

Every other 2^32 boundary check in fs/ntfs/mft.c uses '>=', so the
strict greater than here is both a real off-by-one and an internal
inconsistency.  A model with ll == 2^32 confirms the current check
accepts the value while the corrected check rejects it.

Use '>=' so the boundary matches the layout-time rejection and the
surrounding bitmap-scan checks.

Fixes: d3ad708fecaa ("ntfs: Initial commit")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/mft.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 70c1aa76181b..f8f2e481c5dc 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -1279,7 +1279,7 @@ static s64 ntfs_mft_bitmap_find_and_alloc_free_rec_nolock(struct ntfs_volume *vo
 				b = ffz((unsigned long)*byte);
 				if (b < 8 && b >= (bit & 7)) {
 					ll = data_pos + (bit & ~7ull) + b;
-					if (unlikely(ll > (1ll << 32))) {
+					if (unlikely(ll >= (1ll << 32))) {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
 						folio_unlock(folio);
 						kunmap_local(buf);
--
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] ntfs: validate attribute name bounds before returning it
  2026-05-08 15:34 [PATCH 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
  2026-05-08 15:34 ` [PATCH 1/3] ntfs: validate MFT attrs_offset against bytes_in_use DaeMyung Kang
  2026-05-08 15:34 ` [PATCH 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check DaeMyung Kang
@ 2026-05-08 15:34 ` DaeMyung Kang
  2026-05-09  6:12 ` [PATCH v2 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: DaeMyung Kang @ 2026-05-08 15:34 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

ntfs_attr_find() validates a named attribute before comparing it with the
requested name, but that check is currently after the AT_UNUSED handling.
When callers enumerate attributes with AT_UNUSED, ntfs_attr_find() can
return a malformed named attribute before checking whether name_offset
and name_length stay within the attribute record.

Some enumeration callers use the returned attribute name pointer
directly.  For example, one path passes (attr + name_offset, name_length)
to ntfs_attr_iget(), where the name can later be copied according to
name_length.  A malformed on-disk name_offset/name_length pair should not
be exposed to those callers.

Move the existing name bounds validation before returning attributes
during AT_UNUSED enumeration, and write it as an offset/remaining-size
check so the subtraction cannot underflow.  Extract the converted values
into local variables (name_offset, attr_len, name_size) to make the
intent explicit and avoid repeating the endian conversions inside the
bounds check.  This keeps matching attributes on the same checked path
while also covering attribute enumeration.

A small userspace ASAN model with attr length=32, name_offset=124 and
name_length=8 reproduces a heap-buffer-overflow read in the old
enumeration path.  With this change the same malformed attribute is
rejected before the name pointer is returned to the caller.

Fixes: d3ad708fecaa ("ntfs: Initial commit")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/attrib.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 330127975b26..f51917b4a494 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -675,6 +675,9 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 	__le16 *upcase = vol->upcase;
 	u32 upcase_len = vol->upcase_len;
 	unsigned int space;
+	u16 name_offset;
+	u32 attr_len;
+	u32 name_size;

 	/*
 	 * Iterate over attributes in mft record starting at @ctx->attr, or the
@@ -702,6 +705,20 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			return -ENOENT;
 		if (unlikely(!a->length))
 			break;
+		if (a->name_length) {
+			name_offset = le16_to_cpu(a->name_offset);
+			attr_len = le32_to_cpu(a->length);
+			name_size = a->name_length * sizeof(__le16);
+
+			if (name_offset > attr_len ||
+			    attr_len - name_offset < name_size) {
+				ntfs_error(vol->sb,
+					   "Corrupt attribute name in MFT record %llu\n",
+					   ctx->ntfs_ino->mft_no);
+				break;
+			}
+		}
+
 		if (type == AT_UNUSED)
 			return 0;
 		if (a->type != type)
@@ -715,14 +732,6 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			if (a->name_length)
 				return -ENOENT;
 		} else {
-			if (a->name_length && ((le16_to_cpu(a->name_offset) +
-					       a->name_length * sizeof(__le16)) >
-						le32_to_cpu(a->length))) {
-				ntfs_error(vol->sb, "Corrupt attribute name in MFT record %llu\n",
-					   ctx->ntfs_ino->mft_no);
-				break;
-			}
-
 			if (!ntfs_are_names_equal(name, name_len,
 					(__le16 *)((u8 *)a + le16_to_cpu(a->name_offset)),
 					a->name_length, ic, upcase, upcase_len)) {
--
2.34.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check
  2026-05-08 15:34 ` [PATCH 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check DaeMyung Kang
@ 2026-05-09  4:03   ` Namjae Jeon
  2026-05-09  6:14     ` CharSyam
  0 siblings, 1 reply; 11+ messages in thread
From: Namjae Jeon @ 2026-05-09  4:03 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

> Fixes: d3ad708fecaa ("ntfs: Initial commit")
The warning from checkpatch.pl indicates that the commit ID you
provided in the Fixes: tag does not exist...

WARNING: Unknown commit id 'd3ad708fecaa', maybe rebased or not pulled?
#67:
Fixes: d3ad708fecaa ("ntfs: Initial commit")

If the Fixes: tag refers to a commit from an oot version, please do
either remove the tag or replace it with the correct upstream commit
ID that introduced the bug. Other patches also have this tag, so
please update them all.

> @@ -1279,7 +1279,7 @@ static s64 ntfs_mft_bitmap_find_and_alloc_free_rec_nolock(struct ntfs_volume *vo
>                                 b = ffz((unsigned long)*byte);
>                                 if (b < 8 && b >= (bit & 7)) {
>                                         ll = data_pos + (bit & ~7ull) + b;
> -                                       if (unlikely(ll > (1ll << 32))) {
> +                                       if (unlikely(ll >= (1ll << 32))) {
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
The kernel version check code is not in the mainline, so it seems this
patch was created in the oot version... I cannot apply it unless you
remove this.
>                                                 folio_unlock(folio);
>                                                 kunmap_local(buf);
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 0/3] ntfs: harden MFT record and attribute parsing
  2026-05-08 15:34 [PATCH 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
                   ` (2 preceding siblings ...)
  2026-05-08 15:34 ` [PATCH 3/3] ntfs: validate attribute name bounds before returning it DaeMyung Kang
@ 2026-05-09  6:12 ` DaeMyung Kang
  2026-05-09 15:44   ` Namjae Jeon
  2026-05-09  6:12 ` [PATCH v2 1/3] ntfs: validate MFT attrs_offset against bytes_in_use DaeMyung Kang
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: DaeMyung Kang @ 2026-05-09  6:12 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: linux-fsdevel, linux-kernel, DaeMyung Kang

This series tightens fs/ntfs against malformed on-disk metadata and
fixes one off-by-one in the MFT bitmap scan.

Patches 1/3 and 3/3 are complementary: 1/3 rejects MFT records whose
attrs_offset points past bytes_in_use at record entry, and 3/3 moves
the per-attribute name bounds check earlier so it covers the AT_UNUSED
enumeration path that hands the name pointer back to callers.  Without
3/3, two enumeration paths can read past an attribute record: one in
fs/ntfs/attrib.c passes the returned name pointer to ntfs_attr_iget(),
and another in fs/ntfs/inode.c copies the name while building an
attribute list.

Patch 2/3 is independent: ntfs_mft_record_layout() rejects
mft_no >= 2^32, but the bitmap scan in
ntfs_mft_bitmap_find_and_alloc_free_rec_nolock() used '>'.  Bring it in
line with the other 2^32 boundary checks in fs/ntfs/mft.c.

All three carry the same upstream Fixes tag (1da177e4c3f4) since the
issues date from the initial upstream import of fs/ntfs.

Changes since v1:
  - All patches: replaced the OOT Fixes tag d3ad708fecaa ("ntfs:
    Initial commit") with the upstream commit 1da177e4c3f4
    ("Linux-2.6.12-rc2"), since checkpatch flagged the v1 commit id
    as unknown to mainline and all three issues date from the initial
    fs/ntfs import.
  - 2/3: removed the OOT-only "#if LINUX_VERSION_CODE >= KERNEL_VERSION(
    6, 6, 0)" guard that surrounded the folio_unlock()/kunmap_local()/
    folio_put() block; mainline does not need it and it prevented the
    patch from applying.
  - 1/3, 3/3: no functional change.

v1: https://lore.kernel.org/linux-ntfs/20260508153410.2624801-1-charsyam@gmail.com/

DaeMyung Kang (3):
  ntfs: validate MFT attrs_offset against bytes_in_use
  ntfs: fix MFT bitmap scan 2^32 boundary check
  ntfs: validate attribute name bounds before returning it

 fs/ntfs/attrib.c | 25 +++++++++++++++++--------
 fs/ntfs/mft.c    | 15 +++++++++++++--
 2 files changed, 30 insertions(+), 10 deletions(-)


base-commit: 70390501d1944d4e5b8f7352be180fceb3a44132
-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] ntfs: validate MFT attrs_offset against bytes_in_use
  2026-05-08 15:34 [PATCH 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
                   ` (3 preceding siblings ...)
  2026-05-09  6:12 ` [PATCH v2 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
@ 2026-05-09  6:12 ` DaeMyung Kang
  2026-05-09  6:12 ` [PATCH v2 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check DaeMyung Kang
  2026-05-09  6:12 ` [PATCH v2 3/3] ntfs: validate attribute name bounds before returning it DaeMyung Kang
  6 siblings, 0 replies; 11+ messages in thread
From: DaeMyung Kang @ 2026-05-09  6:12 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: linux-fsdevel, linux-kernel, DaeMyung Kang

ntfs_mft_record_check() verifies that attrs_offset is aligned and that
the resulting pointer stays within the allocated MFT record buffer, but
it does not check that the first attribute header starts within the
bytes_in_use area.

A malformed record with attrs_offset greater than bytes_in_use can pass
this check as long as attrs_offset is still within bytes_allocated.  The
attribute parser then computes the remaining record space by subtracting
the attribute pointer from bytes_in_use.  Because that value is unsigned,
the subtraction can underflow and allow bytes after bytes_in_use to be
interpreted as an attribute.

Reject records where attrs_offset is outside bytes_in_use or where the
used area does not even contain the four-byte attribute type/AT_END
terminator at attrs_offset.

A small userspace model with attrs_offset=128 and bytes_in_use=64 shows
the current check accepts the record and the parser space calculation
underflows to 0xffffffc0.  With this change the same malformed record is
rejected before the attribute walker is entered.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/mft.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 7d989267a82b..827b99f4597a 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -30,6 +30,8 @@ int ntfs_mft_record_check(const struct ntfs_volume *vol, struct mft_record *m,
 {
 	struct attr_record *a;
 	struct super_block *sb = vol->sb;
+	u16 attrs_offset;
+	u32 bytes_in_use;
 
 	if (!ntfs_is_file_record(m->magic)) {
 		ntfs_error(sb, "Record %llu has no FILE magic (0x%x)\n",
@@ -65,7 +67,16 @@ int ntfs_mft_record_check(const struct ntfs_volume *vol, struct mft_record *m,
 		goto err_out;
 	}
 
-	a = (struct attr_record *)((char *)m + le16_to_cpu(m->attrs_offset));
+	attrs_offset = le16_to_cpu(m->attrs_offset);
+	bytes_in_use = le32_to_cpu(m->bytes_in_use);
+
+	if (attrs_offset > bytes_in_use ||
+	    bytes_in_use - attrs_offset < sizeof_field(struct attr_record, type)) {
+		ntfs_error(sb, "Record %llu has corrupt attribute offset\n", mft_no);
+		goto err_out;
+	}
+
+	a = (struct attr_record *)((char *)m + attrs_offset);
 	if ((char *)a < (char *)m || (char *)a > (char *)m + vol->mft_record_size) {
 		ntfs_error(sb, "Record %llu is corrupt\n", mft_no);
 		goto err_out;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check
  2026-05-08 15:34 [PATCH 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
                   ` (4 preceding siblings ...)
  2026-05-09  6:12 ` [PATCH v2 1/3] ntfs: validate MFT attrs_offset against bytes_in_use DaeMyung Kang
@ 2026-05-09  6:12 ` DaeMyung Kang
  2026-05-09  6:12 ` [PATCH v2 3/3] ntfs: validate attribute name bounds before returning it DaeMyung Kang
  6 siblings, 0 replies; 11+ messages in thread
From: DaeMyung Kang @ 2026-05-09  6:12 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: linux-fsdevel, linux-kernel, DaeMyung Kang

NTFS MFT record numbers are limited to the 32-bit range, and
ntfs_mft_record_layout() rejects mft_no >= 2^32.  The free-MFT-record
bitmap scan in ntfs_mft_bitmap_find_and_alloc_free_rec_nolock() also
guards against this overflow but uses a strict greater than comparison,
allowing record number 2^32 itself through this earlier check.

Every other 2^32 boundary check in fs/ntfs/mft.c uses '>=', so the
strict greater than here is both a real off-by-one and an internal
inconsistency.  A model with ll == 2^32 confirms the current check
accepts the value while the corrected check rejects it.

Use '>=' so the boundary matches the layout-time rejection and the
surrounding bitmap-scan checks.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/mft.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 827b99f4597a..b056c9bbdf5f 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -1045,7 +1045,7 @@ static s64 ntfs_mft_bitmap_find_and_alloc_free_rec_nolock(struct ntfs_volume *vo
 				b = ffz((unsigned long)*byte);
 				if (b < 8 && b >= (bit & 7)) {
 					ll = data_pos + (bit & ~7ull) + b;
-					if (unlikely(ll > (1ll << 32))) {
+					if (unlikely(ll >= (1ll << 32))) {
 						folio_unlock(folio);
 						kunmap_local(buf);
 						folio_put(folio);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] ntfs: validate attribute name bounds before returning it
  2026-05-08 15:34 [PATCH 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
                   ` (5 preceding siblings ...)
  2026-05-09  6:12 ` [PATCH v2 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check DaeMyung Kang
@ 2026-05-09  6:12 ` DaeMyung Kang
  6 siblings, 0 replies; 11+ messages in thread
From: DaeMyung Kang @ 2026-05-09  6:12 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: linux-fsdevel, linux-kernel, DaeMyung Kang

ntfs_attr_find() validates a named attribute before comparing it with the
requested name, but that check is currently after the AT_UNUSED handling.
When callers enumerate attributes with AT_UNUSED, ntfs_attr_find() can
return a malformed named attribute before checking whether name_offset
and name_length stay within the attribute record.

Some enumeration callers use the returned attribute name pointer
directly.  For example, one path passes (attr + name_offset, name_length)
to ntfs_attr_iget(), where the name can later be copied according to
name_length.  A malformed on-disk name_offset/name_length pair should not
be exposed to those callers.

Move the existing name bounds validation before returning attributes
during AT_UNUSED enumeration, and write it as an offset/remaining-size
check so the subtraction cannot underflow.  Extract the converted values
into local variables (name_offset, attr_len, name_size) to make the
intent explicit and avoid repeating the endian conversions inside the
bounds check.  This keeps matching attributes on the same checked path
while also covering attribute enumeration.

A small userspace ASAN model with attr length=32, name_offset=124 and
name_length=8 reproduces a heap-buffer-overflow read in the old
enumeration path.  With this change the same malformed attribute is
rejected before the name pointer is returned to the caller.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/attrib.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 97b660eaa00c..1a0e1f9e0853 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -672,6 +672,9 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 	__le16 *upcase = vol->upcase;
 	u32 upcase_len = vol->upcase_len;
 	unsigned int space;
+	u16 name_offset;
+	u32 attr_len;
+	u32 name_size;
 
 	/*
 	 * Iterate over attributes in mft record starting at @ctx->attr, or the
@@ -699,6 +702,20 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			return -ENOENT;
 		if (unlikely(!a->length))
 			break;
+		if (a->name_length) {
+			name_offset = le16_to_cpu(a->name_offset);
+			attr_len = le32_to_cpu(a->length);
+			name_size = a->name_length * sizeof(__le16);
+
+			if (name_offset > attr_len ||
+			    attr_len - name_offset < name_size) {
+				ntfs_error(vol->sb,
+					   "Corrupt attribute name in MFT record %llu\n",
+					   ctx->ntfs_ino->mft_no);
+				break;
+			}
+		}
+
 		if (type == AT_UNUSED)
 			return 0;
 		if (a->type != type)
@@ -712,14 +729,6 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			if (a->name_length)
 				return -ENOENT;
 		} else {
-			if (a->name_length && ((le16_to_cpu(a->name_offset) +
-					       a->name_length * sizeof(__le16)) >
-						le32_to_cpu(a->length))) {
-				ntfs_error(vol->sb, "Corrupt attribute name in MFT record %llu\n",
-					   ctx->ntfs_ino->mft_no);
-				break;
-			}
-
 			if (!ntfs_are_names_equal(name, name_len,
 					(__le16 *)((u8 *)a + le16_to_cpu(a->name_offset)),
 					a->name_length, ic, upcase, upcase_len)) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check
  2026-05-09  4:03   ` Namjae Jeon
@ 2026-05-09  6:14     ` CharSyam
  0 siblings, 0 replies; 11+ messages in thread
From: CharSyam @ 2026-05-09  6:14 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

I will send v2 patch again.

Thanks for pointing it out.

2026년 5월 9일 (토) 오후 1:03, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> > Fixes: d3ad708fecaa ("ntfs: Initial commit")
> The warning from checkpatch.pl indicates that the commit ID you
> provided in the Fixes: tag does not exist...
>
> WARNING: Unknown commit id 'd3ad708fecaa', maybe rebased or not pulled?
> #67:
> Fixes: d3ad708fecaa ("ntfs: Initial commit")
>
> If the Fixes: tag refers to a commit from an oot version, please do
> either remove the tag or replace it with the correct upstream commit
> ID that introduced the bug. Other patches also have this tag, so
> please update them all.
>
> > @@ -1279,7 +1279,7 @@ static s64 ntfs_mft_bitmap_find_and_alloc_free_rec_nolock(struct ntfs_volume *vo
> >                                 b = ffz((unsigned long)*byte);
> >                                 if (b < 8 && b >= (bit & 7)) {
> >                                         ll = data_pos + (bit & ~7ull) + b;
> > -                                       if (unlikely(ll > (1ll << 32))) {
> > +                                       if (unlikely(ll >= (1ll << 32))) {
> >  #if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
> The kernel version check code is not in the mainline, so it seems this
> patch was created in the oot version... I cannot apply it unless you
> remove this.
> >                                                 folio_unlock(folio);
> >                                                 kunmap_local(buf);
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/3] ntfs: harden MFT record and attribute parsing
  2026-05-09  6:12 ` [PATCH v2 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
@ 2026-05-09 15:44   ` Namjae Jeon
  0 siblings, 0 replies; 11+ messages in thread
From: Namjae Jeon @ 2026-05-09 15:44 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

On Sat, May 9, 2026 at 3:12 PM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> This series tightens fs/ntfs against malformed on-disk metadata and
> fixes one off-by-one in the MFT bitmap scan.
>
> Patches 1/3 and 3/3 are complementary: 1/3 rejects MFT records whose
> attrs_offset points past bytes_in_use at record entry, and 3/3 moves
> the per-attribute name bounds check earlier so it covers the AT_UNUSED
> enumeration path that hands the name pointer back to callers.  Without
> 3/3, two enumeration paths can read past an attribute record: one in
> fs/ntfs/attrib.c passes the returned name pointer to ntfs_attr_iget(),
> and another in fs/ntfs/inode.c copies the name while building an
> attribute list.
>
> Patch 2/3 is independent: ntfs_mft_record_layout() rejects
> mft_no >= 2^32, but the bitmap scan in
> ntfs_mft_bitmap_find_and_alloc_free_rec_nolock() used '>'.  Bring it in
> line with the other 2^32 boundary checks in fs/ntfs/mft.c.
>
> All three carry the same upstream Fixes tag (1da177e4c3f4) since the
> issues date from the initial upstream import of fs/ntfs.
>
> Changes since v1:
>   - All patches: replaced the OOT Fixes tag d3ad708fecaa ("ntfs:
>     Initial commit") with the upstream commit 1da177e4c3f4
>     ("Linux-2.6.12-rc2"), since checkpatch flagged the v1 commit id
>     as unknown to mainline and all three issues date from the initial
>     fs/ntfs import.
>   - 2/3: removed the OOT-only "#if LINUX_VERSION_CODE >= KERNEL_VERSION(
>     6, 6, 0)" guard that surrounded the folio_unlock()/kunmap_local()/
>     folio_put() block; mainline does not need it and it prevented the
>     patch from applying.
>   - 1/3, 3/3: no functional change.
>
> v1: https://lore.kernel.org/linux-ntfs/20260508153410.2624801-1-charsyam@gmail.com/
>
> DaeMyung Kang (3):
>   ntfs: validate MFT attrs_offset against bytes_in_use
>   ntfs: fix MFT bitmap scan 2^32 boundary check
>   ntfs: validate attribute name bounds before returning it
Applied them to #ntfs-next.
Thanks!

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-05-09 15:44 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 15:34 [PATCH 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
2026-05-08 15:34 ` [PATCH 1/3] ntfs: validate MFT attrs_offset against bytes_in_use DaeMyung Kang
2026-05-08 15:34 ` [PATCH 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check DaeMyung Kang
2026-05-09  4:03   ` Namjae Jeon
2026-05-09  6:14     ` CharSyam
2026-05-08 15:34 ` [PATCH 3/3] ntfs: validate attribute name bounds before returning it DaeMyung Kang
2026-05-09  6:12 ` [PATCH v2 0/3] ntfs: harden MFT record and attribute parsing DaeMyung Kang
2026-05-09 15:44   ` Namjae Jeon
2026-05-09  6:12 ` [PATCH v2 1/3] ntfs: validate MFT attrs_offset against bytes_in_use DaeMyung Kang
2026-05-09  6:12 ` [PATCH v2 2/3] ntfs: fix MFT bitmap scan 2^32 boundary check DaeMyung Kang
2026-05-09  6:12 ` [PATCH v2 3/3] ntfs: validate attribute name bounds before returning it DaeMyung Kang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox