mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] exfat: bound uniname advance in exfat_find_dir_entry()
@ 2026-06-12  3:45 Bryam Vargas via B4 Relay
  2026-06-12 14:04 ` Namjae Jeon
  0 siblings, 1 reply; 3+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-12  3:45 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Yuezhang Mo, linux-fsdevel, linux-kernel, Sungjong Seo

From: Bryam Vargas <hexlabsecurity@proton.me>

In exfat_find_dir_entry(), each TYPE_EXTEND (file name) entry advances the
output pointer by a fixed amount while the loop guard only tracks the
accumulated name length:

	if (++order == 2)
		uniname = p_uniname->name;
	else
		uniname += EXFAT_FILE_NAME_LEN;
	len = exfat_extract_uni_name(ep, entry_uniname);
	name_len += len;
	unichar = *(uniname+len);
	*(uniname+len) = 0x0;

uniname grows by EXFAT_FILE_NAME_LEN (15) per name entry, but name_len
grows only by the actual extracted length, which is shorter when a name
fragment contains an early NUL.  The only guard is
`name_len >= MAX_NAME_LENGTH`, so a crafted directory with many short
name fragments lets uniname run far past the
p_uniname->name[MAX_NAME_LENGTH + 3] buffer while name_len stays small,
causing an out-of-bounds read and write at *(uniname+len).

The sibling extractor exfat_get_uniname_from_ext_entry() already stops
on a short fragment (the lockstep `len != EXFAT_FILE_NAME_LEN` guard
added in commit d42334578eba ("exfat: check if filename entries exceeds
max filename length")); exfat_find_dir_entry() never got the
equivalent.  Bound the access to the p_uniname->name buffer.

Fixes: ca06197382bd ("exfat: add directory operations")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 fs/exfat/dir.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index ac008ccaa97d..d4769a0f9978 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -1043,6 +1043,11 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
 					uniname += EXFAT_FILE_NAME_LEN;
 
 				len = exfat_extract_uni_name(ep, entry_uniname);
+				if (uniname + len >
+				    p_uniname->name + MAX_NAME_LENGTH) {
+					step = DIRENT_STEP_FILE;
+					continue;
+				}
 				name_len += len;
 
 				unichar = *(uniname+len);

---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260611-b4-disp-c0e7d249-c1db1bca7353

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>



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

* Re: [PATCH] exfat: bound uniname advance in exfat_find_dir_entry()
  2026-06-12  3:45 [PATCH] exfat: bound uniname advance in exfat_find_dir_entry() Bryam Vargas via B4 Relay
@ 2026-06-12 14:04 ` Namjae Jeon
  2026-06-12 19:30   ` Bryam Vargas
  0 siblings, 1 reply; 3+ messages in thread
From: Namjae Jeon @ 2026-06-12 14:04 UTC (permalink / raw)
  To: hexlabsecurity; +Cc: Yuezhang Mo, linux-fsdevel, linux-kernel, Sungjong Seo

> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
> index ac008ccaa97d..d4769a0f9978 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -1043,6 +1043,11 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
>                                         uniname += EXFAT_FILE_NAME_LEN;
>
>                                 len = exfat_extract_uni_name(ep, entry_uniname);
> +                               if (uniname + len >
> +                                   p_uniname->name + MAX_NAME_LENGTH) {
> +                                       step = DIRENT_STEP_FILE;
> +                                       continue;
> +                               }
uniname may already be outside the array before this check, so
pointer-base bounds check is not safe.
How about changing like this ?

+ unsigned int offset;

+ offset = (++order - 2) * EXFAT_FILE_NAME_LEN;
len = exfat_extract_uni_name(ep, entry_uniname);
brelse(bh);

+ if (offset > MAX_NAME_LENGTH ||
+     len > MAX_NAME_LENGTH - offset) {
+        step = DIRENT_STEP_FILE;
+        continue;
+ }

+ uniname = p_uniname->name + offset;
name_len += len;

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

* Re: [PATCH] exfat: bound uniname advance in exfat_find_dir_entry()
  2026-06-12 14:04 ` Namjae Jeon
@ 2026-06-12 19:30   ` Bryam Vargas
  0 siblings, 0 replies; 3+ messages in thread
From: Bryam Vargas @ 2026-06-12 19:30 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Sungjong Seo, Yuezhang Mo, linux-fsdevel, linux-kernel

You're right -- by the time the check runs, uniname has already been advanced
by the previous iterations, so `uniname + len` forms a pointer past the end of
p_uniname->name[] (undefined behavior; the compiler is free to drop the check).
Bounding the integer offset before the pointer is formed is the correct fix.

v2 uses your form -- compute offset = (++order - 2) * EXFAT_FILE_NAME_LEN, reject
when offset > MAX_NAME_LENGTH || len > MAX_NAME_LENGTH - offset, and only then set
uniname = p_uniname->name + offset. Carried as Suggested-by: you.

I re-ran the KASAN repro with your version on the same crafted image (high
SecondaryCount of all-NUL name entries): the bound now rejects the walk at
order 20 (offset 270 > 255) before any out-of-bounds pointer is formed, the
lookup returns -ENOENT, and there is no KASAN report. A legitimate 255-character
name (17 * 15) still resolves -- offset reaches 240 on the last entry, len 15,
240 + 15 = 255, within bounds.

Posted as v2: https://lore.kernel.org/all/20260612-b4-disp-d3abc2b6-v2-1-93ec15095270@proton.me/


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

end of thread, other threads:[~2026-06-12 19:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-12  3:45 [PATCH] exfat: bound uniname advance in exfat_find_dir_entry() Bryam Vargas via B4 Relay
2026-06-12 14:04 ` Namjae Jeon
2026-06-12 19:30   ` Bryam Vargas

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®