linux-kernel - Re: [PATCH] kobject: Don't trigger kobject_uevent(KOBJ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190220150753.GB17103@kroah.com>
Date:   Wed, 20 Feb 2019 16:07:53 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        linux-kernel@...r.kernel.org,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Kay Sievers <kay@...y.org>,
        syzbot <syzbot+f648cfb7e0b52bf7ae32@...kaller.appspotmail.com>
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@...kaller.appspotmail.com>
> Analyzed-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> Cc: Kay Sievers <kay@...y.org>
> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.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