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

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

[Abhishek Pandit-Subedi]

Hi maintainers,

A little while ago, I got a bug report of a regression caused by a patch
I submitted a45aca510b73b7 (PM: sleep: core: Emit changed uevent
on wakeup_sysfs_add/remove)

https://bugzilla.kernel.org/show_bug.cgi?id=209469

It seems possible for a "change" event to be sent before the device is
added to the sysfs (so when the rule runs, it can't access the device
path that emitted it). The bug report had the following log that made me
identify this is possible:
        > Use global config file: /etc/usb_modeswitch.conf
        > Use top device dir /sys/bus/usb/devices/2-3
        > Check class of first interface ...
        >  No access to first interface. Exit

I've added a patch to fix the former problem here and confirmed via
udevadm monitor that no CHANGE requests are seen for devices before they
emit the ADD event.

Thanks
Abhishek



Abhishek Pandit-Subedi (1):
  kobject: Don't emit change events if not in sysfs

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

-- 
2.29.0.rc1.297.gfa9743e501-goog

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

[Abhishek Pandit-Subedi]

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(+)

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__);
 
-- 
2.29.0.rc1.297.gfa9743e501-goog

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

[Greg Kroah-Hartman]

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

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

[Abhishek Pandit-Subedi]

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