Messages in this thread | | | Date | Wed, 20 Feb 2019 16:07:53 +0100 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH] kobject: Don't trigger kobject_uevent(KOBJ_REMOVE) twice. |
| |
On Wed, Feb 20, 2019 at 10:38:34PM +0900, Tetsuo Handa wrote: > syzbot is hitting use-after-free bug in uinput module [1]. This is because > kobject_uevent(KOBJ_REMOVE) is called again due to commit 0f4dafc0563c6c49 > ("Kobject: auto-cleanup on final unref") after memory allocation fault > injection made kobject_uevent(KOBJ_REMOVE) from device_del() from > input_unregister_device() fail, while uinput_destroy_device() is expecting > that kobject_uevent(KOBJ_REMOVE) is not called after device_del() from > input_unregister_device() completed. Fix this problem by marking "remove" > event done regardless of result. > > [1] https://syzkaller.appspot.com/bug?id=8b17c134fe938bbddd75a45afaa9e68af43a362d > > Reported-by: syzbot <syzbot+f648cfb7e0b52bf7ae32@syzkaller.appspotmail.com> > Analyzed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Cc: Kay Sievers <kay@vrfy.org> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > lib/kobject_uevent.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c > index f058026..7ec4165 100644 > --- a/lib/kobject_uevent.c > +++ b/lib/kobject_uevent.c > @@ -466,6 +466,13 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, > int i = 0; > int retval = 0; > > + /* > + * Mark "remove" event done regardless of result, for some subsystems > + * do not want to re-trigger "remove" event via automatic cleanup. > + */ > + if (action == KOBJ_REMOVE && kobj->state_add_uevent_sent) > + kobj->state_remove_uevent_sent = 1; > + > pr_debug("kobject: '%s' (%p): %s\n", > kobject_name(kobj), kobj, __func__);
If you really want to do this, put it below the debugging line.
But I would argue that this is not ok, as the remove uevent did NOT get sent, and you are saying it did.
What memory is being used-after-free here when we fail to properly send a uevent? Shouldn't we fix up that problem correctly?
thanks,
greg k-h
| |