* [PATCH v4] exfat: fix out-of-bounds in exfat_nls_to_ucs2()
@ 2025-10-15 7:34 Jeongjun Park
2025-10-15 8:56 ` Namjae Jeon
0 siblings, 1 reply; 2+ messages in thread
From: Jeongjun Park @ 2025-10-15 7:34 UTC (permalink / raw)
To: Namjae Jeon, Sungjong Seo
Cc: Yuezhang Mo, pali, Ethan Ferguson, linux-fsdevel, linux-kernel,
syzbot+98cc76a76de46b3714d4, Jeongjun Park
Since the len argument value passed to exfat_ioctl_set_volume_label()
from exfat_nls_to_utf16() is passed 1 too large, an out-of-bounds read
occurs when dereferencing p_cstring in exfat_nls_to_ucs2() later.
And because of the NLS_NAME_OVERLEN macro, another error occurs when
creating a file with a period at the end using utf8 and other iocharsets.
So to avoid this, you should remove the code that uses NLS_NAME_OVERLEN
macro and make the len argument value be the length of the label string,
but with a maximum length of FSLABEL_MAX - 1.
Reported-by: syzbot+98cc76a76de46b3714d4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=98cc76a76de46b3714d4
Fixes: d01579d590f7 ("exfat: Add support for FS_IOC_{GET,SET}FSLABEL")
Suggested-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
fs/exfat/exfat_fs.h | 1 -
fs/exfat/file.c | 7 ++++---
fs/exfat/namei.c | 2 +-
fs/exfat/nls.c | 3 ---
4 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 329697c89d09..38210fb6901c 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -29,7 +29,6 @@ enum exfat_error_mode {
enum {
NLS_NAME_NO_LOSSY = 0, /* no lossy */
NLS_NAME_LOSSY = 1 << 0, /* just detected incorrect filename(s) */
- NLS_NAME_OVERLEN = 1 << 1, /* the length is over than its limit */
};
#define EXFAT_HASH_BITS 8
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index f246cf439588..adc37b4d7fc2 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -509,8 +509,8 @@ static int exfat_ioctl_get_volume_label(struct super_block *sb, unsigned long ar
static int exfat_ioctl_set_volume_label(struct super_block *sb,
unsigned long arg)
{
- int ret = 0, lossy;
- char label[FSLABEL_MAX];
+ int ret = 0, lossy, label_len;
+ char label[FSLABEL_MAX] = {0};
struct exfat_uni_name uniname;
if (!capable(CAP_SYS_ADMIN))
@@ -520,8 +520,9 @@ static int exfat_ioctl_set_volume_label(struct super_block *sb,
return -EFAULT;
memset(&uniname, 0, sizeof(uniname));
+ label_len = strnlen(label, FSLABEL_MAX - 1);
if (label[0]) {
- ret = exfat_nls_to_utf16(sb, label, FSLABEL_MAX,
+ ret = exfat_nls_to_utf16(sb, label, label_len,
&uniname, &lossy);
if (ret < 0)
return ret;
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 7eb9c67fd35f..a6426a52fd29 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -442,7 +442,7 @@ static int __exfat_resolve_path(struct inode *inode, const unsigned char *path,
return namelen; /* return error value */
if ((lossy && !lookup) || !namelen)
- return (lossy & NLS_NAME_OVERLEN) ? -ENAMETOOLONG : -EINVAL;
+ return lossy ? -ENAMETOOLONG : -EINVAL;
return 0;
}
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c
index 8243d94ceaf4..57db08a5271c 100644
--- a/fs/exfat/nls.c
+++ b/fs/exfat/nls.c
@@ -616,9 +616,6 @@ static int exfat_nls_to_ucs2(struct super_block *sb,
unilen++;
}
- if (p_cstring[i] != '\0')
- lossy |= NLS_NAME_OVERLEN;
-
*uniname = '\0';
p_uniname->name_len = unilen;
p_uniname->name_hash = exfat_calc_chksum16(upname, unilen << 1, 0,
--
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v4] exfat: fix out-of-bounds in exfat_nls_to_ucs2()
2025-10-15 7:34 [PATCH v4] exfat: fix out-of-bounds in exfat_nls_to_ucs2() Jeongjun Park
@ 2025-10-15 8:56 ` Namjae Jeon
0 siblings, 0 replies; 2+ messages in thread
From: Namjae Jeon @ 2025-10-15 8:56 UTC (permalink / raw)
To: Jeongjun Park
Cc: Sungjong Seo, Yuezhang Mo, pali, Ethan Ferguson, linux-fsdevel,
linux-kernel, syzbot+98cc76a76de46b3714d4
On Wed, Oct 15, 2025 at 4:35 PM Jeongjun Park <aha310510@gmail.com> wrote:
>
> Since the len argument value passed to exfat_ioctl_set_volume_label()
> from exfat_nls_to_utf16() is passed 1 too large, an out-of-bounds read
> occurs when dereferencing p_cstring in exfat_nls_to_ucs2() later.
>
> And because of the NLS_NAME_OVERLEN macro, another error occurs when
> creating a file with a period at the end using utf8 and other iocharsets.
>
> So to avoid this, you should remove the code that uses NLS_NAME_OVERLEN
> macro and make the len argument value be the length of the label string,
> but with a maximum length of FSLABEL_MAX - 1.
>
> Reported-by: syzbot+98cc76a76de46b3714d4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=98cc76a76de46b3714d4
> Fixes: d01579d590f7 ("exfat: Add support for FS_IOC_{GET,SET}FSLABEL")
> Suggested-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
> fs/exfat/exfat_fs.h | 1 -
> fs/exfat/file.c | 7 ++++---
> fs/exfat/namei.c | 2 +-
> fs/exfat/nls.c | 3 ---
> 4 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
> index 329697c89d09..38210fb6901c 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -29,7 +29,6 @@ enum exfat_error_mode {
> enum {
> NLS_NAME_NO_LOSSY = 0, /* no lossy */
> NLS_NAME_LOSSY = 1 << 0, /* just detected incorrect filename(s) */
> - NLS_NAME_OVERLEN = 1 << 1, /* the length is over than its limit */
> };
>
> #define EXFAT_HASH_BITS 8
> diff --git a/fs/exfat/file.c b/fs/exfat/file.c
> index f246cf439588..adc37b4d7fc2 100644
> --- a/fs/exfat/file.c
> +++ b/fs/exfat/file.c
> @@ -509,8 +509,8 @@ static int exfat_ioctl_get_volume_label(struct super_block *sb, unsigned long ar
> static int exfat_ioctl_set_volume_label(struct super_block *sb,
> unsigned long arg)
> {
> - int ret = 0, lossy;
> - char label[FSLABEL_MAX];
> + int ret = 0, lossy, label_len;
> + char label[FSLABEL_MAX] = {0};
> struct exfat_uni_name uniname;
>
> if (!capable(CAP_SYS_ADMIN))
> @@ -520,8 +520,9 @@ static int exfat_ioctl_set_volume_label(struct super_block *sb,
> return -EFAULT;
>
> memset(&uniname, 0, sizeof(uniname));
> + label_len = strnlen(label, FSLABEL_MAX - 1);
> if (label[0]) {
> - ret = exfat_nls_to_utf16(sb, label, FSLABEL_MAX,
> + ret = exfat_nls_to_utf16(sb, label, label_len,
> &uniname, &lossy);
> if (ret < 0)
> return ret;
> diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
> index 7eb9c67fd35f..a6426a52fd29 100644
> --- a/fs/exfat/namei.c
> +++ b/fs/exfat/namei.c
> @@ -442,7 +442,7 @@ static int __exfat_resolve_path(struct inode *inode, const unsigned char *path,
> return namelen; /* return error value */
>
> if ((lossy && !lookup) || !namelen)
> - return (lossy & NLS_NAME_OVERLEN) ? -ENAMETOOLONG : -EINVAL;
> + return lossy ? -ENAMETOOLONG : -EINVAL;
+ return -EINVAL;
I have directly changed it and applied it to #dev.
Thanks!
>
> return 0;
> }
> diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c
> index 8243d94ceaf4..57db08a5271c 100644
> --- a/fs/exfat/nls.c
> +++ b/fs/exfat/nls.c
> @@ -616,9 +616,6 @@ static int exfat_nls_to_ucs2(struct super_block *sb,
> unilen++;
> }
>
> - if (p_cstring[i] != '\0')
> - lossy |= NLS_NAME_OVERLEN;
> -
> *uniname = '\0';
> p_uniname->name_len = unilen;
> p_uniname->name_hash = exfat_calc_chksum16(upname, unilen << 1, 0,
> --
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-10-15 8:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-15 7:34 [PATCH v4] exfat: fix out-of-bounds in exfat_nls_to_ucs2() Jeongjun Park
2025-10-15 8:56 ` Namjae Jeon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).