| 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
| ||
|
Date: Sat, 11 Feb 2023 16:41:11 -0500
From: Alan Stern <stern@...land.harvard.edu>
To: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc: syzkaller <syzkaller@...glegroups.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Boqun Feng <boqun.feng@...il.com>,
LKML <linux-kernel@...r.kernel.org>,
USB list <linux-usb@...r.kernel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Hillf Danton <hdanton@...a.com>
Subject: [PATCH RFC] drivers/core: Replace lockdep_set_novalidate_class()
with unique class keys
Lockdep is blind to the dev->mutex field of struct device, owing to
the fact that these mutexes are assigned to lockdep's "novalidate"
class. Commit 1704f47b50b5 ("lockdep: Add novalidate class for
dev->mutex conversion") did this because the hierarchical nature of
the device tree makes it impossible in practice to determine whether
acquiring one of these mutexes is safe or might lead to a deadlock.
Unfortunately, this means that lockdep is unable to help diagnose real
deadlocks involving these mutexes when they occur in testing [1] [2]
or in actual use, or to detect bad locking patterns that might lead to
a deadlock. We would like to obtain as much of lockdep's benefits as
possible without generating a flood of false positives -- which is
what happens if one naively removes these mutexes from the
"novalidate" class.
Accordingly, as a middle ground the mutex in each non-static struct
device will be placed in its own unique locking class. This approach
gives up some of lockdep's advantages (for example, all devices having
a particular bus_type or device_type might reasonably be put into the
same locking class), but it should at least allow us to gain the
benefit of some of lockdep's capabilities.
Link: https://syzkaller.appspot.com/bug?extid=2d6ac90723742279e101 [1]
Link: https://syzkaller.appspot.com/bug?extid=2e39bc6569d281acbcfb [2]
Link: https://lore.kernel.org/all/28a82f50-39d5-a45f-7c7a-57a66cec0741@I-love.SAKURA.ne.jp/
Suggested-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Signed-off-by: Alan Stern <stern@...land.harvard.edu>
CC: Peter Zijlstra <peterz@...radead.org>
CC: Ingo Molnar <mingo@...hat.com>
CC: Boqun Feng <boqun.feng@...il.com>
---
I decided to take your suggestion about introducing a new
lockdep_static_obj() function, to reduce the size of the patch. It
can always be combined with the original static_obj() function later
on, if that's what the lockdep developers want.
If Hillf Danton contributed any of the code for this patch, I haven't
seen it in any messages sent to me or in the mailing list archives.
That's why I didn't include a Co-developed-by: tag for him.
drivers/base/core.c | 8 +++++++-
include/linux/device.h | 1 +
include/linux/lockdep.h | 6 ++++++
kernel/locking/lockdep.c | 5 +++++
4 files changed, 19 insertions(+), 1 deletion(-)
Index: usb-devel/drivers/base/core.c
===================================================================
--- usb-devel.orig/drivers/base/core.c
+++ usb-devel/drivers/base/core.c
@@ -2322,6 +2322,9 @@ static void device_release(struct kobjec
devres_release_all(dev);
kfree(dev->dma_range_map);
+ mutex_destroy(&dev->mutex);
+ if (!lockdep_static_obj(dev))
+ lockdep_unregister_key(&dev->mutex_key);
if (dev->release)
dev->release(dev);
@@ -2941,7 +2944,10 @@ void device_initialize(struct device *de
kobject_init(&dev->kobj, &device_ktype);
INIT_LIST_HEAD(&dev->dma_pools);
mutex_init(&dev->mutex);
- lockdep_set_novalidate_class(&dev->mutex);
+ if (!lockdep_static_obj(dev)) {
+ lockdep_register_key(&dev->mutex_key);
+ lockdep_set_class(&dev->mutex, &dev->mutex_key);
+ }
spin_lock_init(&dev->devres_lock);
INIT_LIST_HEAD(&dev->devres_head);
device_pm_init(dev);
Index: usb-devel/include/linux/device.h
===================================================================
--- usb-devel.orig/include/linux/device.h
+++ usb-devel/include/linux/device.h
@@ -570,6 +570,7 @@ struct device {
struct mutex mutex; /* mutex to synchronize calls to
* its driver.
*/
+ struct lock_class_key mutex_key; /* Unique key for each device */
struct dev_links_info links;
struct dev_pm_info power;
Index: usb-devel/include/linux/lockdep.h
===================================================================
--- usb-devel.orig/include/linux/lockdep.h
+++ usb-devel/include/linux/lockdep.h
@@ -172,6 +172,7 @@ do { \
current->lockdep_recursion -= LOCKDEP_OFF; \
} while (0)
+extern int lockdep_static_obj(const void *obj);
extern void lockdep_register_key(struct lock_class_key *key);
extern void lockdep_unregister_key(struct lock_class_key *key);
@@ -391,6 +392,11 @@ static inline void lockdep_set_selftest_
# define lockdep_free_key_range(start, size) do { } while (0)
# define lockdep_sys_exit() do { } while (0)
+static inline int lockdep_static_obj(const void *obj)
+{
+ return 0;
+}
+
static inline void lockdep_register_key(struct lock_class_key *key)
{
}
Index: usb-devel/kernel/locking/lockdep.c
===================================================================
--- usb-devel.orig/kernel/locking/lockdep.c
+++ usb-devel/kernel/locking/lockdep.c
@@ -857,6 +857,11 @@ static int static_obj(const void *obj)
*/
return is_module_address(addr) || is_module_percpu_address(addr);
}
+
+int lockdep_static_obj(const void *obj)
+{
+ return static_obj(obj);
+}
#endif
/*
Powered by blists - more mailing lists