| 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
| ||
|
Message-ID: <20250904225010.1804783-1-dan.j.williams@intel.com>
Date: Thu, 4 Sep 2025 15:50:10 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: <peterz@...radead.org>
CC: <linux-kernel@...r.kernel.org>, <linux-cxl@...r.kernel.org>, "Nathan
Chancellor" <nathan@...nel.org>, Linus Torvalds
<torvalds@...ux-foundation.org>, Dave Jiang <dave.jiang@...el.com>, "David
Lechner" <dlechner@...libre.com>, Jonathan Cameron
<jonathan.cameron@...wei.com>, Andy Shevchenko <andriy.shevchenko@...el.com>
Subject: [PATCH v2] cleanup: Fix "unused function" warnings with conditional guards
While the warning could simply be moved to W=2 [1], there is some small
value, and not much cost to fixing it.
The issue, Andy reports that the "lock_timer" scheme in
kernel/time/posix-timers.c, with its custom usage of
DEFINE_CLASS_IS_COND_GUARD(), results in:
kernel/time/posix-timers.c:89:1: error: unused function 'class_lock_timer_lock_err' [-Werror,-Wunused-function]
89 | DEFINE_CLASS_IS_COND_GUARD(lock_timer);
...with a clang W=1 build. This warning has some value because it can catch
when a conditional guard is defined, but not evaluated by a conditional
acquisition helper like scoped_cond_guard() or ACQUIRE().
Andy also reports that plain DEFINE_GUARD() also encounters this warning:
drivers/pwm/core.c:54:1: error: unused function 'class_pwmchip_lock_err' [-Werror,-Wunused-function]
54 | DEFINE_GUARD(pwmchip, struct pwm_chip *, pwmchip_lock(_T), pwmchip_unlock(_T))
...which *is* a false positive.
Fix those 2 issues by teaching scoped_cond_guard() to check for error
values, and otherwise teach the DEFINE_GUARD() path to mark the conditional
helpers as __maybe_unused.
The compiler does change the polarity of some tests, but the size of the
binary is identical:
Before:
scoped_timer_get_or_fail(timer_id)
1246: 44 89 ff mov %r15d,%edi
1249: e8 d2 5a 00 00 call 6d20 <class_lock_timer_constructor>
124e: 49 89 c7 mov %rax,%r15
1251: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
1257: 0f 92 c0 setb %al
125a: 4d 85 ff test %r15,%r15
125d: 0f 95 c1 setne %cl
1260: 84 c8 test %cl,%al
1262: 0f 84 85 00 00 00 je 12ed <__ia32_sys_timer_gettime+0x15d>
...
12ed: 48 c7 c3 ea ff ff ff mov $0xffffffffffffffea,%rbx
if (likely((timr)))
12f4: 4d 85 ff test %r15,%r15
12f7: 74 0c je 1305 <__ia32_sys_timer_gettime+0x175>
After:
scoped_timer_get_or_fail(timer_id)
1246: 44 89 ff mov %r15d,%edi
1249: e8 d2 5a 00 00 call 6d20 <class_lock_timer_constructor>
124e: 49 89 c7 mov %rax,%r15
1251: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
1257: 0f 92 c0 setb %al
125a: 4d 85 ff test %r15,%r15
125d: 0f 95 c1 setne %cl
1260: 84 c8 test %cl,%al
1262: 75 21 jne 1285 <__ia32_sys_timer_gettime+0xf5>
1264: 48 c7 c3 ea ff ff ff mov $0xffffffffffffffea,%rbx
126b: 4d 85 ff test %r15,%r15
if (likely((timr)))
126e: 0f 84 94 00 00 00 je 1308 <__ia32_sys_timer_gettime+0x178>
Alternatively just merge the suggestion in [1], and call it a day.
Link: http://lore.kernel.org/20250813152142.GP4067720@noisy.programming.kicks-ass.net [1]
Cc: Nathan Chancellor <nathan@...nel.org>
Cc: "Peter Zijlstra (Intel)" <peterz@...radead.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Dave Jiang <dave.jiang@...el.com>
Cc: David Lechner <dlechner@...libre.com>
Cc: Jonathan Cameron <jonathan.cameron@...wei.com>
Reported-by: Andy Shevchenko <andriy.shevchenko@...el.com>
Closes: http://lore.kernel.org/aIo18KZpmKuR4hVZ@black.igk.intel.com
Tested-by: Andy Shevchenko <andriy.shevchenko@...el.com>
Fixes: 857d18f23ab1 ("cleanup: Introduce ACQUIRE() and ACQUIRE_ERR() for conditional locks")
Signed-off-by: Dan Williams <dan.j.williams@...el.com>
---
v1: http://lore.kernel.org/20250804220955.1453135-1-dan.j.williams@intel.com
include/linux/cleanup.h | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index 2573585b7f06..b3a7f6b2142d 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -354,26 +354,30 @@ static __maybe_unused const bool class_##_name##_is_conditional = _is_cond
_ptr = NULL; \
} \
return _ptr; \
- } \
- static inline int class_##_name##_lock_err(class_##_name##_t *_T) \
- { \
- long _rc = (__force unsigned long)*(_exp); \
- if (!_rc) { \
- _rc = -EBUSY; \
- } \
- if (!IS_ERR_VALUE(_rc)) { \
- _rc = 0; \
- } \
- return _rc; \
+ }
+
+#define __DEFINE_GUARD_LOCK_ERR(_name, _exp, _attr) \
+ static _attr int class_##_name##_lock_err(class_##_name##_t *_T) \
+ { \
+ long _rc = (__force unsigned long)*(_exp); \
+ if (!_rc) { \
+ _rc = -EBUSY; \
+ } \
+ if (!IS_ERR_VALUE(_rc)) { \
+ _rc = 0; \
+ } \
+ return _rc; \
}
#define DEFINE_CLASS_IS_GUARD(_name) \
__DEFINE_CLASS_IS_CONDITIONAL(_name, false); \
- __DEFINE_GUARD_LOCK_PTR(_name, _T)
+ __DEFINE_GUARD_LOCK_PTR(_name, _T) \
+ __DEFINE_GUARD_LOCK_ERR(_name, _T, __maybe_unused)
#define DEFINE_CLASS_IS_COND_GUARD(_name) \
__DEFINE_CLASS_IS_CONDITIONAL(_name, true); \
- __DEFINE_GUARD_LOCK_PTR(_name, _T)
+ __DEFINE_GUARD_LOCK_PTR(_name, _T) \
+ __DEFINE_GUARD_LOCK_ERR(_name, _T, inline)
#define DEFINE_GUARD(_name, _type, _lock, _unlock) \
DEFINE_CLASS(_name, _type, if (!__GUARD_IS_ERR(_T)) { _unlock; }, ({ _lock; _T; }), _type _T); \
@@ -430,7 +434,8 @@ _label: \
#define __scoped_cond_guard(_name, _fail, _label, args...) \
for (CLASS(_name, scope)(args); true; ({ goto _label; })) \
- if (!__guard_ptr(_name)(&scope)) { \
+ if (!__guard_ptr(_name)(&scope) || \
+ __guard_err(_name)(&scope)) { \
BUILD_BUG_ON(!__is_cond_ptr(_name)); \
_fail; \
_label: \
@@ -471,7 +476,8 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T) \
if (!__GUARD_IS_ERR(_T->lock)) { _unlock; } \
} \
\
-__DEFINE_GUARD_LOCK_PTR(_name, &_T->lock)
+__DEFINE_GUARD_LOCK_PTR(_name, &_T->lock) \
+__DEFINE_GUARD_LOCK_ERR(_name, &_T->lock, __maybe_unused)
#define __DEFINE_LOCK_GUARD_1(_name, _type, _lock) \
static inline class_##_name##_t class_##_name##_constructor(_type *l) \
base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
--
2.51.0
Powered by blists - more mailing lists