Re: [PATCH] exfat: move utf8 mount option setup to exfat_parse_param()

[Sang-Heon Jeon <ekffu200098@gmail.com>]

On Fri, Sep 26, 2025 at 12:40 PM Yuezhang.Mo@sony.com
<Yuezhang.Mo@sony.com> wrote:
>
> On 2025/9/26 2:40, Sang-Heon Jeon wrote:
> > Currently, exfat utf8 mount option depends on the iocharset option
> > value. After exfat remount, utf8 option may become inconsistent with
> > iocharset option.
> >
> > If the options are inconsistent; (specifically, iocharset=utf8 but
> > utf8=0) readdir may reference uninitalized NLS, leading to a null
> > pointer dereference.
> >
> > Move utf8 option setup logic from exfat_fill_super() to
> > exfat_parse_param() to prevent utf8/iocharset option inconsistency
> > after remount.
> >
> > Reported-by: syzbot+3e9cb93e3c5f90d28e19@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=3e9cb93e3c5f90d28e19
> > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > Fixes: acab02ffcd6b ("exfat: support modifying mount options via remount")
> > Tested-by: syzbot+3e9cb93e3c5f90d28e19@syzkaller.appspotmail.com
> > ---
> > Instead of moving `utf8` mount option (also, can resolve this problem)
> > setup to exfat_parse_param(), we can re-setup `utf8` mount option on
> > exfat_reconfigure(). IMHO, it's better to move setup logic to parse
> > section in terms of consistency.
> >
> > If my analysis is wrong or If there is better approach, please let me
> > know. Thanks for your consideration.
>
> It makes sense to put settings utf8 and iocharset together.

Thanks for reviewing, Yuezhang!

> If so, utf8 is also needed to set in exfat_init_fs_context(), otherwise
> utf8 will not be initialized if mounted without specifying iocharset.

Oh, I missed that point. I'd prefer to extract setting utf and
iocharset both to a tiny function, and then call it both
exfat_param_param() and exfa_init_fs_context(), then we can prevent
inconsistency between two values. If you don't have any other
opinions, I'll make a v2 patch this way.

> > ---
> >  fs/exfat/super.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/exfat/super.c b/fs/exfat/super.c
> > index e1cffa46eb73..3b07b2a5502d 100644
> > --- a/fs/exfat/super.c
> > +++ b/fs/exfat/super.c
> > @@ -293,6 +293,12 @@ static int exfat_parse_param(struct fs_context *fc, struct fs_parameter *param)
> >       case Opt_charset:
> >               exfat_free_iocharset(sbi);
> >               opts->iocharset = param->string;
> > +
> > +             if (!strcmp(opts->iocharset, "utf8"))
> > +                     opts->utf8 = 1;
> > +             else
> > +                     opts->utf8 = 0;
> > +
> >               param->string = NULL;
> >               break;
> >       case Opt_errors:
> > @@ -664,8 +670,8 @@ static int exfat_fill_super(struct super_block *sb, struct fs_context *fc)
> >       /* set up enough so that it can read an inode */
> >       exfat_hash_init(sb);
> >
> > -     if (!strcmp(sbi->options.iocharset, "utf8"))
> > -             opts->utf8 = 1;
> > +     if (sbi->options.utf8)
> > +             set_default_d_op(sb, &exfat_utf8_dentry_ops);
> >       else {
> >               sbi->nls_io = load_nls(sbi->options.iocharset);
> >               if (!sbi->nls_io) {
> > @@ -674,12 +680,8 @@ static int exfat_fill_super(struct super_block *sb, struct fs_context *fc)
> >                       err = -EINVAL;
> >                       goto free_table;
> >               }
> > -     }
> > -
> > -     if (sbi->options.utf8)
> > -             set_default_d_op(sb, &exfat_utf8_dentry_ops);
> > -     else
> >               set_default_d_op(sb, &exfat_dentry_ops);
> > +     }
> >
> >       root_inode = new_inode(sb);
> >       if (!root_inode) {
>

Best Regards,
Sang-Heon Jeon