[PATCH 0/2] Fix return code and improve feature check for synthetic uevents

[PATCH 0/2] Fix return code and improve feature check for synthetic uevents

[Peter Rajnoha]

Two small patches to aid handling of synthetic uevents back in userspace:

  - Return error code back to userspace on /sys/.../uevent file write
    failure so userspace knows and it can act accordingly.

  - Add new 'kernel/uevent_features' sysfs file to make it possible for
    userspace to check that the extended synthetic uevent arguments are
    supported without relying on kernel version check only.

Peter Rajnoha (2):
  kobject: return error code if writing /sys/.../uevent fails
  kobject: add kernel/uevent_features sysfs file

 drivers/base/bus.c  | 12 ++++++++----
 drivers/base/core.c |  8 +++++++-
 kernel/ksysfs.c     |  8 ++++++++
 kernel/module.c     |  6 ++++--
 4 files changed, 27 insertions(+), 7 deletions(-)

-- 
2.19.2

[PATCH 1/2] kobject: return error code if writing /sys/.../uevent fails

[Peter Rajnoha]

Propagate error code back to userspace if writing the /sys/.../uevent
file fails. Before, the write operation always returned with success,
even if we failed to recognize the input string or if we failed to
generate the uevent itself.

With the error codes properly propagated back to userspace, we are
able to react in userspace accordingly by not assuming and awaiting
a uevent that is not delivered.

Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
 drivers/base/bus.c  | 12 ++++++++----
 drivers/base/core.c |  8 +++++++-
 kernel/module.c     |  6 ++++--
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 8bfd27ec73d6..b886b15cb53b 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -611,8 +611,10 @@ static void remove_probe_files(struct bus_type *bus)
 static ssize_t uevent_store(struct device_driver *drv, const char *buf,
 			    size_t count)
 {
-	kobject_synth_uevent(&drv->p->kobj, buf, count);
-	return count;
+	int rc;
+
+	rc = kobject_synth_uevent(&drv->p->kobj, buf, count);
+	return rc ? rc : count;
 }
 static DRIVER_ATTR_WO(uevent);
 
@@ -828,8 +830,10 @@ static void klist_devices_put(struct klist_node *n)
 static ssize_t bus_uevent_store(struct bus_type *bus,
 				const char *buf, size_t count)
 {
-	kobject_synth_uevent(&bus->p->subsys.kobj, buf, count);
-	return count;
+	int rc;
+
+	rc = kobject_synth_uevent(&bus->p->subsys.kobj, buf, count);
+	return rc ? rc : count;
 }
 static BUS_ATTR(uevent, S_IWUSR, NULL, bus_uevent_store);
 
