[PATCH 1/2] f2fs: fix to avoid NULL pointer dereference in f2fs_check_quota_consistency()

[PATCH 1/2] f2fs: fix to avoid NULL pointer dereference in f2fs_check_quota_consistency()

[Chao Yu]

syzbot reported a f2fs bug as below:

Oops: gen[  107.736417][ T5848] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
CPU: 1 UID: 0 PID: 5848 Comm: syz-executor263 Tainted: G        W           6.17.0-rc1-syzkaller-00014-g0e39a731820a #0 PREEMPT_{RT,(full)}
RIP: 0010:strcmp+0x3c/0xc0 lib/string.c:284
Call Trace:
 <TASK>
 f2fs_check_quota_consistency fs/f2fs/super.c:1188 [inline]
 f2fs_check_opt_consistency+0x1378/0x2c10 fs/f2fs/super.c:1436
 __f2fs_remount fs/f2fs/super.c:2653 [inline]
 f2fs_reconfigure+0x482/0x1770 fs/f2fs/super.c:5297
 reconfigure_super+0x224/0x890 fs/super.c:1077
 do_remount fs/namespace.c:3314 [inline]
 path_mount+0xd18/0xfe0 fs/namespace.c:4112
 do_mount fs/namespace.c:4133 [inline]
 __do_sys_mount fs/namespace.c:4344 [inline]
 __se_sys_mount+0x317/0x410 fs/namespace.c:4321
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

The direct reason is f2fs_check_quota_consistency() may suffer null-ptr-deref
issue in strcmp().

The bug can be reproduced w/ below scripts:
mkfs.f2fs -f /dev/vdb
mount -t f2fs -o usrquota /dev/vdb /mnt/f2fs
quotacheck -uc /mnt/f2fs/
umount /mnt/f2fs
mount -t f2fs -o usrjquota=aquota.user,jqfmt=vfsold /dev/vdb /mnt/f2fs
mount -t f2fs -o remount,usrjquota=,jqfmt=vfsold /dev/vdb /mnt/f2fs
umount /mnt/f2fs

So, before old_qname and new_qname comparison, we need to check whether
they are all valid pointers, fix it.

