linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] exfat: move utf8 mount option setup to exfat_parse_param()
@ 2025-09-25 18:40 Sang-Heon Jeon
  0 siblings, 0 replies; 4+ messages in thread
From: Sang-Heon Jeon @ 2025-09-25 18:40 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo, yuezhang.mo
  Cc: linux-fsdevel, Sang-Heon Jeon, syzbot+3e9cb93e3c5f90d28e19

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.
---
 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) {
-- 
2.43.0


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

* Re: [PATCH] exfat: move utf8 mount option setup to exfat_parse_param()
@ 2025-09-26  3:40 Yuezhang.Mo
  2025-09-26  4:12 ` Sang-Heon Jeon
  0 siblings, 1 reply; 4+ messages in thread
From: Yuezhang.Mo @ 2025-09-26  3:40 UTC (permalink / raw)
  To: ekffu200098@gmail.com
  Cc: linkinjeon@kernel.org, linux-fsdevel@vger.kernel.org,
	sj1557.seo@samsung.com

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.

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

> ---
>  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) {


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

* Re: [PATCH] exfat: move utf8 mount option setup to exfat_parse_param()
  2025-09-26  3:40 [PATCH] exfat: move utf8 mount option setup to exfat_parse_param() Yuezhang.Mo
@ 2025-09-26  4:12 ` Sang-Heon Jeon
  2025-09-26  5:19   ` Yuezhang.Mo
  0 siblings, 1 reply; 4+ messages in thread
From: Sang-Heon Jeon @ 2025-09-26  4:12 UTC (permalink / raw)
  To: Yuezhang.Mo@sony.com
  Cc: linkinjeon@kernel.org, linux-fsdevel@vger.kernel.org,
	sj1557.seo@samsung.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

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

* Re: [PATCH] exfat: move utf8 mount option setup to exfat_parse_param()
  2025-09-26  4:12 ` Sang-Heon Jeon
@ 2025-09-26  5:19   ` Yuezhang.Mo
  0 siblings, 0 replies; 4+ messages in thread
From: Yuezhang.Mo @ 2025-09-26  5:19 UTC (permalink / raw)
  To: Sang-Heon Jeon
  Cc: linkinjeon@kernel.org, linux-fsdevel@vger.kernel.org,
	sj1557.seo@samsung.com

On 2025/9/26 12:12, Sang-Heon Jeon wrote:
> 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://urldefense.com/v3/__https://syzkaller.appspot.com/bug?extid=3e9cb93e3c5f90d28e19__;!!O7_YSHcmd9jp3hj_4dEAcyQ!3hi3dbeYDNZiImOtB9mLnZ7Y7SKGKL4Lsdj4C98YsbefK4sh51BbPM-i1ZW9qG2ZIXopMQEd$
> > > 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.

It is good, thanks.

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

end of thread, other threads:[~2025-09-26  5:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-26  3:40 [PATCH] exfat: move utf8 mount option setup to exfat_parse_param() Yuezhang.Mo
2025-09-26  4:12 ` Sang-Heon Jeon
2025-09-26  5:19   ` Yuezhang.Mo
  -- strict thread matches above, loose matches on Subject: below --
2025-09-25 18:40 Sang-Heon 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).