[PATCH v4 1/2] f2fs: avoid NULL checkpoint thread access in sysfs

[PATCH v4 1/2] f2fs: avoid NULL checkpoint thread access in sysfs

[Wenjie Qi]

checkpoint_merge can be enabled even when no checkpoint merge thread is
running. A read-only mount is one case: f2fs does not start
f2fs_issue_ckpt there, but ckpt_thread_ioprio is still writable through
sysfs.

The ckpt_thread_ioprio store path updates the saved ioprio value and,
when checkpoint_merge is enabled, calls set_task_ioprio() for the
checkpoint thread. If cprc->f2fs_issue_ckpt is NULL, that dereferences a
NULL task pointer.

Protect ckpt_thread_ioprio sysfs writes with s_umount as well, so the
checkpoint thread cannot disappear under the store path while updating
its ioprio.

Fixes: e65920661708 ("f2fs: add ckpt_thread_ioprio sysfs node")
Cc: stable@kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
 fs/f2fs/sysfs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index be92c05a5420..0729b3670415 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,14 @@ 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 = !strcmp(a->attr.name, "ckpt_thread_ioprio");
 
-	if (gc_entry) {
+	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 (gc_entry || thread_entry)
 		up_read(&sbi->sb->s_umount);
 
 	return ret;
-- 
2.43.0

[PATCH v4 2/2] f2fs: protect critical_task_priority updates with s_umount

[Wenjie Qi]

The sysfs store path already takes s_umount for GC thread control
entries, and ckpt_thread_ioprio is covered as well.

critical_task_priority also updates checkpoint or GC kthread scheduling
state, but it is not covered by that serialization. It can race with
remount or teardown paths that are stopping those threads.

Protect critical_task_priority sysfs writes with s_umount too.

Fixes: 52190933c37a ("f2fs: sysfs: introduce critical_task_priority")
Cc: stable@kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
v4:
- add Fixes tag
- add Cc: stable@kernel.org
---
 fs/f2fs/sysfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 0729b3670415..aa1621419932 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -1007,7 +1007,8 @@ 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 = !strcmp(a->attr.name, "ckpt_thread_ioprio");
+	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))
-- 
2.43.0

Re: [PATCH v4 2/2] f2fs: protect critical_task_priority updates with s_umount

[Chao Yu]

On 8/4/26 09:48, Wenjie Qi wrote:
> The sysfs store path already takes s_umount for GC thread control
> entries, and ckpt_thread_ioprio is covered as well.
> 
> critical_task_priority also updates checkpoint or GC kthread scheduling
> state, but it is not covered by that serialization. It can race with
> remount or teardown paths that are stopping those threads.
> 
> Protect critical_task_priority sysfs writes with s_umount too.
> 
> Fixes: 52190933c37a ("f2fs: sysfs: introduce critical_task_priority")
> Cc: stable@kernel.org
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [PATCH v4 1/2] f2fs: avoid NULL checkpoint thread access in sysfs

[Chao Yu]

On 8/4/26 09:48, Wenjie Qi wrote:
> checkpoint_merge can be enabled even when no checkpoint merge thread is
> running. A read-only mount is one case: f2fs does not start
> f2fs_issue_ckpt there, but ckpt_thread_ioprio is still writable through
> sysfs.
> 
> The ckpt_thread_ioprio store path updates the saved ioprio value and,
> when checkpoint_merge is enabled, calls set_task_ioprio() for the
> checkpoint thread. If cprc->f2fs_issue_ckpt is NULL, that dereferences a
> NULL task pointer.
> 
> Protect ckpt_thread_ioprio sysfs writes with s_umount as well, so the
> checkpoint thread cannot disappear under the store path while updating
> its ioprio.
> 
> Fixes: e65920661708 ("f2fs: add ckpt_thread_ioprio sysfs node")
> Cc: stable@kernel.org
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH v4 1/2] f2fs: avoid NULL checkpoint thread access in sysfs

[patchwork-bot+f2fs]

Hello:

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

On Tue,  4 Aug 2026 09:48:48 +0800 you wrote:
> checkpoint_merge can be enabled even when no checkpoint merge thread is
> running. A read-only mount is one case: f2fs does not start
> f2fs_issue_ckpt there, but ckpt_thread_ioprio is still writable through
> sysfs.
> 
> The ckpt_thread_ioprio store path updates the saved ioprio value and,
> when checkpoint_merge is enabled, calls set_task_ioprio() for the
> checkpoint thread. If cprc->f2fs_issue_ckpt is NULL, that dereferences a
> NULL task pointer.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,v4,1/2] f2fs: avoid NULL checkpoint thread access in sysfs
    https://git.kernel.org/jaegeuk/f2fs/c/5cb33b00c8fb
  - [f2fs-dev,v4,2/2] f2fs: protect critical_task_priority updates with s_umount
    https://git.kernel.org/jaegeuk/f2fs/c/8e4692c6c165

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