Skip to content
Snippets Groups Projects
Commit 1e073f27 authored by Tetsuo Handa's avatar Tetsuo Handa Committed by Ben Hutchings
Browse files

kobject: Don't trigger kobject_uevent(KOBJ_REMOVE) twice.

commit c03a0fd0 upstream.

syzbot is hitting use-after-free bug in uinput module [1]. This is because
kobject_uevent(KOBJ_REMOVE) is called again due to commit 0f4dafc0
("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.

That commit intended to catch cases where nobody even attempted to send
"remove" uevents. But there is no guarantee that an event will ultimately
be sent. We are at the point of no return as far as the rest of the kernel
is concerned; there are no repeats or do-overs.

Also, it is not clear whether some subsystem depends on that commit.
If no subsystem depends on that commit, it will be better to remove
the state_{add,remove}_uevent_sent logic. But we don't want to risk
a regression (in a patch which will be backported) by trying to remove
that logic. Therefore, as a first step, let's avoid the use-after-free bug
by making sure that kobject_uevent(KOBJ_REMOVE) won't be triggered twice.

[1] https://syzkaller.appspot.com/bug?id=8b17c134fe938bbddd75a45afaa9e68af43a362d



Reported-by: default avatarsyzbot <syzbot+f648cfb7e0b52bf7ae32@syzkaller.appspotmail.com>
Analyzed-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
Fixes: 0f4dafc0 ("Kobject: auto-cleanup on final unref")
Cc: Kay Sievers <kay@vrfy.org>
Signed-off-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent ec0c0a0e
No related branches found
No related tags found
No related merge requests found
...@@ -178,6 +178,13 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, ...@@ -178,6 +178,13 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
struct uevent_sock *ue_sk; struct uevent_sock *ue_sk;
#endif #endif
/*
* 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_remove_uevent_sent = 1;
pr_debug("kobject: '%s' (%p): %s\n", pr_debug("kobject: '%s' (%p): %s\n",
kobject_name(kobj), kobj, __func__); kobject_name(kobj), kobj, __func__);
...@@ -275,8 +282,6 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, ...@@ -275,8 +282,6 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
*/ */
if (action == KOBJ_ADD) if (action == KOBJ_ADD)
kobj->state_add_uevent_sent = 1; kobj->state_add_uevent_sent = 1;
else if (action == KOBJ_REMOVE)
kobj->state_remove_uevent_sent = 1;
mutex_lock(&uevent_sock_mutex); mutex_lock(&uevent_sock_mutex);
/* we will send an event, so request a new sequence number */ /* we will send an event, so request a new sequence number */
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment