Skip to content

Commit 8aed90a

Browse files
committed
driver core: fix potential NULL pointer dereference in dev_uevent()
jira LE-3587 Rebuild_History Non-Buildable kernel-4.18.0-553.62.1.el8_10 commit-author Dmitry Torokhov <[email protected]> commit 18daa52 Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-4.18.0-553.62.1.el8_10/18daa524.failed If userspace reads "uevent" device attribute at the same time as another threads unbinds the device from its driver, change to dev->driver from a valid pointer to NULL may result in crash. Fix this by using READ_ONCE() when fetching the pointer, and take bus' drivers klist lock to make sure driver instance will not disappear while we access it. Use WRITE_ONCE() when setting the driver pointer to ensure there is no tearing. Signed-off-by: Dmitry Torokhov <[email protected]> Reviewed-by: Masami Hiramatsu (Google) <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]> (cherry picked from commit 18daa52) Signed-off-by: Jonathan Maple <[email protected]> # Conflicts: # drivers/base/base.h # drivers/base/core.c
1 parent fa444de commit 8aed90a

File tree

1 file changed

+156
-0
lines changed

1 file changed

+156
-0
lines changed
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
driver core: fix potential NULL pointer dereference in dev_uevent()
2+
3+
jira LE-3587
4+
Rebuild_History Non-Buildable kernel-4.18.0-553.62.1.el8_10
5+
commit-author Dmitry Torokhov <[email protected]>
6+
commit 18daa52418e7e4629ed1703b64777294209d2622
7+
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
8+
Will be included in final tarball splat. Ref for failed cherry-pick at:
9+
ciq/ciq_backports/kernel-4.18.0-553.62.1.el8_10/18daa524.failed
10+
11+
If userspace reads "uevent" device attribute at the same time as another
12+
threads unbinds the device from its driver, change to dev->driver from a
13+
valid pointer to NULL may result in crash. Fix this by using READ_ONCE()
14+
when fetching the pointer, and take bus' drivers klist lock to make sure
15+
driver instance will not disappear while we access it.
16+
17+
Use WRITE_ONCE() when setting the driver pointer to ensure there is no
18+
tearing.
19+
20+
Signed-off-by: Dmitry Torokhov <[email protected]>
21+
Reviewed-by: Masami Hiramatsu (Google) <[email protected]>
22+
Link: https://lore.kernel.org/r/[email protected]
23+
Signed-off-by: Greg Kroah-Hartman <[email protected]>
24+
(cherry picked from commit 18daa52418e7e4629ed1703b64777294209d2622)
25+
Signed-off-by: Jonathan Maple <[email protected]>
26+
27+
# Conflicts:
28+
# drivers/base/base.h
29+
# drivers/base/core.c
30+
diff --cc drivers/base/base.h
31+
index f1ca94ed4ea3,123031a757d9..000000000000
32+
--- a/drivers/base/base.h
33+
+++ b/drivers/base/base.h
34+
@@@ -60,6 -73,9 +60,12 @@@ static inline void subsys_put(struct su
35+
kset_put(&sp->subsys);
36+
}
37+
38+
++<<<<<<< HEAD
39+
++=======
40+
+ struct subsys_private *bus_to_subsys(const struct bus_type *bus);
41+
+ struct subsys_private *class_to_subsys(const struct class *class);
42+
+
43+
++>>>>>>> 18daa52418e7 (driver core: fix potential NULL pointer dereference in dev_uevent())
44+
struct driver_private {
45+
struct kobject kobj;
46+
struct klist klist_devices;
47+
@@@ -161,19 -177,33 +167,43 @@@ static inline void dev_sync_state(struc
48+
dev->driver->sync_state(dev);
49+
}
50+
51+
-int driver_add_groups(const struct device_driver *drv, const struct attribute_group **groups);
52+
-void driver_remove_groups(const struct device_driver *drv, const struct attribute_group **groups);
53+
+extern int driver_add_groups(struct device_driver *drv,
54+
+ const struct attribute_group **groups);
55+
+extern void driver_remove_groups(struct device_driver *drv,
56+
+ const struct attribute_group **groups);
57+
void device_driver_detach(struct device *dev);
58+
59+
++<<<<<<< HEAD
60+
+extern int devres_release_all(struct device *dev);
61+
+extern void device_block_probing(void);
62+
+extern void device_unblock_probing(void);
63+
++=======
64+
+ static inline void device_set_driver(struct device *dev, const struct device_driver *drv)
65+
+ {
66+
+ /*
67+
+ * Majority (all?) read accesses to dev->driver happens either
68+
+ * while holding device lock or in bus/driver code that is only
69+
+ * invoked when the device is bound to a driver and there is no
70+
+ * concern of the pointer being changed while it is being read.
71+
+ * However when reading device's uevent file we read driver pointer
72+
+ * without taking device lock (so we do not block there for
73+
+ * arbitrary amount of time). We use WRITE_ONCE() here to prevent
74+
+ * tearing so that READ_ONCE() can safely be used in uevent code.
75+
+ */
76+
+ // FIXME - this cast should not be needed "soon"
77+
+ WRITE_ONCE(dev->driver, (struct device_driver *)drv);
78+
+ }
79+
+
80+
+ int devres_release_all(struct device *dev);
81+
+ void device_block_probing(void);
82+
+ void device_unblock_probing(void);
83+
+ void deferred_probe_extend_timeout(void);
84+
+ void driver_deferred_probe_trigger(void);
85+
++>>>>>>> 18daa52418e7 (driver core: fix potential NULL pointer dereference in dev_uevent())
86+
const char *device_get_devnode(const struct device *dev, umode_t *mode,
87+
kuid_t *uid, kgid_t *gid, const char **tmp);
88+
+extern void deferred_probe_extend_timeout(void);
89+
+extern void driver_deferred_probe_trigger(void);
90+
91+
/* /sys/devices directory */
92+
extern struct kset *devices_kset;
93+
diff --cc drivers/base/core.c
94+
index 85168dc6be18,cbc0099d8ef2..000000000000
95+
--- a/drivers/base/core.c
96+
+++ b/drivers/base/core.c
97+
@@@ -2469,10 -2624,38 +2469,43 @@@ static const char *dev_uevent_name(stru
98+
return NULL;
99+
}
100+
101+
++<<<<<<< HEAD
102+
+static int dev_uevent(struct kset *kset, struct kobject *kobj,
103+
+ struct kobj_uevent_env *env)
104+
++=======
105+
+ /*
106+
+ * Try filling "DRIVER=<name>" uevent variable for a device. Because this
107+
+ * function may race with binding and unbinding the device from a driver,
108+
+ * we need to be careful. Binding is generally safe, at worst we miss the
109+
+ * fact that the device is already bound to a driver (but the driver
110+
+ * information that is delivered through uevents is best-effort, it may
111+
+ * become obsolete as soon as it is generated anyways). Unbinding is more
112+
+ * risky as driver pointer is transitioning to NULL, so READ_ONCE() should
113+
+ * be used to make sure we are dealing with the same pointer, and to
114+
+ * ensure that driver structure is not going to disappear from under us
115+
+ * we take bus' drivers klist lock. The assumption that only registered
116+
+ * driver can be bound to a device, and to unregister a driver bus code
117+
+ * will take the same lock.
118+
+ */
119+
+ static void dev_driver_uevent(const struct device *dev, struct kobj_uevent_env *env)
120+
+ {
121+
+ struct subsys_private *sp = bus_to_subsys(dev->bus);
122+
+
123+
+ if (sp) {
124+
+ scoped_guard(spinlock, &sp->klist_drivers.k_lock) {
125+
+ struct device_driver *drv = READ_ONCE(dev->driver);
126+
+ if (drv)
127+
+ add_uevent_var(env, "DRIVER=%s", drv->name);
128+
+ }
129+
+
130+
+ subsys_put(sp);
131+
+ }
132+
+ }
133+
+
134+
+ static int dev_uevent(const struct kobject *kobj, struct kobj_uevent_env *env)
135+
++>>>>>>> 18daa52418e7 (driver core: fix potential NULL pointer dereference in dev_uevent())
136+
{
137+
- const struct device *dev = kobj_to_dev(kobj);
138+
+ struct device *dev = kobj_to_dev(kobj);
139+
int retval = 0;
140+
141+
/* add device node properties if present */
142+
* Unmerged path drivers/base/base.h
143+
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
144+
index cca569e4d734..443c008fdfca 100644
145+
--- a/drivers/base/bus.c
146+
+++ b/drivers/base/bus.c
147+
@@ -56,7 +56,7 @@ static int __must_check bus_rescan_devices_helper(struct device *dev,
148+
* NULL. A call to subsys_put() must be done when finished with the pointer in
149+
* order for it to be properly freed.
150+
*/
151+
-static struct subsys_private *bus_to_subsys(const struct bus_type *bus)
152+
+struct subsys_private *bus_to_subsys(const struct bus_type *bus)
153+
{
154+
struct subsys_private *sp = NULL;
155+
struct kobject *kobj;
156+
* Unmerged path drivers/base/core.c

0 commit comments

Comments
 (0)