* [PATCH] fs/ntfs3: validate the EA value length in ntfs_read_ea()
@ 2026-08-03 4:34 Yuejie Shi
2026-10-28 7:40 ` Konstantin Komarov
0 siblings, 1 reply; 2+ messages in thread
From: Yuejie Shi @ 2026-08-03 4:34 UTC (permalink / raw)
To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, stable
ntfs_read_ea() checks the $EA list for consistency, but it only
recomputes the packed size of an entry when the entry's own size field is
zero:
for (off = 0; off < size; off += ea_size) {
const struct EA_FULL *ef = Add2Ptr(ea_p, off);
u32 bytes = size - off;
...
if (ef->size) {
ea_size = le32_to_cpu(ef->size);
if (ea_size > bytes)
goto out1;
continue; /* <- elength never checked */
}
...
ea_size = ALIGN(struct_size(ef, name,
1 + ef->name_len +
le16_to_cpu(ef->elength)),
4);
if (ea_size > bytes)
goto out1;
}
ef->size is non-zero for every entry except the last one, and it is what
ntfs3 itself writes (ntfs_set_ea() stores exactly that aligned packed
size), so in practice elength is not validated at all. find_ea() then
uses unpacked_ea_size(), which also prefers ea->size when it is set, so
it agrees and hands the entry back.
ntfs_get_ea() copies elength bytes with no further bound of its own:
len = le16_to_cpu(ea->elength);
...
if (len > size) { /* size is the *caller's* buffer */
err = -ERANGE;
...
}
memcpy(buffer, ea->name + ea->name_len + 1, len);
so a caller that supplies a large enough buffer receives up to 64 KiB of
whatever follows the small $EA allocation -- straight to userspace via
getxattr(2).
Compute the packed size unconditionally and require it to fit in the
entry. Hoist the "bytes < offsetof(struct EA_FULL, name)" test above the
ef->size branch as well: name_len and elength were being read there
without it, which is itself a small over-read of the kmalloc'd buffer
when fewer than 8 bytes are left. The old "bytes < sizeof(ef->size)"
test is subsumed by it.
# mkntfs image; create /mnt/ntfs/poc with user.ntfs6 set, then rewrite
# that EA_FULL's elength from 0xf to 0xff00 on the raw device
mount -t ntfs3 -o ro /dev/vda /mnt/ntfs
getxattr("/mnt/ntfs/poc", "user.ntfs6", buf, 128 * 1024);
BUG: KASAN: slab-out-of-bounds in ntfs_get_ea+0x294/0x3e0
Read of size 65280 at addr ffff0000cca53acf by task xattr_probe/142
__asan_memcpy+0x3c/0xa0
ntfs_get_ea+0x294/0x3e0
ntfs_getxattr+0x284/0x318
__vfs_getxattr+0x104/0x160
vfs_getxattr+0x1b4/0x1e0
do_getxattr+0xcc/0x230
path_getxattrat+0x184/0x270
__arm64_sys_getxattr+0x64/0x80
Allocated by task 142:
__kmalloc_noprof+0x294/0x668
ntfs_read_ea+0x1e4/0x420
ntfs_get_ea+0x2c8/0x3e0
The buggy address belongs to the object at ffff0000cca53a80
which belongs to the cache kmalloc-96 of size 96
The object being over-read is the one ntfs_read_ea() just allocated, and
it is 96 bytes.
Entries written by ntfs3 and by Windows set size to the aligned packed
size, so the new test does not reject anything a working filesystem
produces.
Fixes: be71b5cba2e6 ("fs/ntfs3: Add attrib operations")
Cc: stable@vger.kernel.org
Signed-off-by: Yuejie Shi <syjcnss@gmail.com>
---
fs/ntfs3/xattr.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 04814dd29375..e67a1100d0cd 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -146,26 +146,31 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct
for (off = 0; off < size; off += ea_size) {
const struct EA_FULL *ef = Add2Ptr(ea_p, off);
u32 bytes = size - off;
+ u32 packed_size;
- /* Check if we can use field ea->size. */
- if (bytes < sizeof(ef->size))
+ /* Check if we can use fields ef->name_len and ef->elength. */
+ if (bytes < offsetof(struct EA_FULL, name))
goto out1;
+ /*
+ * The name and the value have to fit in the entry whether or
+ * not ef->size is set, because ntfs_get_ea() copies elength
+ * bytes out of this buffer and checks them only against the
+ * caller supplied buffer size.
+ */
+ packed_size = ALIGN(struct_size(ef, name,
+ 1 + ef->name_len +
+ le16_to_cpu(ef->elength)),
+ 4);
+
if (ef->size) {
ea_size = le32_to_cpu(ef->size);
- if (ea_size > bytes)
+ if (ea_size > bytes || packed_size > ea_size)
goto out1;
continue;
}
- /* Check if we can use fields ef->name_len and ef->elength. */
- if (bytes < offsetof(struct EA_FULL, name))
- goto out1;
-
- ea_size = ALIGN(struct_size(ef, name,
- 1 + ef->name_len +
- le16_to_cpu(ef->elength)),
- 4);
+ ea_size = packed_size;
if (ea_size > bytes)
goto out1;
}
--
2.51.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] fs/ntfs3: validate the EA value length in ntfs_read_ea()
2026-08-03 4:34 [PATCH] fs/ntfs3: validate the EA value length in ntfs_read_ea() Yuejie Shi
@ 2026-10-28 7:40 ` Konstantin Komarov
0 siblings, 0 replies; 2+ messages in thread
From: Konstantin Komarov @ 2026-10-28 7:40 UTC (permalink / raw)
To: Yuejie Shi; +Cc: ntfs3, linux-kernel, stable
On 8/3/26 06:34, Yuejie Shi wrote:
> ntfs_read_ea() checks the $EA list for consistency, but it only
> recomputes the packed size of an entry when the entry's own size field is
> zero:
>
> for (off = 0; off < size; off += ea_size) {
> const struct EA_FULL *ef = Add2Ptr(ea_p, off);
> u32 bytes = size - off;
> ...
> if (ef->size) {
> ea_size = le32_to_cpu(ef->size);
> if (ea_size > bytes)
> goto out1;
> continue; /* <- elength never checked */
> }
> ...
> ea_size = ALIGN(struct_size(ef, name,
> 1 + ef->name_len +
> le16_to_cpu(ef->elength)),
> 4);
> if (ea_size > bytes)
> goto out1;
> }
>
> ef->size is non-zero for every entry except the last one, and it is what
> ntfs3 itself writes (ntfs_set_ea() stores exactly that aligned packed
> size), so in practice elength is not validated at all. find_ea() then
> uses unpacked_ea_size(), which also prefers ea->size when it is set, so
> it agrees and hands the entry back.
>
> ntfs_get_ea() copies elength bytes with no further bound of its own:
>
> len = le16_to_cpu(ea->elength);
> ...
> if (len > size) { /* size is the *caller's* buffer */
> err = -ERANGE;
> ...
> }
> memcpy(buffer, ea->name + ea->name_len + 1, len);
>
> so a caller that supplies a large enough buffer receives up to 64 KiB of
> whatever follows the small $EA allocation -- straight to userspace via
> getxattr(2).
>
> Compute the packed size unconditionally and require it to fit in the
> entry. Hoist the "bytes < offsetof(struct EA_FULL, name)" test above the
> ef->size branch as well: name_len and elength were being read there
> without it, which is itself a small over-read of the kmalloc'd buffer
> when fewer than 8 bytes are left. The old "bytes < sizeof(ef->size)"
> test is subsumed by it.
>
> # mkntfs image; create /mnt/ntfs/poc with user.ntfs6 set, then rewrite
> # that EA_FULL's elength from 0xf to 0xff00 on the raw device
> mount -t ntfs3 -o ro /dev/vda /mnt/ntfs
> getxattr("/mnt/ntfs/poc", "user.ntfs6", buf, 128 * 1024);
>
> BUG: KASAN: slab-out-of-bounds in ntfs_get_ea+0x294/0x3e0
> Read of size 65280 at addr ffff0000cca53acf by task xattr_probe/142
> __asan_memcpy+0x3c/0xa0
> ntfs_get_ea+0x294/0x3e0
> ntfs_getxattr+0x284/0x318
> __vfs_getxattr+0x104/0x160
> vfs_getxattr+0x1b4/0x1e0
> do_getxattr+0xcc/0x230
> path_getxattrat+0x184/0x270
> __arm64_sys_getxattr+0x64/0x80
> Allocated by task 142:
> __kmalloc_noprof+0x294/0x668
> ntfs_read_ea+0x1e4/0x420
> ntfs_get_ea+0x2c8/0x3e0
> The buggy address belongs to the object at ffff0000cca53a80
> which belongs to the cache kmalloc-96 of size 96
>
> The object being over-read is the one ntfs_read_ea() just allocated, and
> it is 96 bytes.
>
> Entries written by ntfs3 and by Windows set size to the aligned packed
> size, so the new test does not reject anything a working filesystem
> produces.
>
> Fixes: be71b5cba2e6 ("fs/ntfs3: Add attrib operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yuejie Shi <syjcnss@gmail.com>
> ---
> fs/ntfs3/xattr.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index 04814dd29375..e67a1100d0cd 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -146,26 +146,31 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct
> for (off = 0; off < size; off += ea_size) {
> const struct EA_FULL *ef = Add2Ptr(ea_p, off);
> u32 bytes = size - off;
> + u32 packed_size;
>
> - /* Check if we can use field ea->size. */
> - if (bytes < sizeof(ef->size))
> + /* Check if we can use fields ef->name_len and ef->elength. */
> + if (bytes < offsetof(struct EA_FULL, name))
> goto out1;
>
> + /*
> + * The name and the value have to fit in the entry whether or
> + * not ef->size is set, because ntfs_get_ea() copies elength
> + * bytes out of this buffer and checks them only against the
> + * caller supplied buffer size.
> + */
> + packed_size = ALIGN(struct_size(ef, name,
> + 1 + ef->name_len +
> + le16_to_cpu(ef->elength)),
> + 4);
> +
> if (ef->size) {
> ea_size = le32_to_cpu(ef->size);
> - if (ea_size > bytes)
> + if (ea_size > bytes || packed_size > ea_size)
> goto out1;
> continue;
> }
>
> - /* Check if we can use fields ef->name_len and ef->elength. */
> - if (bytes < offsetof(struct EA_FULL, name))
> - goto out1;
> -
> - ea_size = ALIGN(struct_size(ef, name,
> - 1 + ef->name_len +
> - le16_to_cpu(ef->elength)),
> - 4);
> + ea_size = packed_size;
> if (ea_size > bytes)
> goto out1;
> }
> --
> 2.51.0
Hello,
The patch was applied, thank you.
Regards,
Konstantin
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-28 7:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-03 4:34 [PATCH] fs/ntfs3: validate the EA value length in ntfs_read_ea() Yuejie Shi
2026-10-28 7:40 ` Konstantin Komarov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®