diff --git a/drivers/base/core.c b/drivers/base/core.c
index ed145fbfeddf..92faafd03caf 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1074,8 +1074,14 @@ static ssize_t uevent_show(struct device *dev, struct device_attribute *attr,
 static ssize_t uevent_store(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t count)
 {
-	if (kobject_synth_uevent(&dev->kobj, buf, count))
+	int rc;
+
+	rc = kobject_synth_uevent(&dev->kobj, buf, count);
+
+	if (rc) {
 		dev_err(dev, "uevent: failed to send synthetic uevent\n");
+		return rc;
+	}
 
 	return count;
 }
diff --git a/kernel/module.c b/kernel/module.c
index 49a405891587..0812a7f80fa7 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1207,8 +1207,10 @@ static ssize_t store_uevent(struct module_attribute *mattr,
 			    struct module_kobject *mk,
 			    const char *buffer, size_t count)
 {
-	kobject_synth_uevent(&mk->kobj, buffer, count);
-	return count;
+	int rc;
+
+	rc = kobject_synth_uevent(&mk->kobj, buffer, count);
+	return rc ? rc : count;
 }
 
 struct module_attribute module_uevent =
-- 
2.19.2

[PATCH 2/2] kobject: add kernel/uevent_features sysfs file

[Peter Rajnoha]

We can use extended format when writing /sys/.../uevent files to
generate synthetic uevents, introduced with commit f36776fafbaa
("kobject: support passing in variables for synthetic uevents").

Before using this extended format, we need to know if it's supported
and kernel version check may not be appropriate in all cases - there
are possible differences from upstream kernel in distributions with
backports.

This patch adds /sys/kernel/uevent_features file which currently lists
'synthargs' string to denote that the kernel is able to recognize the
extended synthetic uevent arguments. Userspace can easily check for
the feature then.

Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
 kernel/ksysfs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index 46ba853656f6..d893d7442f61 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -37,6 +37,13 @@ static ssize_t uevent_seqnum_show(struct kobject *kobj,
 }
 KERNEL_ATTR_RO(uevent_seqnum);
 
+static ssize_t uevent_features_show(struct kobject *kobj,
+				    struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "synthargs\n");
+}
+KERNEL_ATTR_RO(uevent_features);
+
 #ifdef CONFIG_UEVENT_HELPER
 /* uevent helper program, used during early boot */
 static ssize_t uevent_helper_show(struct kobject *kobj,
@@ -213,6 +220,7 @@ EXPORT_SYMBOL_GPL(kernel_kobj);
 static struct attribute * kernel_attrs[] = {
 	&fscaps_attr.attr,
 	&uevent_seqnum_attr.attr,
+	&uevent_features_attr.attr,
 #ifdef CONFIG_UEVENT_HELPER
 	&uevent_helper_attr.attr,
 #endif
-- 
2.19.2

Re: [PATCH 0/2] Fix return code and improve feature check for synthetic uevents

[Greg KH]

On Wed, Dec 05, 2018 at 12:27:43PM +0100, Peter Rajnoha wrote:
> Two small patches to aid handling of synthetic uevents back in userspace:
> 
>   - Return error code back to userspace on /sys/.../uevent file write
>     failure so userspace knows and it can act accordingly.
> 
>   - Add new 'kernel/uevent_features' sysfs file to make it possible for
>     userspace to check that the extended synthetic uevent arguments are
>     supported without relying on kernel version check only.
> 
> Peter Rajnoha (2):
>   kobject: return error code if writing /sys/.../uevent fails
>   kobject: add kernel/uevent_features sysfs file

Did you send this series twice?  Which one should I pay attention to?

confused,

greg k-h

Re: [PATCH 0/2] Fix return code and improve feature check for synthetic uevents

[Peter Rajnoha]

Dňa 5. 12. 2018 o 17:30 Greg KH napísal(a):
> On Wed, Dec 05, 2018 at 12:27:43PM +0100, Peter Rajnoha wrote:
>> Two small patches to aid handling of synthetic uevents back in userspace:
>>
>>   - Return error code back to userspace on /sys/.../uevent file write
>>     failure so userspace knows and it can act accordingly.
>>
>>   - Add new 'kernel/uevent_features' sysfs file to make it possible for
>>     userspace to check that the extended synthetic uevent arguments are
>>     supported without relying on kernel version check only.
>>
>> Peter Rajnoha (2):
>>   kobject: return error code if writing /sys/.../uevent fails
>>   kobject: add kernel/uevent_features sysfs file
> 
> Did you send this series twice?  Which one should I pay attention to?
> 
> confused,
> 
> greg k-h
> 

They're same, I just sent it again because I forgot to add the LKML list
with the first one. So please take the latter one with that list on CC.
Sorry for the confusion.

-- 
Peter

Re: [PATCH 2/2] kobject: add kernel/uevent_features sysfs file

[Greg KH]

On Wed, Dec 05, 2018 at 12:27:45PM +0100, Peter Rajnoha wrote:
> We can use extended format when writing /sys/.../uevent files to
> generate synthetic uevents, introduced with commit f36776fafbaa
> ("kobject: support passing in variables for synthetic uevents").
> 
> Before using this extended format, we need to know if it's supported
> and kernel version check may not be appropriate in all cases - there
> are possible differences from upstream kernel in distributions with
> backports.
> 
> This patch adds /sys/kernel/uevent_features file which currently lists
> 'synthargs' string to denote that the kernel is able to recognize the
> extended synthetic uevent arguments. Userspace can easily check for
> the feature then.

No Documentation/ABI/ update for your new sysfs file?  Not good, I can't
take this patch without that :(

greg k-h

Re: [PATCH 1/2] kobject: return error code if writing /sys/.../uevent fails

[Phillip Susi]

Just an FYI, I've been tracking down a bug that was causing the debian
bullseye installer to immediately crash and reboot the xen domU and
bisected it to this commit.  It appears that the Xen Virtual Keyboard
driver ( and at least one other, probably more ) were always broken and
failed to trigger the udev event, but this was never noticed.  When this
patch returned the error code, it caused the d-i init scripts triggering
coldplug events to fail, which caused init to bail out, causing a kernel
panic.

In the future, when fixing error returns like this, could you please add
a big fat printk so that hopefully things that always should have failed
but didn't and now do can get noticed and fixed more quickly.

https://bugzilla.kernel.org/show_bug.cgi?id=207695
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983357

Re: [PATCH 1/2] kobject: return error code if writing /sys/.../uevent fails

[Greg KH]

On Thu, Apr 29, 2021 at 09:40:08AM -0400, Phillip Susi wrote:
> Just an FYI, I've been tracking down a bug that was causing the debian
> bullseye installer to immediately crash and reboot the xen domU and
> bisected it to this commit.  It appears that the Xen Virtual Keyboard
> driver ( and at least one other, probably more ) were always broken and
> failed to trigger the udev event, but this was never noticed.  When this
> patch returned the error code, it caused the d-i init scripts triggering
> coldplug events to fail, which caused init to bail out, causing a kernel
> panic.
> 
> In the future, when fixing error returns like this, could you please add
> a big fat printk so that hopefully things that always should have failed
> but didn't and now do can get noticed and fixed more quickly.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=207695
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983357
> 

Spaming kernel logs with things that userspace can trigger is generally
a bad idea.  But I understand the pain here, sorry to hear it.

Was the offending drivers ever fixed?

thanks,

greg k-h