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 <[email protected]>
> Analyzed-by: Dmitry Torokhov <[email protected]>
> Cc: Kay Sievers <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>
> ---
> 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
