[PATCH RFC 07/24] cleanup: Basic compatibility with capability analysis
From: Marco Elver
Date: Thu Feb 06 2025 - 13:24:47 EST
Due to the scoped cleanup helpers used for lock guards wrapping
acquire/release around their own constructors/destructors that store
pointers to the passed locks in a separate struct, we currently cannot
accurately annotate *destructors* which lock was released. While it's
possible to annotate the constructor to say which lock was acquired,
that alone would result in false positives claiming the lock was not
released on function return.
Instead, to avoid false positives, we can claim that the constructor
"asserts" that the taken lock is held. This will ensure we can still
benefit from the analysis where scoped guards are used to protect access
to guarded variables, while avoiding false positives. The only downside
are false negatives where we might accidentally lock the same lock
again:
raw_spin_lock(&my_lock);
...
guard(raw_spinlock)(&my_lock); // no warning
Arguably, lockdep will immediately catch issues like this.
While Clang's analysis supports scoped guards in C++ [1], there's no way
to apply this to C right now. Better support for Linux's scoped guard
design could be added in future if deemed critical.
[1] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#scoped-capability
Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
---
include/linux/cleanup.h | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index ec00e3f7af2b..93a166549add 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -223,7 +223,7 @@ const volatile void * __must_check_fn(const volatile void *val)
* @exit is an expression using '_T' -- similar to FREE above.
* @init is an expression in @init_args resulting in @type
*
- * EXTEND_CLASS(name, ext, init, init_args...):
+ * EXTEND_CLASS(name, ext, ctor_attrs, init, init_args...):
* extends class @name to @name@ext with the new constructor
*
* CLASS(name, var)(args...):
@@ -243,15 +243,18 @@ const volatile void * __must_check_fn(const volatile void *val)
#define DEFINE_CLASS(_name, _type, _exit, _init, _init_args...) \
typedef _type class_##_name##_t; \
static inline void class_##_name##_destructor(_type *p) \
+ __no_capability_analysis \
{ _type _T = *p; _exit; } \
static inline _type class_##_name##_constructor(_init_args) \
+ __no_capability_analysis \
{ _type t = _init; return t; }
-#define EXTEND_CLASS(_name, ext, _init, _init_args...) \
+#define EXTEND_CLASS(_name, ext, ctor_attrs, _init, _init_args...) \
typedef class_##_name##_t class_##_name##ext##_t; \
static inline void class_##_name##ext##_destructor(class_##_name##_t *p)\
{ class_##_name##_destructor(p); } \
static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
+ __no_capability_analysis ctor_attrs \
{ class_##_name##_t t = _init; return t; }
#define CLASS(_name, var) \
@@ -299,7 +302,7 @@ static __maybe_unused const bool class_##_name##_is_conditional = _is_cond
#define DEFINE_GUARD_COND(_name, _ext, _condlock) \
__DEFINE_CLASS_IS_CONDITIONAL(_name##_ext, true); \
- EXTEND_CLASS(_name, _ext, \
+ EXTEND_CLASS(_name, _ext,, \
({ void *_t = _T; if (_T && !(_condlock)) _t = NULL; _t; }), \
class_##_name##_t _T) \
static inline void * class_##_name##_ext##_lock_ptr(class_##_name##_t *_T) \
@@ -371,6 +374,7 @@ typedef struct { \
} class_##_name##_t; \
\
static inline void class_##_name##_destructor(class_##_name##_t *_T) \
+ __no_capability_analysis \
{ \
if (_T->lock) { _unlock; } \
} \
@@ -383,6 +387,7 @@ static inline void *class_##_name##_lock_ptr(class_##_name##_t *_T) \
#define __DEFINE_LOCK_GUARD_1(_name, _type, _lock) \
static inline class_##_name##_t class_##_name##_constructor(_type *l) \
+ __no_capability_analysis __asserts_cap(l) \
{ \
class_##_name##_t _t = { .lock = l }, *_T = &_t; \
_lock; \
@@ -391,6 +396,7 @@ static inline class_##_name##_t class_##_name##_constructor(_type *l) \
#define __DEFINE_LOCK_GUARD_0(_name, _lock) \
static inline class_##_name##_t class_##_name##_constructor(void) \
+ __no_capability_analysis \
{ \
class_##_name##_t _t = { .lock = (void*)1 }, \
*_T __maybe_unused = &_t; \
@@ -410,7 +416,7 @@ __DEFINE_LOCK_GUARD_0(_name, _lock)
#define DEFINE_LOCK_GUARD_1_COND(_name, _ext, _condlock) \
__DEFINE_CLASS_IS_CONDITIONAL(_name##_ext, true); \
- EXTEND_CLASS(_name, _ext, \
+ EXTEND_CLASS(_name, _ext, __asserts_cap(l), \
({ class_##_name##_t _t = { .lock = l }, *_T = &_t;\
if (_T->lock && !(_condlock)) _T->lock = NULL; \
_t; }), \
--
2.48.1.502.g6dc24dfdaf-goog