linux-kernel - Re: [PATCH v4] exfat: fix out-of-bounds in exfat_nls_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKYAXd_4AOm2fjyBM5FF=1+aAM2Qj5_34PZzjD+riz3s=SCQQQ@mail.gmail.com>
Date: Wed, 15 Oct 2025 17:56:39 +0900
From: Namjae Jeon <linkinjeon@...nel.org>
To: Jeongjun Park <aha310510@...il.com>
Cc: Sungjong Seo <sj1557.seo@...sung.com>, Yuezhang Mo <yuezhang.mo@...y.com>, pali@...nel.org, 
	Ethan Ferguson <ethan.ferguson@...ier.com>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	syzbot+98cc76a76de46b3714d4@...kaller.appspotmail.com
Subject: Re: [PATCH v4] exfat: fix out-of-bounds in exfat_nls_to_ucs2()

On Wed, Oct 15, 2025 at 4:35 PM Jeongjun Park <aha310510@...il.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@...kaller.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@...nel.org>
> Signed-off-by: Jeongjun Park <aha310510@...il.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,
> --