Re: [PATCH 01/14] bug/kunit: Core support for suppressing warning backtraces

2024-03-12 Thread Kees Cook
On Tue, Mar 12, 2024 at 10:02:56AM -0700, Guenter Roeck wrote:
> Some unit tests intentionally trigger warning backtraces by passing
> bad parameters to API functions. Such unit tests typically check the
> return value from those calls, not the existence of the warning backtrace.
> 
> Such intentionally generated warning backtraces are neither desirable
> nor useful for a number of reasons.
> - They can result in overlooked real problems.
> - A warning that suddenly starts to show up in unit tests needs to be
>   investigated and has to be marked to be ignored, for example by
>   adjusting filter scripts. Such filters are ad-hoc because there is
>   no real standard format for warnings. On top of that, such filter
>   scripts would require constant maintenance.
> 
> One option to address problem would be to add messages such as "expected
> warning backtraces start / end here" to the kernel log.  However, that
> would again require filter scripts, it might result in missing real
> problematic warning backtraces triggered while the test is running, and
> the irrelevant backtrace(s) would still clog the kernel log.
> 
> Solve the problem by providing a means to identify and suppress specific
> warning backtraces while executing test code.
> 
> Cc: Dan Carpenter 
> Cc: Daniel Diaz 
> Cc: Naresh Kamboju 
> Cc: Kees Cook 
> Signed-off-by: Guenter Roeck 

Yup, this looks fine to me.

Reviewed-by: Kees Cook 

-- 
Kees Cook


[PATCH 01/14] bug/kunit: Core support for suppressing warning backtraces

2024-03-12 Thread Guenter Roeck
Some unit tests intentionally trigger warning backtraces by passing
bad parameters to API functions. Such unit tests typically check the
return value from those calls, not the existence of the warning backtrace.

Such intentionally generated warning backtraces are neither desirable
nor useful for a number of reasons.
- They can result in overlooked real problems.
- A warning that suddenly starts to show up in unit tests needs to be
  investigated and has to be marked to be ignored, for example by
  adjusting filter scripts. Such filters are ad-hoc because there is
  no real standard format for warnings. On top of that, such filter
  scripts would require constant maintenance.

One option to address problem would be to add messages such as "expected
warning backtraces start / end here" to the kernel log.  However, that
would again require filter scripts, it might result in missing real
problematic warning backtraces triggered while the test is running, and
the irrelevant backtrace(s) would still clog the kernel log.

Solve the problem by providing a means to identify and suppress specific
warning backtraces while executing test code.

Cc: Dan Carpenter 
Cc: Daniel Diaz 
Cc: Naresh Kamboju 
Cc: Kees Cook 
Signed-off-by: Guenter Roeck 
---
 include/asm-generic/bug.h | 16 +---
 include/kunit/bug.h   | 51 +++
 include/kunit/test.h  |  1 +
 include/linux/bug.h   | 13 ++
 lib/bug.c | 51 ---
 lib/kunit/Makefile|  6 +++--
 lib/kunit/bug.c   | 40 ++
 7 files changed, 169 insertions(+), 9 deletions(-)
 create mode 100644 include/kunit/bug.h
 create mode 100644 lib/kunit/bug.c

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 6e794420bd39..c170b6477689 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -18,6 +18,7 @@
 #endif
 
 #ifndef __ASSEMBLY__
+#include 
 #include 
 #include 
 
@@ -39,8 +40,14 @@ struct bug_entry {
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 #ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
const char  *file;
+#ifdef HAVE_BUG_FUNCTION
+   const char  *function;
+#endif
 #else
signed int  file_disp;
+#ifdef HAVE_BUG_FUNCTION
+   signed int  function_disp;
+#endif
 #endif
unsigned short  line;
 #endif
@@ -96,15 +103,18 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, 
...);
 #define __WARN()   __WARN_printf(TAINT_WARN, NULL)
 #define __WARN_printf(taint, arg...) do {  \
instrumentation_begin();\
-   warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);  \
+   if (!IS_SUPPRESSED_WARNING(__func__))   \
+   warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);\
instrumentation_end();  \
} while (0)
 #else
 #define __WARN()   __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
 #define __WARN_printf(taint, arg...) do {  \
instrumentation_begin();\
-   __warn_printk(arg); \
-   __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+   if (!IS_SUPPRESSED_WARNING(__func__)) { \
+   __warn_printk(arg); \
+   __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | 
BUGFLAG_TAINT(taint));\
+   }   \
instrumentation_end();  \
} while (0)
 #define WARN_ON_ONCE(condition) ({ \
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
new file mode 100644
index ..1e34da961599
--- /dev/null
+++ b/include/kunit/bug.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit helpers for backtrace suppression
+ *
+ * Copyright (c) 2024 Guenter Roeck 
+ */
+
+#ifndef _KUNIT_BUG_H
+#define _KUNIT_BUG_H
+
+#ifndef __ASSEMBLY__
+
+#include 
+
+#if IS_ENABLED(CONFIG_KUNIT)
+
+#include 
+#include 
+
+struct __suppressed_warning {
+   struct list_head node;
+   const char *function;
+};
+
+void __start_suppress_warning(struct __suppressed_warning *warning);
+void __end_suppress_warning(struct __suppressed_warning *warning);
+bool __is_suppressed_warning(const char *function);
+
+#define DEFINE_SUPPRESSED_WARNING(func)\
+   struct __suppressed_warning __kunit_suppress_##func = \
+   { .function = __stringify(func) }
+
+#define START_SUPPRESSED_WARNING(func) \
+   __start_suppress_warning(&__kunit_suppress_##func)
+
+#define END_SUPPRESSED_WARNING(func) \
+   __end_suppress_warning(&__kunit_suppress_##func)
+
+#define IS_SUPPRESSED_WARNING(func) \
+