[1/1] kobject: Don't emit change events if not in sysfs

Message ID 20201019153232.1.I797f9874972a07fc381fe586b6748ce71c7b1fda@changeid
State New
Headers show
Series
  • [1/1] kobject: Don't emit change events if not in sysfs
Related show

Commit Message

Abhishek Pandit-Subedi Oct. 19, 2020, 10:32 p.m.
Add a check to make sure the kobj is created and in sysfs before sending
a change event notification. Otherwise, udev rules that depend on the
change notification may find that the path that changed doesn't actually
exist.

Fixes: a45aca510b73b7 (PM: sleep: core: Emit changed uevent on wakeup_sysfs_add/remove)
Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 lib/kobject_uevent.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Greg Kroah-Hartman Oct. 20, 2020, 5:57 a.m. | #1
On Mon, Oct 19, 2020 at 03:32:57PM -0700, Abhishek Pandit-Subedi wrote:
> Add a check to make sure the kobj is created and in sysfs before sending
> a change event notification. Otherwise, udev rules that depend on the
> change notification may find that the path that changed doesn't actually
> exist.

Why is the user of the kobject trying to emit a uevent before it is
registered?  Shouldn't we fix the root problem here instead?  Otherwise
the event is still "gone", the caller will not know what to do about it.

Please fix the root problem here.

thanks,

greg k-h
Abhishek Pandit-Subedi Oct. 21, 2020, 3:27 a.m. | #2
Hi Greg,

I was debugging without a live repro and I was told this patch
improved behavior but it's only by chance (someone bisected a Dell
D6000 dock's displayport issue to this commit and this change seemed
to help; udev logs later shows that's not the case). I took another
look at device_init_wakeup and I can see that
device_set_wakeup_capable does indeed check for device_is_registered
before adding the wakeup attributes so the ordering of events I
suspected cannot occur.

Thanks for pushing back Greg. It made me take a deeper look at an
assumption I hadn't challenged. Please consider this patch abandoned.

Abhishek

On Mon, Oct 19, 2020 at 10:56 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Oct 19, 2020 at 03:32:57PM -0700, Abhishek Pandit-Subedi wrote:
> > Add a check to make sure the kobj is created and in sysfs before sending
> > a change event notification. Otherwise, udev rules that depend on the
> > change notification may find that the path that changed doesn't actually
> > exist.
>
> Why is the user of the kobject trying to emit a uevent before it is
> registered?  Shouldn't we fix the root problem here instead?  Otherwise
> the event is still "gone", the caller will not know what to do about it.
>
> Please fix the root problem here.
>
> thanks,
>
> greg k-h

Patch

diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index 7998affa45d49a..f08197e907d5ce 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -473,6 +473,11 @@  int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
 	if (action == KOBJ_REMOVE)
 		kobj->state_remove_uevent_sent = 1;
 
+	if (action == KOBJ_CHANGE && !kobj->state_in_sysfs) {
+		pr_debug("kobject: can't emit KOBJ_CHANGE until in sysfs\n");
+		return -EINVAL;
+	}
+
 	pr_debug("kobject: '%s' (%p): %s\n",
 		 kobject_name(kobj), kobj, __func__);