* Re: [PATCH] ocfs2: Publish the filecheck owner before adding sysfs attributes
2026-10-09 10:04 [PATCH] ocfs2: Publish the filecheck owner before adding sysfs attributes Cen Zhang
@ 2026-10-10 7:38 ` Heming Zhao
2026-10-11 15:40 ` Cen Zhang
2026-10-12 5:13 ` Heming Zhao
2026-10-12 6:18 ` Joseph Qi
2 siblings, 1 reply; 6+ messages in thread
From: Heming Zhao @ 2026-10-10 7:38 UTC (permalink / raw)
To: Cen Zhang
Cc: mark, jlbec, joseph.qi, akpm, ghe, ocfs2-devel, linux-kernel,
baijiaju1990, jjzuming
On Fri, Oct 09, 2026 at 06:04:49PM +0800, Cen Zhang wrote:
> ocfs2_filecheck_create_sysfs() initializes the filecheck allocation
> before calling kobject_init_and_add(), but assigns entry->fs_fcheck
> only after that call returns. The default attribute group can already
> be visible while registration is in progress. A concurrent read can
> therefore enter ocfs2_filecheck_attr_show() with a NULL fs_fcheck and
> try to lock its fc_lock.
>
> Assign the initialized owner before adding the kobject, so every
> activated attribute has valid state. Clear the pointer when addition
> fails, preserving the existing teardown guard and error cleanup.
>
> A controlled probe-instrumented test kernel with the original OCFS2
> registration ordering produced this Oops during a concurrent sysfs read:
>
> KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
> RIP: 0010:kasan_byte_accessible+0x15/0x30
> Call Trace:
> <TASK>
> __kasan_check_byte+0x13/0x50
> lock_acquire+0x113/0x300
> _raw_spin_lock+0x31/0x40
> ocfs2_filecheck_attr_show+0x138/0x5f0
> ocfs2_filecheck_show+0x5d/0x90
> sysfs_kf_seq_show+0x1d3/0x360
> seq_read_iter+0x480/0x1110
> kernfs_fop_read_iter+0x419/0x5a0
> vfs_read+0x7d2/0xc20
> ksys_read+0x111/0x200
> do_syscall_64+0x114/0x620
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
> Modules linked in:
>
> Fixes: a860f6eb4c6a ("ocfs2: sysfile interfaces for online file check")
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> fs/ocfs2/filecheck.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ocfs2/filecheck.c b/fs/ocfs2/filecheck.c
> index be713d7d24a76b9aa8fcded3d83b3d91d124eb6e..862c1ee7099c943b2e3e57e31f887380c6dc8a88 100644
> --- a/fs/ocfs2/filecheck.c
> +++ b/fs/ocfs2/filecheck.c
> @@ -181,2 +181,4 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
>
> + /* Publish the initialized owner before activating its sysfs callbacks. */
> + entry->fs_fcheck = fcheck;
> entry->fs_kobj.kset = osb->osb_dev_kset;
> @@ -186,2 +188,3 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
> if (ret) {
> + entry->fs_fcheck = NULL;
setting to NULL is unnecessary, it becomes a harmless dangling pointer here.
Other code looks good to me.
Thanks,
Heming
> kobject_put(&entry->fs_kobj);
> @@ -191,3 +194,2 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
>
> - entry->fs_fcheck = fcheck;
> return 0;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ocfs2: Publish the filecheck owner before adding sysfs attributes
2026-10-10 7:38 ` Heming Zhao
@ 2026-10-11 15:40 ` Cen Zhang
2026-10-12 5:10 ` Heming Zhao
0 siblings, 1 reply; 6+ messages in thread
From: Cen Zhang @ 2026-10-11 15:40 UTC (permalink / raw)
To: Heming Zhao
Cc: mark, jlbec, joseph.qi, akpm, ghe, ocfs2-devel, linux-kernel,
baijiaju1990, jjzuming
Hi Heming,
Heming Zhao <heming.zhao@suse.com> 于2026年10月10日周六 15:38写道：
>
> setting to NULL is unnecessary, it becomes a harmless dangling pointer here.
> Other code looks good to me.
>
> Thanks,
> Heming
Thanks for reviewing this.
I checked the failure path again with the help of LLM, and I think clearing
fs_fcheck is still needed on this baseline. If kobject_init_and_add()
fails, the
function puts the kobject and frees fcheck. Its caller in
ocfs2_fill_super() then
jumps to out_dismount, which calls ocfs2_dismount_volume() and, in turn,
ocfs2_filecheck_remove_sysfs().
That last function returns early only when fs_fcheck is NULL. Leaving
the dangling pointer would make it operate on the already-put kobject
and access the freed fcheck in ocfs2_filecheck_sysfs_free().
I may still be missing something, but based on this path it seems the NULL
assignment is still needed. Would you agree, or did I overlook a case where
it is safe to leave it?
Thanks again for taking a look.
Best regards,
Cen Zhang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ocfs2: Publish the filecheck owner before adding sysfs attributes
2026-10-11 15:40 ` Cen Zhang
@ 2026-10-12 5:10 ` Heming Zhao
0 siblings, 0 replies; 6+ messages in thread
From: Heming Zhao @ 2026-10-12 5:10 UTC (permalink / raw)
To: Cen Zhang
Cc: mark, jlbec, joseph.qi, akpm, ghe, ocfs2-devel, linux-kernel,
baijiaju1990, jjzuming
On 10/11/26 23:40, Cen Zhang wrote:
> Hi Heming,
>
> Heming Zhao <heming.zhao@suse.com> 于2026年10月10日周六 15:38写道：
>>
>> setting to NULL is unnecessary, it becomes a harmless dangling pointer here.
>> Other code looks good to me.
>>
>> Thanks,
>> Heming
>
> Thanks for reviewing this.
>
> I checked the failure path again with the help of LLM, and I think clearing
> fs_fcheck is still needed on this baseline. If kobject_init_and_add()
> fails, the
> function puts the kobject and frees fcheck. Its caller in
> ocfs2_fill_super() then
> jumps to out_dismount, which calls ocfs2_dismount_volume() and, in turn,
> ocfs2_filecheck_remove_sysfs().
>
> That last function returns early only when fs_fcheck is NULL. Leaving
> the dangling pointer would make it operate on the already-put kobject
> and access the freed fcheck in ocfs2_filecheck_sysfs_free().
>
> I may still be missing something, but based on this path it seems the NULL
> assignment is still needed. Would you agree, or did I overlook a case where
> it is safe to leave it?
>
> Thanks again for taking a look.
>
> Best regards,
> Cen Zhang
Thanks for the info, I will add my reviewed-by tag.
- Heming
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ocfs2: Publish the filecheck owner before adding sysfs attributes
2026-10-09 10:04 [PATCH] ocfs2: Publish the filecheck owner before adding sysfs attributes Cen Zhang
2026-10-10 7:38 ` Heming Zhao
@ 2026-10-12 5:13 ` Heming Zhao
2026-10-12 6:18 ` Joseph Qi
2 siblings, 0 replies; 6+ messages in thread
From: Heming Zhao @ 2026-10-12 5:13 UTC (permalink / raw)
To: Cen Zhang
Cc: mark, jlbec, joseph.qi, akpm, ghe, ocfs2-devel, linux-kernel,
baijiaju1990, jjzuming
On Fri, Oct 09, 2026 at 06:04:49PM +0800, Cen Zhang wrote:
> ocfs2_filecheck_create_sysfs() initializes the filecheck allocation
> before calling kobject_init_and_add(), but assigns entry->fs_fcheck
> only after that call returns. The default attribute group can already
> be visible while registration is in progress. A concurrent read can
> therefore enter ocfs2_filecheck_attr_show() with a NULL fs_fcheck and
> try to lock its fc_lock.
>
> Assign the initialized owner before adding the kobject, so every
> activated attribute has valid state. Clear the pointer when addition
> fails, preserving the existing teardown guard and error cleanup.
>
> A controlled probe-instrumented test kernel with the original OCFS2
> registration ordering produced this Oops during a concurrent sysfs read:
>
> KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
> RIP: 0010:kasan_byte_accessible+0x15/0x30
> Call Trace:
> <TASK>
> __kasan_check_byte+0x13/0x50
> lock_acquire+0x113/0x300
> _raw_spin_lock+0x31/0x40
> ocfs2_filecheck_attr_show+0x138/0x5f0
> ocfs2_filecheck_show+0x5d/0x90
> sysfs_kf_seq_show+0x1d3/0x360
> seq_read_iter+0x480/0x1110
> kernfs_fop_read_iter+0x419/0x5a0
> vfs_read+0x7d2/0xc20
> ksys_read+0x111/0x200
> do_syscall_64+0x114/0x620
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
> Modules linked in:
>
> Fixes: a860f6eb4c6a ("ocfs2: sysfile interfaces for online file check")
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
LGTM.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
> fs/ocfs2/filecheck.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ocfs2/filecheck.c b/fs/ocfs2/filecheck.c
> index be713d7d24a76b9aa8fcded3d83b3d91d124eb6e..862c1ee7099c943b2e3e57e31f887380c6dc8a88 100644
> --- a/fs/ocfs2/filecheck.c
> +++ b/fs/ocfs2/filecheck.c
> @@ -181,2 +181,4 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
>
> + /* Publish the initialized owner before activating its sysfs callbacks. */
> + entry->fs_fcheck = fcheck;
> entry->fs_kobj.kset = osb->osb_dev_kset;
> @@ -186,2 +188,3 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
> if (ret) {
> + entry->fs_fcheck = NULL;
> kobject_put(&entry->fs_kobj);
> @@ -191,3 +194,2 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
>
> - entry->fs_fcheck = fcheck;
> return 0;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ocfs2: Publish the filecheck owner before adding sysfs attributes
2026-10-09 10:04 [PATCH] ocfs2: Publish the filecheck owner before adding sysfs attributes Cen Zhang
2026-10-10 7:38 ` Heming Zhao
2026-10-12 5:13 ` Heming Zhao
@ 2026-10-12 6:18 ` Joseph Qi
2 siblings, 0 replies; 6+ messages in thread
From: Joseph Qi @ 2026-10-12 6:18 UTC (permalink / raw)
To: Cen Zhang
Cc: mark, jlbec, akpm, ghe, ocfs2-devel, linux-kernel, baijiaju1990,
jjzuming
On 10/9/26 6:04 PM, Cen Zhang wrote:
> ocfs2_filecheck_create_sysfs() initializes the filecheck allocation
> before calling kobject_init_and_add(), but assigns entry->fs_fcheck
> only after that call returns. The default attribute group can already
> be visible while registration is in progress. A concurrent read can
> therefore enter ocfs2_filecheck_attr_show() with a NULL fs_fcheck and
> try to lock its fc_lock.
>
> Assign the initialized owner before adding the kobject, so every
> activated attribute has valid state. Clear the pointer when addition
> fails, preserving the existing teardown guard and error cleanup.
>
> A controlled probe-instrumented test kernel with the original OCFS2
> registration ordering produced this Oops during a concurrent sysfs read:
>
> KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
> RIP: 0010:kasan_byte_accessible+0x15/0x30
> Call Trace:
> <TASK>
> __kasan_check_byte+0x13/0x50
> lock_acquire+0x113/0x300
> _raw_spin_lock+0x31/0x40
> ocfs2_filecheck_attr_show+0x138/0x5f0
> ocfs2_filecheck_show+0x5d/0x90
> sysfs_kf_seq_show+0x1d3/0x360
> seq_read_iter+0x480/0x1110
> kernfs_fop_read_iter+0x419/0x5a0
> vfs_read+0x7d2/0xc20
> ksys_read+0x111/0x200
> do_syscall_64+0x114/0x620
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
> Modules linked in:
>
> Fixes: a860f6eb4c6a ("ocfs2: sysfile interfaces for online file check")
It seems the right Fixes tag is:
5f483c4abb50 ("ocfs2: add kobject for online file check")
Also Cc: <stable@vger.kernel.org> tag is wanted here as well.
The code change looks fine to me. So with the above addressed,
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> fs/ocfs2/filecheck.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ocfs2/filecheck.c b/fs/ocfs2/filecheck.c
> index be713d7d24a76b9aa8fcded3d83b3d91d124eb6e..862c1ee7099c943b2e3e57e31f887380c6dc8a88 100644
> --- a/fs/ocfs2/filecheck.c
> +++ b/fs/ocfs2/filecheck.c
> @@ -181,2 +181,4 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
>
> + /* Publish the initialized owner before activating its sysfs callbacks. */
> + entry->fs_fcheck = fcheck;
> entry->fs_kobj.kset = osb->osb_dev_kset;
> @@ -186,2 +188,3 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
> if (ret) {
> + entry->fs_fcheck = NULL;
> kobject_put(&entry->fs_kobj);
> @@ -191,3 +194,2 @@ int ocfs2_filecheck_create_sysfs(struct ocfs2_super *osb)
>
> - entry->fs_fcheck = fcheck;
> return 0;
^ permalink raw reply [flat|nested] 6+ messages in thread