[PATCH] f2fs: avoid NULL checkpoint thread access in sysfs

[PATCH] f2fs: avoid NULL checkpoint thread access in sysfs

[Wenjie Qi]

checkpoint_merge is set by default, but the checkpoint merge thread is not
created for read-only mounts. The ckpt_thread_ioprio sysfs store path uses
the mount option to decide whether to update the task ioprio. On such
mounts, writing the node passes a NULL task to set_task_ioprio().

Keep storing the requested ioprio, but apply it only when the checkpoint
thread exists. Take s_umount for ckpt_thread_ioprio and
critical_task_priority too, matching the existing protection for GC thread
entries.

Fixes: e65920661708 ("f2fs: add ckpt_thread_ioprio sysfs node")
Cc: stable@kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
Reproducer:
  mount -o ro -t f2fs /dev/vdb /mnt/f2fs
  echo be,4 > /sys/fs/f2fs/vdb/ckpt_thread_ioprio

Baseline dmesg:
  Oops: general protection fault
  KASAN: null-ptr-deref in range [0x00000000000007b0-0x00000000000007b7]
  RIP: set_task_ioprio+0x8b/0x360
  Call Trace:
   f2fs_sbi_store+0x2af/0x2580
   kernfs_fop_write_iter+0x360/0x620
   vfs_write+0x5f8/0xf50
   ksys_write+0xf9/0x1d0
   do_syscall_64+0x5f/0x550

 fs/f2fs/sysfs.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index be92c05a5420..0294544d3d34 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -557,7 +557,7 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
 			return -EINVAL;
 
 		cprc->ckpt_thread_ioprio = IOPRIO_PRIO_VALUE(class, level);