Reported-by: syzbot+d371efea57d5aeab877b@syzkaller.appspotmail.com
Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
Closes: https://lore.kernel.org/linux-f2fs-devel/689ff889.050a0220.e29e5.0037.GAE@google.com
Cc: Hongbo Li <lihongbo22@huawei.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/super.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 5aa9d650512d..465604fdc5dd 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1219,7 +1219,8 @@ static int f2fs_check_quota_consistency(struct fs_context *fc,
 				goto err_jquota_change;
 
 			if (old_qname) {
-				if (strcmp(old_qname, new_qname) == 0) {
+				if (new_qname &&
+					strcmp(old_qname, new_qname) == 0) {
 					ctx->qname_mask &= ~(1 << i);
 					continue;
 				}
-- 
2.49.0

[PATCH 2/2] f2fs: fix to allow removing qf_name

[Chao Yu]

The mount behavior changed after commit d18535132523 ("f2fs: separate the
options parsing and options checking"), let's fix it.

[Scripts]
mkfs.f2fs -f /dev/vdb
mount -t f2fs -o usrquota /dev/vdb /mnt/f2fs
quotacheck -uc /mnt/f2fs
umount /mnt/f2fs
mount -t f2fs -o usrjquota=aquota.user,jqfmt=vfsold /dev/vdb /mnt/f2fs
mount|grep f2fs
mount -t f2fs -o remount,usrjquota=,jqfmt=vfsold /dev/vdb /mnt/f2fs
mount|grep f2fs
dmesg

[Before commit]
mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
mount#2: ...,quota,jqfmt=vfsold,...
kmsg: no output

[After commit]
mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
mount#2: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
kmsg: "user quota file already specified"

[After patch]
mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
mount#2: ...,quota,jqfmt=vfsold,...
kmsg: "remove qf_name aquota.user"

Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
Cc: Hongbo Li <lihongbo22@huawei.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/super.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 465604fdc5dd..07f6c8cac07a 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1219,8 +1219,11 @@ static int f2fs_check_quota_consistency(struct fs_context *fc,
 				goto err_jquota_change;
 
 			if (old_qname) {
-				if (new_qname &&
-					strcmp(old_qname, new_qname) == 0) {
+				if (!new_qname) {
+					f2fs_info(sbi, "remove qf_name %s",
+								old_qname);
+					continue;
+				} else if (strcmp(old_qname, new_qname) == 0) {
 					ctx->qname_mask &= ~(1 << i);
 					continue;
 				}
-- 
2.49.0

Re: [PATCH 2/2] f2fs: fix to allow removing qf_name

[Hongbo Li]

Hi Chao,

On 2025/8/18 10:09, Chao Yu wrote:
> The mount behavior changed after commit d18535132523 ("f2fs: separate the
> options parsing and options checking"), let's fix it.
> 
> [Scripts]
> mkfs.f2fs -f /dev/vdb
> mount -t f2fs -o usrquota /dev/vdb /mnt/f2fs
> quotacheck -uc /mnt/f2fs
> umount /mnt/f2fs
> mount -t f2fs -o usrjquota=aquota.user,jqfmt=vfsold /dev/vdb /mnt/f2fs
> mount|grep f2fs
> mount -t f2fs -o remount,usrjquota=,jqfmt=vfsold /dev/vdb /mnt/f2fs
> mount|grep f2fs
> dmesg
> 
> [Before commit]
> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
> mount#2: ...,quota,jqfmt=vfsold,...
> kmsg: no output
> 
> [After commit]
> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
> mount#2: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
> kmsg: "user quota file already specified"
> 
> [After patch]
> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
> mount#2: ...,quota,jqfmt=vfsold,...
> kmsg: "remove qf_name aquota.user"
> 
> Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
> Cc: Hongbo Li <lihongbo22@huawei.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>   fs/f2fs/super.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 465604fdc5dd..07f6c8cac07a 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1219,8 +1219,11 @@ static int f2fs_check_quota_consistency(struct fs_context *fc,
>   				goto err_jquota_change;
>   
>   			if (old_qname) {
> -				if (new_qname &&
> -					strcmp(old_qname, new_qname) == 0) {
> +				if (!new_qname) {

Thanks for catching this. Do we also need the patch 1/2 ? It seems this 
patch also solve the syzbot problems.

Thanks,
Hongbo

> +					f2fs_info(sbi, "remove qf_name %s",
> +								old_qname);
> +					continue;
> +				} else if (strcmp(old_qname, new_qname) == 0) {
>   					ctx->qname_mask &= ~(1 << i);
>   					continue;
>   				}

Re: [PATCH 2/2] f2fs: fix to allow removing qf_name

[Chao Yu]

On 8/18/25 16:06, Hongbo Li wrote:
> Hi Chao,
> 
> On 2025/8/18 10:09, Chao Yu wrote:
>> The mount behavior changed after commit d18535132523 ("f2fs: separate the
>> options parsing and options checking"), let's fix it.
>>
>> [Scripts]
>> mkfs.f2fs -f /dev/vdb
>> mount -t f2fs -o usrquota /dev/vdb /mnt/f2fs
>> quotacheck -uc /mnt/f2fs
>> umount /mnt/f2fs
>> mount -t f2fs -o usrjquota=aquota.user,jqfmt=vfsold /dev/vdb /mnt/f2fs
>> mount|grep f2fs
>> mount -t f2fs -o remount,usrjquota=,jqfmt=vfsold /dev/vdb /mnt/f2fs
>> mount|grep f2fs
>> dmesg
>>
>> [Before commit]
>> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
>> mount#2: ...,quota,jqfmt=vfsold,...
>> kmsg: no output
>>
>> [After commit]
>> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
>> mount#2: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
>> kmsg: "user quota file already specified"
>>
>> [After patch]
>> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
>> mount#2: ...,quota,jqfmt=vfsold,...
>> kmsg: "remove qf_name aquota.user"
>>
>> Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
>> Cc: Hongbo Li <lihongbo22@huawei.com>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fs/f2fs/super.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 465604fdc5dd..07f6c8cac07a 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -1219,8 +1219,11 @@ static int f2fs_check_quota_consistency(struct fs_context *fc,
>>                   goto err_jquota_change;
>>                 if (old_qname) {
>> -                if (new_qname &&
>> -                    strcmp(old_qname, new_qname) == 0) {
>> +                if (!new_qname) {
> 
> Thanks for catching this. Do we also need the patch 1/2 ? It seems this patch also solve the syzbot problems.

I prefer to split it, since it'd better to use one patch to resolve one
problem. :)

Thanks,

> 
> Thanks,
> Hongbo
> 
>> +                    f2fs_info(sbi, "remove qf_name %s",
>> +                                old_qname);
>> +                    continue;
>> +                } else if (strcmp(old_qname, new_qname) == 0) {
>>                       ctx->qname_mask &= ~(1 << i);
>>                       continue;
>>                   }

Re: [PATCH 2/2] f2fs: fix to allow removing qf_name

[Hongbo Li]

On 2025/8/18 10:09, Chao Yu wrote:
> The mount behavior changed after commit d18535132523 ("f2fs: separate the
> options parsing and options checking"), let's fix it.
> 
> [Scripts]
> mkfs.f2fs -f /dev/vdb
> mount -t f2fs -o usrquota /dev/vdb /mnt/f2fs
> quotacheck -uc /mnt/f2fs
> umount /mnt/f2fs
> mount -t f2fs -o usrjquota=aquota.user,jqfmt=vfsold /dev/vdb /mnt/f2fs
> mount|grep f2fs
> mount -t f2fs -o remount,usrjquota=,jqfmt=vfsold /dev/vdb /mnt/f2fs
> mount|grep f2fs
> dmesg
> 
> [Before commit]
> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
> mount#2: ...,quota,jqfmt=vfsold,...
> kmsg: no output
> 
> [After commit]
> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
> mount#2: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
> kmsg: "user quota file already specified"
> 
> [After patch]
> mount#1: ...,quota,jqfmt=vfsold,usrjquota=aquota.user,...
> mount#2: ...,quota,jqfmt=vfsold,...
> kmsg: "remove qf_name aquota.user"
> 
> Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
> Cc: Hongbo Li <lihongbo22@huawei.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---

Reviewed-by: Hongbo Li <lihongbo22@huawei.com>

Thanks,
Hongbo

>   fs/f2fs/super.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 465604fdc5dd..07f6c8cac07a 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1219,8 +1219,11 @@ static int f2fs_check_quota_consistency(struct fs_context *fc,
>   				goto err_jquota_change;
>   
>   			if (old_qname) {
> -				if (new_qname &&
> -					strcmp(old_qname, new_qname) == 0) {
> +				if (!new_qname) {
> +					f2fs_info(sbi, "remove qf_name %s",
> +								old_qname);
> +					continue;
> +				} else if (strcmp(old_qname, new_qname) == 0) {
>   					ctx->qname_mask &= ~(1 << i);
>   					continue;
>   				}

Re: [PATCH 1/2] f2fs: fix to avoid NULL pointer dereference in f2fs_check_quota_consistency()

[Hongbo Li]

On 2025/8/18 10:09, Chao Yu wrote:
> syzbot reported a f2fs bug as below:
> 
> Oops: gen[  107.736417][ T5848] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN PTI
> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> CPU: 1 UID: 0 PID: 5848 Comm: syz-executor263 Tainted: G        W           6.17.0-rc1-syzkaller-00014-g0e39a731820a #0 PREEMPT_{RT,(full)}
> RIP: 0010:strcmp+0x3c/0xc0 lib/string.c:284
> Call Trace:
>   <TASK>
>   f2fs_check_quota_consistency fs/f2fs/super.c:1188 [inline]
>   f2fs_check_opt_consistency+0x1378/0x2c10 fs/f2fs/super.c:1436
>   __f2fs_remount fs/f2fs/super.c:2653 [inline]
>   f2fs_reconfigure+0x482/0x1770 fs/f2fs/super.c:5297
>   reconfigure_super+0x224/0x890 fs/super.c:1077
>   do_remount fs/namespace.c:3314 [inline]
>   path_mount+0xd18/0xfe0 fs/namespace.c:4112
>   do_mount fs/namespace.c:4133 [inline]
>   __do_sys_mount fs/namespace.c:4344 [inline]
>   __se_sys_mount+0x317/0x410 fs/namespace.c:4321
>   do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>   do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
>   entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> The direct reason is f2fs_check_quota_consistency() may suffer null-ptr-deref
> issue in strcmp().
> 
> The bug can be reproduced w/ below scripts:
> mkfs.f2fs -f /dev/vdb
> mount -t f2fs -o usrquota /dev/vdb /mnt/f2fs
> quotacheck -uc /mnt/f2fs/
> umount /mnt/f2fs
> mount -t f2fs -o usrjquota=aquota.user,jqfmt=vfsold /dev/vdb /mnt/f2fs
> mount -t f2fs -o remount,usrjquota=,jqfmt=vfsold /dev/vdb /mnt/f2fs
> umount /mnt/f2fs
> 
> So, before old_qname and new_qname comparison, we need to check whether
> they are all valid pointers, fix it.
> 
> Reported-by: syzbot+d371efea57d5aeab877b@syzkaller.appspotmail.com
> Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
> Closes: https://lore.kernel.org/linux-f2fs-devel/689ff889.050a0220.e29e5.0037.GAE@google.com
> Cc: Hongbo Li <lihongbo22@huawei.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---

Reviewed-by: Hongbo Li <lihongbo22@huawei.com>

Thanks,
Hongbo

>   fs/f2fs/super.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 5aa9d650512d..465604fdc5dd 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1219,7 +1219,8 @@ static int f2fs_check_quota_consistency(struct fs_context *fc,
>   				goto err_jquota_change;
>   
>   			if (old_qname) {
> -				if (strcmp(old_qname, new_qname) == 0) {
> +				if (new_qname &&
> +					strcmp(old_qname, new_qname) == 0) {
>   					ctx->qname_mask &= ~(1 << i);
>   					continue;
>   				}

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix to avoid NULL pointer dereference in f2fs_check_quota_consistency()

[patchwork-bot+f2fs]

Hello:

This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Mon, 18 Aug 2025 10:09:38 +0800 you wrote:
> syzbot reported a f2fs bug as below:
> 
> Oops: gen[  107.736417][ T5848] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN PTI
> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> CPU: 1 UID: 0 PID: 5848 Comm: syz-executor263 Tainted: G        W           6.17.0-rc1-syzkaller-00014-g0e39a731820a #0 PREEMPT_{RT,(full)}
> RIP: 0010:strcmp+0x3c/0xc0 lib/string.c:284
> Call Trace:
>  <TASK>
>  f2fs_check_quota_consistency fs/f2fs/super.c:1188 [inline]
>  f2fs_check_opt_consistency+0x1378/0x2c10 fs/f2fs/super.c:1436
>  __f2fs_remount fs/f2fs/super.c:2653 [inline]
>  f2fs_reconfigure+0x482/0x1770 fs/f2fs/super.c:5297
>  reconfigure_super+0x224/0x890 fs/super.c:1077
>  do_remount fs/namespace.c:3314 [inline]
>  path_mount+0xd18/0xfe0 fs/namespace.c:4112
>  do_mount fs/namespace.c:4133 [inline]
>  __do_sys_mount fs/namespace.c:4344 [inline]
>  __se_sys_mount+0x317/0x410 fs/namespace.c:4321
>  do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>  do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,1/2] f2fs: fix to avoid NULL pointer dereference in f2fs_check_quota_consistency()
    https://git.kernel.org/jaegeuk/f2fs/c/930a9a6ee8e7
  - [f2fs-dev,2/2] f2fs: fix to allow removing qf_name
    https://git.kernel.org/jaegeuk/f2fs/c/ff11d8701b77

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html