[PATCH] ntfs: bound $AttrDef table walk to the loaded table size

Dennis Tighe <[email protected]>
Newsgroups org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
ntfs_attr_find_in_attrdef() walks the in-memory $AttrDef table, but the
loop condition bounds only the start of each entry, not the whole entry:

	for (ad = vol->attrdef; (u8 *)ad - (u8 *)vol->attrdef <
			vol->attrdef_size && ad->type; ++ad)

struct attr_def is 160 bytes; the guard reads ad->type at offset 128 and
the loop body reads further fields. vol->attrdef is kvzalloc(i_size),
where i_size is the on-disk $AttrDef data size, checked in
load_and_init_attrdef() only as 0 < i_size <= 0x7fffffff. A volume whose
$AttrDef data size is smaller than one entry (e.g. 120 bytes) makes the
read of ad->type run past the allocation. Creating a file reaches this
through ntfs_attr_size_bounds_check() and reads out of bounds:

  BUG: KASAN: slab-out-of-bounds in ntfs_attr_find_in_attrdef+0x66/0xa0
  Read of size 4 at addr ffff888005833280 by task init/1
   ntfs_attr_find_in_attrdef
   ntfs_attr_size_bounds_check
   ntfs_attr_can_be_non_resident
   ntfs_attr_add

Require the whole entry to lie within attrdef_size in the loop guard, and
reject at mount a $AttrDef too small to hold one attr_def entry.

Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dennis Tighe <[email protected]>
---
Two parts in this change.
1/ The loop guard in ntfs_attr_find_in_attrdef() to fix the OOB access.
2/ A mount-time check in load_and_init_attrdef() that rejects a $AttrDef
that can't hold an entry. This is a clearly corrupt table and is refused up
front.

A reproducer is available on request.

 fs/ntfs/attrib.c | 2 +-
 fs/ntfs/super.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 990c3937a..3f68a8ad1 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -1737,7 +1737,7 @@ static struct attr_def *ntfs_attr_find_in_attrdef(const struct ntfs_volume *vol,
 	struct attr_def *ad;
 
 	WARN_ON(!type);
-	for (ad = vol->attrdef; (u8 *)ad - (u8 *)vol->attrdef <
+	for (ad = vol->attrdef; (u8 *)(ad + 1) - (u8 *)vol->attrdef <=
 			vol->attrdef_size && ad->type; ++ad) {
 		/* We have not found it yet, carry on searching. */
 		if (likely(le32_to_cpu(ad->type) < le32_to_cpu(type)))
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index cd8fa2c13..2fd7db672 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -1241,9 +1241,9 @@ static bool load_and_init_attrdef(struct ntfs_volume *vol)
 		goto failed;
 	}
 	NInoSetSparseDisabled(NTFS_I(ino));
-	/* The size of FILE_AttrDef must be above 0 and fit inside 31 bits. */
+	/* FILE_AttrDef must hold at least one entry and fit inside 31 bits. */
 	i_size = i_size_read(ino);
-	if (i_size <= 0 || i_size > 0x7fffffff)
+	if (i_size < (s64)sizeof(struct attr_def) || i_size > 0x7fffffff)
 		goto iput_failed;
 	vol->attrdef = kvzalloc(i_size, GFP_NOFS);
 	if (!vol->attrdef)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.