All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Dongliang Mu <mudongliangabcd@gmail.com>
To: Gao Xiang <hsiangkao@linux.alibaba.com>
Cc: syzbot+045796dbe294d53147e6@syzkaller.appspotmail.com,
	linux-erofs@lists.ozlabs.org
Subject: Re: How to fix the bug in "WARNING: kobject bug in erofs_unregister_sysfs"?
Date: Fri, 11 Mar 2022 12:08:43 +0800	[thread overview]
Message-ID: <CAD-N9QXNb3+RL2jLZBG3M6HoBjjA6rDP8SrXLUBZ1mN9jinifw@mail.gmail.com> (raw)
In-Reply-To: <CAD-N9QXrx6=nMz8=wRXP2a7n8uROsH7cws5-K+pJvNHqfWWcug@mail.gmail.com>

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

  reply	other threads:[~2022-03-11  4:09 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2022-03-11  4:56       ` Gao Xiang
2022-03-11  7:49         ` Dongliang Mu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAD-N9QXNb3+RL2jLZBG3M6HoBjjA6rDP8SrXLUBZ1mN9jinifw@mail.gmail.com \
    --to=mudongliangabcd@gmail.com \
    --cc=hsiangkao@linux.alibaba.com \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=syzbot+045796dbe294d53147e6@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.