-		if (test_opt(sbi, MERGE_CHECKPOINT)) {
+		if (cprc->f2fs_issue_ckpt) {
 			ret = set_task_ioprio(cprc->f2fs_issue_ckpt,
 					cprc->ckpt_thread_ioprio);
 			if (ret)
@@ -1007,13 +1007,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 	ssize_t ret;
 	bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
 					a->struct_type == GC_THREAD);
+	bool thread_entry = gc_entry ||
+			!strcmp(a->attr.name, "ckpt_thread_ioprio") ||
+			!strcmp(a->attr.name, "critical_task_priority");
 
-	if (gc_entry) {
+	if (thread_entry) {
 		if (!down_read_trylock(&sbi->sb->s_umount))
 			return -EAGAIN;
 	}
 	ret = __sbi_store(a, sbi, buf, count);
-	if (gc_entry)
+	if (thread_entry)
 		up_read(&sbi->sb->s_umount);
 
 	return ret;
-- 
2.43.0

Re: [PATCH] f2fs: avoid NULL checkpoint thread access in sysfs

[Chao Yu]

On 7/24/26 17:05, Wenjie Qi wrote:
> checkpoint_merge is set by default, but the checkpoint merge thread is not
> created for read-only mounts. The ckpt_thread_ioprio sysfs store path uses
> the mount option to decide whether to update the task ioprio. On such
> mounts, writing the node passes a NULL task to set_task_ioprio().
> 
> Keep storing the requested ioprio, but apply it only when the checkpoint
> thread exists. Take s_umount for ckpt_thread_ioprio and
> critical_task_priority too, matching the existing protection for GC thread
> entries.
> 
> Fixes: e65920661708 ("f2fs: add ckpt_thread_ioprio sysfs node")
> Cc: stable@kernel.org
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> ---
> Reproducer:
>    mount -o ro -t f2fs /dev/vdb /mnt/f2fs
>    echo be,4 > /sys/fs/f2fs/vdb/ckpt_thread_ioprio
> 
> Baseline dmesg:
>    Oops: general protection fault
>    KASAN: null-ptr-deref in range [0x00000000000007b0-0x00000000000007b7]
>    RIP: set_task_ioprio+0x8b/0x360
>    Call Trace:
>     f2fs_sbi_store+0x2af/0x2580
>     kernfs_fop_write_iter+0x360/0x620
>     vfs_write+0x5f8/0xf50
>     ksys_write+0xf9/0x1d0
>     do_syscall_64+0x5f/0x550
> 
>   fs/f2fs/sysfs.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index be92c05a5420..0294544d3d34 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -557,7 +557,7 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
>   			return -EINVAL;
>   
>   		cprc->ckpt_thread_ioprio = IOPRIO_PRIO_VALUE(class, level);
> -		if (test_opt(sbi, MERGE_CHECKPOINT)) {
> +		if (cprc->f2fs_issue_ckpt) {
>   			ret = set_task_ioprio(cprc->f2fs_issue_ckpt,
>   					cprc->ckpt_thread_ioprio);
>   			if (ret)
> @@ -1007,13 +1007,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>   	ssize_t ret;
>   	bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
>   					a->struct_type == GC_THREAD);
> +	bool thread_entry = gc_entry ||
> +			!strcmp(a->attr.name, "ckpt_thread_ioprio") ||
> +			!strcmp(a->attr.name, "critical_task_priority");

Oh, seems you fixed another race issue in this patch as well?

"mount -o remount,ro" vs "echo xx > critical_task_priority"

Can you fix this in a separated patch?

>   
> -	if (gc_entry) {
> +	if (thread_entry) {

bool thread_entry = !strcmp(a->attr.name, "ckpt_thread_ioprio") ||
			!strcmp(a->attr.name, "critical_task_priority");

if (gc_entry || thread_entry)

>   		if (!down_read_trylock(&sbi->sb->s_umount))
>   			return -EAGAIN;
>   	}
>   	ret = __sbi_store(a, sbi, buf, count);
> -	if (gc_entry)
> +	if (thread_entry)

if (gc_entry || thread_entry)

Thanks,

>   		up_read(&sbi->sb->s_umount);
>   
>   	return ret;

Re: [PATCH] f2fs: avoid NULL checkpoint thread access in sysfs

[Wenjie Qi]

  Yes. This patch also extends the existing s_umount protection to
  ckpt_thread_ioprio and critical_task_priority, which is a separate
  remount-vs-sysfs race issue.

  I'll split it:
  - keep only the NULL f2fs_issue_ckpt fix in this patch
  - send the s_umount protection for ckpt_thread_ioprio and
    critical_task_priority as a separate patch

On Mon, Aug 3, 2026 at 3:53 PM Chao Yu <chao@kernel.org> wrote:
>
> On 7/24/26 17:05, Wenjie Qi wrote:
> > checkpoint_merge is set by default, but the checkpoint merge thread is not
> > created for read-only mounts. The ckpt_thread_ioprio sysfs store path uses
> > the mount option to decide whether to update the task ioprio. On such
> > mounts, writing the node passes a NULL task to set_task_ioprio().
> >
> > Keep storing the requested ioprio, but apply it only when the checkpoint
> > thread exists. Take s_umount for ckpt_thread_ioprio and
> > critical_task_priority too, matching the existing protection for GC thread
> > entries.
> >
> > Fixes: e65920661708 ("f2fs: add ckpt_thread_ioprio sysfs node")
> > Cc: stable@kernel.org
> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> > ---
> > Reproducer:
> >    mount -o ro -t f2fs /dev/vdb /mnt/f2fs
> >    echo be,4 > /sys/fs/f2fs/vdb/ckpt_thread_ioprio
> >
> > Baseline dmesg:
> >    Oops: general protection fault
> >    KASAN: null-ptr-deref in range [0x00000000000007b0-0x00000000000007b7]
> >    RIP: set_task_ioprio+0x8b/0x360
> >    Call Trace:
> >     f2fs_sbi_store+0x2af/0x2580
> >     kernfs_fop_write_iter+0x360/0x620
> >     vfs_write+0x5f8/0xf50
> >     ksys_write+0xf9/0x1d0
> >     do_syscall_64+0x5f/0x550
> >
> >   fs/f2fs/sysfs.c | 9 ++++++---
> >   1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> > index be92c05a5420..0294544d3d34 100644
> > --- a/fs/f2fs/sysfs.c
> > +++ b/fs/f2fs/sysfs.c
> > @@ -557,7 +557,7 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
> >                       return -EINVAL;
> >
> >               cprc->ckpt_thread_ioprio = IOPRIO_PRIO_VALUE(class, level);
> > -             if (test_opt(sbi, MERGE_CHECKPOINT)) {
> > +             if (cprc->f2fs_issue_ckpt) {
> >                       ret = set_task_ioprio(cprc->f2fs_issue_ckpt,
> >                                       cprc->ckpt_thread_ioprio);
> >                       if (ret)
> > @@ -1007,13 +1007,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
> >       ssize_t ret;
> >       bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
> >                                       a->struct_type == GC_THREAD);
> > +     bool thread_entry = gc_entry ||
> > +                     !strcmp(a->attr.name, "ckpt_thread_ioprio") ||
> > +                     !strcmp(a->attr.name, "critical_task_priority");
>
> Oh, seems you fixed another race issue in this patch as well?
>
> "mount -o remount,ro" vs "echo xx > critical_task_priority"
>
> Can you fix this in a separated patch?
>
> >
> > -     if (gc_entry) {
> > +     if (thread_entry) {
>
> bool thread_entry = !strcmp(a->attr.name, "ckpt_thread_ioprio") ||
>                         !strcmp(a->attr.name, "critical_task_priority");
>
> if (gc_entry || thread_entry)
>
> >               if (!down_read_trylock(&sbi->sb->s_umount))
> >                       return -EAGAIN;
> >       }
> >       ret = __sbi_store(a, sbi, buf, count);
> > -     if (gc_entry)
> > +     if (thread_entry)
>
> if (gc_entry || thread_entry)
>
> Thanks,
>
> >               up_read(&sbi->sb->s_umount);
> >
> >       return ret;
>