* How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"?
@ 2022-03-10 10:15 Dongliang Mu
2022-03-10 12:39 ` Gao Xiang
0 siblings, 1 reply; 6+ messages in thread
From: Dongliang Mu @ 2022-03-10 10:15 UTC (permalink / raw
To: xiang, Chao Yu, linux-erofs; +Cc: syzbot+045796dbe294d53147e6
Hi kernel developers,
I am writing to kindly ask for some suggestions on fixing "WARNING:
kobject bug in erofs_unregister_sysfs".
The underlying issue is in the following,
erofs_fc_get_tree
-> get_tree_bdev
-> fill_super
-> erofs_fc_fill_super
When erofs_register_sysfs fails in the calling kobject_init_and_add,
it just returned an error code and the parent function will call
deactivate_locked_super to do clean up.
In the following stack trace, it finally calls erofs_unregister_sysfs
without knowing the execution status of erofs_register_sysfs, which
leads to the kobject bug.
erofs_unregister_sysfs+0x46/0x60 fs/erofs/sysfs.c:225
erofs_put_super+0x37/0xb0 fs/erofs/super.c:771
generic_shutdown_super+0x14c/0x400 fs/super.c:465
kill_block_super+0x97/0xf0 fs/super.c:1397
erofs_kill_sb+0x60/0x190 fs/erofs/super.c:752
deactivate_locked_super+0x94/0x160 fs/super.c:335
get_tree_bdev+0x573/0x760 fs/super.c:1297
I am not sure how to fix this bug. Any suggestion is appreciated.
--
My best regards to you.
No System Is Safe!
Dongliang Mu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"?
2022-03-10 10:15 How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"? Dongliang Mu
@ 2022-03-10 12:39 ` Gao Xiang
2022-03-10 14:04 ` Dongliang Mu
0 siblings, 1 reply; 6+ messages in thread
From: Gao Xiang @ 2022-03-10 12:39 UTC (permalink / raw
To: Dongliang Mu; +Cc: syzbot+045796dbe294d53147e6, linux-erofs
Hi Dongliang,
(+ cc Jianan.)
On Thu, Mar 10, 2022 at 06:15:20PM +0800, Dongliang Mu wrote:
> Hi kernel developers,
>
> I am writing to kindly ask for some suggestions on fixing "WARNING:
> kobject bug in erofs_unregister_sysfs".
>
> The underlying issue is in the following,
>
> erofs_fc_get_tree
> -> get_tree_bdev
> -> fill_super
> -> erofs_fc_fill_super
>
Thanks for the report!
> When erofs_register_sysfs fails in the calling kobject_init_and_add,
> it just returned an error code and the parent function will call
> deactivate_locked_super to do clean up.
Yes, in that way we don't need to rewrite another error path (actually
once we had another duplicated error path but Al suggested the current
shape...)
>
> In the following stack trace, it finally calls erofs_unregister_sysfs
> without knowing the execution status of erofs_register_sysfs, which
> leads to the kobject bug.
>
> erofs_unregister_sysfs+0x46/0x60 fs/erofs/sysfs.c:225
> erofs_put_super+0x37/0xb0 fs/erofs/super.c:771
> generic_shutdown_super+0x14c/0x400 fs/super.c:465
> kill_block_super+0x97/0xf0 fs/super.c:1397
> erofs_kill_sb+0x60/0x190 fs/erofs/super.c:752
> deactivate_locked_super+0x94/0x160 fs/super.c:335
> get_tree_bdev+0x573/0x760 fs/super.c:1297
>
> I am not sure how to fix this bug. Any suggestion is appreciated.
I think a simple way is to introduce a `sysfs_inited' boolean to
sbi to indicate that. Or some better suggestion is welcomed.
Thanks,
Gao Xiang
>
> --
> My best regards to you.
>
> No System Is Safe!
> Dongliang Mu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"?
2022-03-10 12:39 ` Gao Xiang
@ 2022-03-10 14:04 ` Dongliang Mu
2022-03-11 4:08 ` Dongliang Mu
0 siblings, 1 reply; 6+ messages in thread
From: Dongliang Mu @ 2022-03-10 14:04 UTC (permalink / raw
To: Gao Xiang; +Cc: syzbot+045796dbe294d53147e6, linux-erofs
On Thu, Mar 10, 2022 at 8:39 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> Hi Dongliang,
>
> (+ cc Jianan.)
>
> On Thu, Mar 10, 2022 at 06:15:20PM +0800, Dongliang Mu wrote:
> > Hi kernel developers,
> >
> > I am writing to kindly ask for some suggestions on fixing "WARNING:
> > kobject bug in erofs_unregister_sysfs".
> >
> > The underlying issue is in the following,
> >
> > erofs_fc_get_tree
> > -> get_tree_bdev
> > -> fill_super
> > -> erofs_fc_fill_super
> >
>
> Thanks for the report!
>
> > When erofs_register_sysfs fails in the calling kobject_init_and_add,
> > it just returned an error code and the parent function will call
> > deactivate_locked_super to do clean up.
>
> Yes, in that way we don't need to rewrite another error path (actually
> once we had another duplicated error path but Al suggested the current
> shape...)
>
> >
> > In the following stack trace, it finally calls erofs_unregister_sysfs
> > without knowing the execution status of erofs_register_sysfs, which
> > leads to the kobject bug.
> >
> > erofs_unregister_sysfs+0x46/0x60 fs/erofs/sysfs.c:225
> > erofs_put_super+0x37/0xb0 fs/erofs/super.c:771
> > generic_shutdown_super+0x14c/0x400 fs/super.c:465
> > kill_block_super+0x97/0xf0 fs/super.c:1397
> > erofs_kill_sb+0x60/0x190 fs/erofs/super.c:752
> > deactivate_locked_super+0x94/0x160 fs/super.c:335
> > get_tree_bdev+0x573/0x760 fs/super.c:1297
> >
> > I am not sure how to fix this bug. Any suggestion is appreciated.
>
> I think a simple way is to introduce a `sysfs_inited' boolean to
> sbi to indicate that. Or some better suggestion is welcomed.
Yes, it's an idea to use a boolean to indicate the execution status.
Let me first try to craft a drafted patch and test it on Syzbot.
>
> Thanks,
> Gao Xiang
>
> >
> > --
> > My best regards to you.
> >
> > No System Is Safe!
> > Dongliang Mu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"?
2022-03-10 14:04 ` Dongliang Mu
@ 2022-03-11 4:08 ` Dongliang Mu
2022-03-11 4:56 ` Gao Xiang
0 siblings, 1 reply; 6+ messages in thread
From: Dongliang Mu @ 2022-03-11 4:08 UTC (permalink / raw
To: Gao Xiang; +Cc: syzbot+045796dbe294d53147e6, linux-erofs
On Thu, Mar 10, 2022 at 10:04 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> On Thu, Mar 10, 2022 at 8:39 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> >
> > Hi Dongliang,
> >
> > (+ cc Jianan.)
> >
> > On Thu, Mar 10, 2022 at 06:15:20PM +0800, Dongliang Mu wrote:
> > > Hi kernel developers,
> > >
> > > I am writing to kindly ask for some suggestions on fixing "WARNING:
> > > kobject bug in erofs_unregister_sysfs".
> > >
> > > The underlying issue is in the following,
> > >
> > > erofs_fc_get_tree
> > > -> get_tree_bdev
> > > -> fill_super
> > > -> erofs_fc_fill_super
> > >
> >
> > Thanks for the report!
> >
> > > When erofs_register_sysfs fails in the calling kobject_init_and_add,
> > > it just returned an error code and the parent function will call
> > > deactivate_locked_super to do clean up.
> >
> > Yes, in that way we don't need to rewrite another error path (actually
> > once we had another duplicated error path but Al suggested the current
> > shape...)
> >
> > >
> > > In the following stack trace, it finally calls erofs_unregister_sysfs
> > > without knowing the execution status of erofs_register_sysfs, which
> > > leads to the kobject bug.
> > >
> > > erofs_unregister_sysfs+0x46/0x60 fs/erofs/sysfs.c:225
> > > erofs_put_super+0x37/0xb0 fs/erofs/super.c:771
> > > generic_shutdown_super+0x14c/0x400 fs/super.c:465
> > > kill_block_super+0x97/0xf0 fs/super.c:1397
> > > erofs_kill_sb+0x60/0x190 fs/erofs/super.c:752
> > > deactivate_locked_super+0x94/0x160 fs/super.c:335
> > > get_tree_bdev+0x573/0x760 fs/super.c:1297
> > >
> > > I am not sure how to fix this bug. Any suggestion is appreciated.
> >
> > I think a simple way is to introduce a `sysfs_inited' boolean to
> > sbi to indicate that. Or some better suggestion is welcomed.
>
> Yes, it's an idea to use a boolean to indicate the execution status.
> Let me first try to craft a drafted patch and test it on Syzbot.
I've drafted one patch and sent it here.
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 5aa2cf2c2f80..ba2db2e9e3b7 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -144,6 +144,7 @@ struct erofs_sb_info {
u32 feature_incompat;
/* sysfs support */
+ bool sysfs_inited;
struct kobject s_kobj; /* /sys/fs/erofs/<devname> */
struct completion s_kobj_unregister;
};
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
index dac252bc9228..795273c15c42 100644
--- a/fs/erofs/sysfs.c
+++ b/fs/erofs/sysfs.c
@@ -209,6 +209,7 @@ int erofs_register_sysfs(struct super_block *sb)
"%s", sb->s_id);
if (err)
goto put_sb_kobj;
+ sbi->sysfs_inited = true;
return 0;
put_sb_kobj:
@@ -221,8 +222,11 @@ void erofs_unregister_sysfs(struct super_block *sb)
{
struct erofs_sb_info *sbi = EROFS_SB(sb);
- kobject_del(&sbi->s_kobj);
- kobject_put(&sbi->s_kobj);
+ if (sbi->sysfs_inited) {
+ kobject_del(&sbi->s_kobj);
+ kobject_put(&sbi->s_kobj);
+ sbi->sysfs_inited = false;
+ }
wait_for_completion(&sbi->s_kobj_unregister);
}
The compilation is fine. However, since the crash report does not have
any reproducer, I cannot test it in Syzbot or my local instance.
>
> >
> > Thanks,
> > Gao Xiang
> >
> > >
> > > --
> > > My best regards to you.
> > >
> > > No System Is Safe!
> > > Dongliang Mu
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"?
2022-03-11 4:08 ` Dongliang Mu
@ 2022-03-11 4:56 ` Gao Xiang
2022-03-11 7:49 ` Dongliang Mu
0 siblings, 1 reply; 6+ messages in thread
From: Gao Xiang @ 2022-03-11 4:56 UTC (permalink / raw
To: Dongliang Mu; +Cc: syzbot+045796dbe294d53147e6, linux-erofs
On Fri, Mar 11, 2022 at 12:08:43PM +0800, Dongliang Mu wrote:
> On Thu, Mar 10, 2022 at 10:04 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >
> > On Thu, Mar 10, 2022 at 8:39 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> > >
> > > Hi Dongliang,
> > >
> > > (+ cc Jianan.)
> > >
> > > On Thu, Mar 10, 2022 at 06:15:20PM +0800, Dongliang Mu wrote:
> > > > Hi kernel developers,
> > > >
> > > > I am writing to kindly ask for some suggestions on fixing "WARNING:
> > > > kobject bug in erofs_unregister_sysfs".
> > > >
> > > > The underlying issue is in the following,
> > > >
> > > > erofs_fc_get_tree
> > > > -> get_tree_bdev
> > > > -> fill_super
> > > > -> erofs_fc_fill_super
> > > >
> > >
> > > Thanks for the report!
> > >
> > > > When erofs_register_sysfs fails in the calling kobject_init_and_add,
> > > > it just returned an error code and the parent function will call
> > > > deactivate_locked_super to do clean up.
> > >
> > > Yes, in that way we don't need to rewrite another error path (actually
> > > once we had another duplicated error path but Al suggested the current
> > > shape...)
> > >
> > > >
> > > > In the following stack trace, it finally calls erofs_unregister_sysfs
> > > > without knowing the execution status of erofs_register_sysfs, which
> > > > leads to the kobject bug.
> > > >
> > > > erofs_unregister_sysfs+0x46/0x60 fs/erofs/sysfs.c:225
> > > > erofs_put_super+0x37/0xb0 fs/erofs/super.c:771
> > > > generic_shutdown_super+0x14c/0x400 fs/super.c:465
> > > > kill_block_super+0x97/0xf0 fs/super.c:1397
> > > > erofs_kill_sb+0x60/0x190 fs/erofs/super.c:752
> > > > deactivate_locked_super+0x94/0x160 fs/super.c:335
> > > > get_tree_bdev+0x573/0x760 fs/super.c:1297
> > > >
> > > > I am not sure how to fix this bug. Any suggestion is appreciated.
> > >
> > > I think a simple way is to introduce a `sysfs_inited' boolean to
> > > sbi to indicate that. Or some better suggestion is welcomed.
> >
> > Yes, it's an idea to use a boolean to indicate the execution status.
> > Let me first try to craft a drafted patch and test it on Syzbot.
>
> I've drafted one patch and sent it here.
>
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 5aa2cf2c2f80..ba2db2e9e3b7 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -144,6 +144,7 @@ struct erofs_sb_info {
> u32 feature_incompat;
>
> /* sysfs support */
> + bool sysfs_inited;
bool s_sysfs_inited; ?
> struct kobject s_kobj; /* /sys/fs/erofs/<devname> */
> struct completion s_kobj_unregister;
> };
> diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
> index dac252bc9228..795273c15c42 100644
> --- a/fs/erofs/sysfs.c
> +++ b/fs/erofs/sysfs.c
> @@ -209,6 +209,7 @@ int erofs_register_sysfs(struct super_block *sb)
> "%s", sb->s_id);
> if (err)
> goto put_sb_kobj;
> + sbi->sysfs_inited = true;
> return 0;
>
> put_sb_kobj:
> @@ -221,8 +222,11 @@ void erofs_unregister_sysfs(struct super_block *sb)
> {
> struct erofs_sb_info *sbi = EROFS_SB(sb);
>
> - kobject_del(&sbi->s_kobj);
> - kobject_put(&sbi->s_kobj);
> + if (sbi->sysfs_inited) {
> + kobject_del(&sbi->s_kobj);
> + kobject_put(&sbi->s_kobj);
> + sbi->sysfs_inited = false;
> + }
> wait_for_completion(&sbi->s_kobj_unregister);
> }
>
> The compilation is fine. However, since the crash report does not have
> any reproducer, I cannot test it in Syzbot or my local instance.
>
Thanks for the patch. I think you could do some fault injection (e.g.
return err forcely in erofs_register_sysfs() ) to verify that.
If possible, would you mind submitting a formal patch then?
Thanks,
Gao Xiang
> >
> > >
> > > Thanks,
> > > Gao Xiang
> > >
> > > >
> > > > --
> > > > My best regards to you.
> > > >
> > > > No System Is Safe!
> > > > Dongliang Mu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"?
2022-03-11 4:56 ` Gao Xiang
@ 2022-03-11 7:49 ` Dongliang Mu
0 siblings, 0 replies; 6+ messages in thread
From: Dongliang Mu @ 2022-03-11 7:49 UTC (permalink / raw
To: Gao Xiang; +Cc: syzbot+045796dbe294d53147e6, linux-erofs
On Fri, Mar 11, 2022 at 12:56 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> On Fri, Mar 11, 2022 at 12:08:43PM +0800, Dongliang Mu wrote:
> > On Thu, Mar 10, 2022 at 10:04 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > >
> > > On Thu, Mar 10, 2022 at 8:39 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> > > >
> > > > Hi Dongliang,
> > > >
> > > > (+ cc Jianan.)
> > > >
> > > > On Thu, Mar 10, 2022 at 06:15:20PM +0800, Dongliang Mu wrote:
> > > > > Hi kernel developers,
> > > > >
> > > > > I am writing to kindly ask for some suggestions on fixing "WARNING:
> > > > > kobject bug in erofs_unregister_sysfs".
> > > > >
> > > > > The underlying issue is in the following,
> > > > >
> > > > > erofs_fc_get_tree
> > > > > -> get_tree_bdev
> > > > > -> fill_super
> > > > > -> erofs_fc_fill_super
> > > > >
> > > >
> > > > Thanks for the report!
> > > >
> > > > > When erofs_register_sysfs fails in the calling kobject_init_and_add,
> > > > > it just returned an error code and the parent function will call
> > > > > deactivate_locked_super to do clean up.
> > > >
> > > > Yes, in that way we don't need to rewrite another error path (actually
> > > > once we had another duplicated error path but Al suggested the current
> > > > shape...)
> > > >
> > > > >
> > > > > In the following stack trace, it finally calls erofs_unregister_sysfs
> > > > > without knowing the execution status of erofs_register_sysfs, which
> > > > > leads to the kobject bug.
> > > > >
> > > > > erofs_unregister_sysfs+0x46/0x60 fs/erofs/sysfs.c:225
> > > > > erofs_put_super+0x37/0xb0 fs/erofs/super.c:771
> > > > > generic_shutdown_super+0x14c/0x400 fs/super.c:465
> > > > > kill_block_super+0x97/0xf0 fs/super.c:1397
> > > > > erofs_kill_sb+0x60/0x190 fs/erofs/super.c:752
> > > > > deactivate_locked_super+0x94/0x160 fs/super.c:335
> > > > > get_tree_bdev+0x573/0x760 fs/super.c:1297
> > > > >
> > > > > I am not sure how to fix this bug. Any suggestion is appreciated.
> > > >
> > > > I think a simple way is to introduce a `sysfs_inited' boolean to
> > > > sbi to indicate that. Or some better suggestion is welcomed.
> > >
> > > Yes, it's an idea to use a boolean to indicate the execution status.
> > > Let me first try to craft a drafted patch and test it on Syzbot.
> >
> > I've drafted one patch and sent it here.
> >
> > diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> > index 5aa2cf2c2f80..ba2db2e9e3b7 100644
> > --- a/fs/erofs/internal.h
> > +++ b/fs/erofs/internal.h
> > @@ -144,6 +144,7 @@ struct erofs_sb_info {
> > u32 feature_incompat;
> >
> > /* sysfs support */
> > + bool sysfs_inited;
>
> bool s_sysfs_inited; ?
I agree with this name.
>
> > struct kobject s_kobj; /* /sys/fs/erofs/<devname> */
> > struct completion s_kobj_unregister;
> > };
> > diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
> > index dac252bc9228..795273c15c42 100644
> > --- a/fs/erofs/sysfs.c
> > +++ b/fs/erofs/sysfs.c
> > @@ -209,6 +209,7 @@ int erofs_register_sysfs(struct super_block *sb)
> > "%s", sb->s_id);
> > if (err)
> > goto put_sb_kobj;
> > + sbi->sysfs_inited = true;
> > return 0;
> >
> > put_sb_kobj:
> > @@ -221,8 +222,11 @@ void erofs_unregister_sysfs(struct super_block *sb)
> > {
> > struct erofs_sb_info *sbi = EROFS_SB(sb);
> >
> > - kobject_del(&sbi->s_kobj);
> > - kobject_put(&sbi->s_kobj);
> > + if (sbi->sysfs_inited) {
> > + kobject_del(&sbi->s_kobj);
> > + kobject_put(&sbi->s_kobj);
> > + sbi->sysfs_inited = false;
> > + }
> > wait_for_completion(&sbi->s_kobj_unregister);
> > }
> >
> > The compilation is fine. However, since the crash report does not have
> > any reproducer, I cannot test it in Syzbot or my local instance.
> >
>
> Thanks for the patch. I think you could do some fault injection (e.g.
> return err forcely in erofs_register_sysfs() ) to verify that.
Let me try this since I already observed the fault injection in the log file.
>
> If possible, would you mind submitting a formal patch then?
>
> Thanks,
> Gao Xiang
>
>
> > >
> > > >
> > > > Thanks,
> > > > Gao Xiang
> > > >
> > > > >
> > > > > --
> > > > > My best regards to you.
> > > > >
> > > > > No System Is Safe!
> > > > > Dongliang Mu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-11 7:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-10 10:15 How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"? Dongliang Mu
2022-03-10 12:39 ` Gao Xiang
2022-03-10 14:04 ` Dongliang Mu
2022-03-11 4:08 ` Dongliang Mu
2022-03-11 4:56 ` Gao Xiang
2022-03-11 7:49 ` Dongliang Mu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.