linux-kernel - Re: [PATCH] driver core: Fix userspace expectations of uevent

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] [day] [month] [year] [list]

Message-ID: <66fdc04c8bdb0_964f229476@dwillia2-xfh.jf.intel.com.notmuch>
Date: Wed, 2 Oct 2024 14:51:08 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Greg KH <gregkh@...uxfoundation.org>, Dan Williams
	<dan.j.williams@...el.com>
CC: <rafael.j.wysocki@...el.com>, <tj@...nel.org>,
	<linux-kernel@...r.kernel.org>, <regressions@...ts.linux.dev>
Subject: Re: [PATCH] driver core: Fix userspace expectations of uevent_show()
 as a probe barrier

Greg KH wrote:
> On Mon, Sep 30, 2024 at 01:05:13PM -0700, Dan Williams wrote:
> > Commit "driver core: Fix uevent_show() vs driver detach race" [1]
> > attempted to fix a lockdep report in uevent_show() by making the lookup
> > of driver information for a given device lockless. It turns out that
> > userspace likely depends on the side-effect of uevent_show() flushing
> > device probing, as evidenced by the removal of the lock causing a
> > regression initializing USB devices [2].
> > 
> > Introduce a new "locked" type for sysfs attributes that arranges for the
> > attribute to be called under the device-lock, but without setting up a
> > reverse locking order dependency with the kernfs_get_active() lock.
> 
> As this is "only" for the driver core, can you move some of the stuff in
> the global .h files to the local ones so that no one else gets the wrong
> idea they can use them?

Makes sense. Moving SYSFS_LOCKED and related bits to fs/sysfs/sysfs.h
is straightforward. It did result in the below hunk inside
drivers/base/base.h, but let me know on the v2 posting if that's
irksome:

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0b53593372d7..0405e7667184 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -246,3 +246,12 @@ static inline int devtmpfs_delete_node(struct device *dev) { return 0; }
 
 void software_node_notify(struct device *dev);
 void software_node_notify_remove(struct device *dev);
+
+/*
+ * "locked" device attributes are an internal exception for the @uevent
+ * attribute.
+ */
+#include "../../fs/sysfs/sysfs.h"
+
+#define DEVICE_ATTR_LOCKED_RW(_name) \
+       struct device_attribute dev_attr_##_name = __ATTR_LOCKED_RW(_name)