[PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@google.com>

Switch KUnit-compatible KASAN tests from using per-task KUnit resources
to console tracepoints.

This allows for two things:

1. Migrating tests that trigger a KASAN report in the context of a task
other than current to KUnit framework.
This is implemented in the patches that follow.

2. Parsing and matching the contents of KASAN reports.
This is not yet implemented.

Reviewed-by: Marco Elver <el...@google.com>
Signed-off-by: Andrey Konovalov <andre...@google.com>

---

Changed v2->v3:
- Rebased onto 6.1-rc1

Changes v1->v2:
- Remove kunit_kasan_status struct definition.
---
lib/Kconfig.kasan | 2 +-
mm/kasan/kasan.h | 8 ----
mm/kasan/kasan_test.c | 85 +++++++++++++++++++++++++++++++------------
mm/kasan/report.c | 31 ----------------
4 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index ca09b1cf8ee9..ba5b27962c34 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -181,7 +181,7 @@ config KASAN_VMALLOC

config KASAN_KUNIT_TEST
tristate "KUnit-compatible tests of KASAN bug detection capabilities" if !KUNIT_ALL_TESTS
- depends on KASAN && KUNIT
+ depends on KASAN && KUNIT && TRACEPOINTS
default KUNIT_ALL_TESTS
help
A KUnit-based KASAN test suite. Triggers different kinds of
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index abbcc1b0eec5..a84491bc4867 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -261,14 +261,6 @@ struct kasan_stack_ring {

#endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */

-#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
-/* Used in KUnit-compatible KASAN tests. */
-struct kunit_kasan_status {
- bool report_found;
- bool sync_fault;
-};
-#endif
-
#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)

static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c
index 0d59098f0876..0ff20bfa3376 100644
--- a/mm/kasan/kasan_test.c
+++ b/mm/kasan/kasan_test.c
@@ -5,8 +5,12 @@
* Author: Andrey Ryabinin <a.rya...@samsung.com>
*/

+#define pr_fmt(fmt) "kasan_test: " fmt
+
+#include <kunit/test.h>
#include <linux/bitops.h>
#include <linux/delay.h>
+#include <linux/io.h>
#include <linux/kasan.h>
#include <linux/kernel.h>
#include <linux/mm.h>
@@ -14,21 +18,28 @@
#include <linux/module.h>
#include <linux/printk.h>
#include <linux/random.h>
+#include <linux/set_memory.h>
#include <linux/slab.h>
#include <linux/string.h>
+#include <linux/tracepoint.h>
#include <linux/uaccess.h>
-#include <linux/io.h>
#include <linux/vmalloc.h>
-#include <linux/set_memory.h>
+#include <trace/events/printk.h>

#include <asm/page.h>

-#include <kunit/test.h>
-
#include "kasan.h"

#define OOB_TAG_OFF (IS_ENABLED(CONFIG_KASAN_GENERIC) ? 0 : KASAN_GRANULE_SIZE)

+static bool multishot;
+
+/* Fields set based on lines observed in the console. */
+static struct {
+ bool report_found;
+ bool async_fault;
+} test_status;
+
/*
* Some tests use these global variables to store return values from function
* calls that could otherwise be eliminated by the compiler as dead code.
@@ -36,35 +47,61 @@
void *kasan_ptr_result;
int kasan_int_result;

-static struct kunit_resource resource;
-static struct kunit_kasan_status test_status;
-static bool multishot;
+/* Probe for console output: obtains test_status lines of interest. */
+static void probe_console(void *ignore, const char *buf, size_t len)
+{
+ if (strnstr(buf, "BUG: KASAN: ", len))
+ WRITE_ONCE(test_status.report_found, true);
+ else if (strnstr(buf, "Asynchronous fault: ", len))
+ WRITE_ONCE(test_status.async_fault, true);
+}

-/*
- * Temporarily enable multi-shot mode. Otherwise, KASAN would only report the
- * first detected bug and panic the kernel if panic_on_warn is enabled. For
- * hardware tag-based KASAN also allow tag checking to be reenabled for each
- * test, see the comment for KUNIT_EXPECT_KASAN_FAIL().
- */
-static int kasan_test_init(struct kunit *test)
+static void register_tracepoints(struct tracepoint *tp, void *ignore)
+{
+ check_trace_callback_type_console(probe_console);
+ if (!strcmp(tp->name, "console"))
+ WARN_ON(tracepoint_probe_register(tp, probe_console, NULL));
+}
+
+static void unregister_tracepoints(struct tracepoint *tp, void *ignore)
+{
+ if (!strcmp(tp->name, "console"))
+ tracepoint_probe_unregister(tp, probe_console, NULL);
+}
+
+static int kasan_suite_init(struct kunit_suite *suite)
{
if (!kasan_enabled()) {
- kunit_err(test, "can't run KASAN tests with KASAN disabled");
+ pr_err("Can't run KASAN tests with KASAN disabled");
return -1;
}

+ /*
+ * Temporarily enable multi-shot mode. Otherwise, KASAN would only
+ * report the first detected bug and panic the kernel if panic_on_warn
+ * is enabled.
+ */
multishot = kasan_save_enable_multi_shot();
- test_status.report_found = false;
- test_status.sync_fault = false;
- kunit_add_named_resource(test, NULL, NULL, &resource,
- "kasan_status", &test_status);
+
+ /*
+ * Because we want to be able to build the test as a module, we need to
+ * iterate through all known tracepoints, since the static registration
+ * won't work here.
+ */
+ for_each_kernel_tracepoint(register_tracepoints, NULL);
return 0;
}

-static void kasan_test_exit(struct kunit *test)
+static void kasan_suite_exit(struct kunit_suite *suite)
{
kasan_restore_multi_shot(multishot);
- KUNIT_EXPECT_FALSE(test, test_status.report_found);
+ for_each_kernel_tracepoint(unregister_tracepoints, NULL);
+ tracepoint_synchronize_unregister();
+}
+
+static void kasan_test_exit(struct kunit *test)
+{
+ KUNIT_EXPECT_FALSE(test, READ_ONCE(test_status.report_found));
}

/**
@@ -106,11 +143,12 @@ static void kasan_test_exit(struct kunit *test)
if (IS_ENABLED(CONFIG_KASAN_HW_TAGS) && \
kasan_sync_fault_possible()) { \
if (READ_ONCE(test_status.report_found) && \
- READ_ONCE(test_status.sync_fault)) \
+ !READ_ONCE(test_status.async_fault)) \
kasan_enable_tagging(); \
migrate_enable(); \
} \
WRITE_ONCE(test_status.report_found, false); \
+ WRITE_ONCE(test_status.async_fault, false); \
} while (0)

#define KASAN_TEST_NEEDS_CONFIG_ON(test, config) do { \
@@ -1447,9 +1485,10 @@ static struct kunit_case kasan_kunit_test_cases[] = {

static struct kunit_suite kasan_kunit_test_suite = {
.name = "kasan",
- .init = kasan_test_init,
.test_cases = kasan_kunit_test_cases,
.exit = kasan_test_exit,
+ .suite_init = kasan_suite_init,
+ .suite_exit = kasan_suite_exit,
};

kunit_test_suite(kasan_kunit_test_suite);
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index df3602062bfd..31355851a5ec 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -30,8 +30,6 @@

#include <asm/sections.h>

-#include <kunit/test.h>
-
#include "kasan.h"
#include "../slab.h"

@@ -114,41 +112,12 @@ EXPORT_SYMBOL_GPL(kasan_restore_multi_shot);

#endif

-#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
-static void update_kunit_status(bool sync)
-{
- struct kunit *test;
- struct kunit_resource *resource;
- struct kunit_kasan_status *status;
-
- test = current->kunit_test;
- if (!test)
- return;
-
- resource = kunit_find_named_resource(test, "kasan_status");
- if (!resource) {
- kunit_set_failure(test);
- return;
- }
-
- status = (struct kunit_kasan_status *)resource->data;
- WRITE_ONCE(status->report_found, true);
- WRITE_ONCE(status->sync_fault, sync);
-
- kunit_put_resource(resource);
-}
-#else
-static void update_kunit_status(bool sync) { }
-#endif
-
static DEFINE_SPINLOCK(report_lock);

static void start_report(unsigned long *flags, bool sync)
{
/* Respect the /proc/sys/kernel/traceoff_on_warning interface. */
disable_trace_on_warning();
- /* Update status of the currently running KASAN test. */
- update_kunit_status(sync);
/* Do not allow LOCKDEP mangling KASAN reports. */
lockdep_off();
/* Make sure we don't end up in loop. */
--
2.25.1

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@google.com>

Migrate the kasan_rcu_uaf test to the KUnit framework.

Changes to the implementation of the test:

- Call rcu_barrier() after call_rcu() to make that the RCU callbacks get
triggered before the test is over.

- Cast pointer passed to rcu_dereference_protected as __rcu to get rid of
the Sparse warning.

- Check that KASAN prints a report via KUNIT_EXPECT_KASAN_FAIL.

Initially, this test was intended to check that Generic KASAN prints
auxiliary stack traces for RCU objects. Nevertheless, the test is enabled
for all modes to make that KASAN reports bad accesses in RCU callbacks.

The presence of auxiliary stack traces for the Generic mode needs to be
inspected manually.

Reviewed-by: Marco Elver <el...@google.com>
Signed-off-by: Andrey Konovalov <andre...@google.com>

---

Changed v2->v3:
- Rebased onto 6.1-rc1

---
mm/kasan/kasan_test.c | 37 ++++++++++++++++++++++++++++++++++++
mm/kasan/kasan_test_module.c | 30 -----------------------------
2 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c
index 0ff20bfa3376..38bf6ed61cb8 100644
--- a/mm/kasan/kasan_test.c
+++ b/mm/kasan/kasan_test.c
@@ -1141,6 +1141,42 @@ static void kmalloc_double_kzfree(struct kunit *test)
KUNIT_EXPECT_KASAN_FAIL(test, kfree_sensitive(ptr));
}

+static struct kasan_rcu_info {
+ int i;
+ struct rcu_head rcu;
+} *global_rcu_ptr;
+
+static void rcu_uaf_reclaim(struct rcu_head *rp)
+{
+ struct kasan_rcu_info *fp =
+ container_of(rp, struct kasan_rcu_info, rcu);
+
+ kfree(fp);
+ ((volatile struct kasan_rcu_info *)fp)->i;
+}
+
+/*
+ * Check that Generic KASAN prints auxiliary stack traces for RCU callbacks.
+ * The report needs to be inspected manually.
+ *
+ * This test is still enabled for other KASAN modes to make sure that all modes
+ * report bad accesses in tested scenarios.
+ */
+static void rcu_uaf(struct kunit *test)
+{
+ struct kasan_rcu_info *ptr;
+
+ ptr = kmalloc(sizeof(struct kasan_rcu_info), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
+
+ global_rcu_ptr = rcu_dereference_protected(
+ (struct kasan_rcu_info __rcu *)ptr, NULL);
+
+ KUNIT_EXPECT_KASAN_FAIL(test,
+ call_rcu(&global_rcu_ptr->rcu, rcu_uaf_reclaim);
+ rcu_barrier());
+}
+
static void vmalloc_helpers_tags(struct kunit *test)
{
void *ptr;
@@ -1472,6 +1508,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
KUNIT_CASE(kasan_bitops_generic),
KUNIT_CASE(kasan_bitops_tags),
KUNIT_CASE(kmalloc_double_kzfree),
+ KUNIT_CASE(rcu_uaf),
KUNIT_CASE(vmalloc_helpers_tags),
KUNIT_CASE(vmalloc_oob),
KUNIT_CASE(vmap_tags),
diff --git a/mm/kasan/kasan_test_module.c b/mm/kasan/kasan_test_module.c
index e4ca82dc2c16..4688cbcd722d 100644
--- a/mm/kasan/kasan_test_module.c
+++ b/mm/kasan/kasan_test_module.c
@@ -62,35 +62,6 @@ static noinline void __init copy_user_test(void)
kfree(kmem);
}

-static struct kasan_rcu_info {
- int i;
- struct rcu_head rcu;
-} *global_rcu_ptr;
-
-static noinline void __init kasan_rcu_reclaim(struct rcu_head *rp)
-{
- struct kasan_rcu_info *fp = container_of(rp,
- struct kasan_rcu_info, rcu);
-
- kfree(fp);
- ((volatile struct kasan_rcu_info *)fp)->i;
-}
-
-static noinline void __init kasan_rcu_uaf(void)
-{
- struct kasan_rcu_info *ptr;
-
- pr_info("use-after-free in kasan_rcu_reclaim\n");
- ptr = kmalloc(sizeof(struct kasan_rcu_info), GFP_KERNEL);
- if (!ptr) {
- pr_err("Allocation failed\n");
- return;
- }
-
- global_rcu_ptr = rcu_dereference_protected(ptr, NULL);
- call_rcu(&global_rcu_ptr->rcu, kasan_rcu_reclaim);
-}
-
static noinline void __init kasan_workqueue_work(struct work_struct *work)
{
kfree(work);
@@ -130,7 +101,6 @@ static int __init test_kasan_module_init(void)
bool multishot = kasan_save_enable_multi_shot();

copy_user_test();
- kasan_rcu_uaf();
kasan_workqueue_uaf();

kasan_restore_multi_shot(multishot);
--
2.25.1

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@google.com>

Migrate the workqueue_uaf test to the KUnit framework.

Initially, this test was intended to check that Generic KASAN prints

auxiliary stack traces for workqueues. Nevertheless, the test is enabled
for all modes to make that KASAN reports bad accesses in the tested
scenario.

The presence of auxiliary stack traces for the Generic mode needs to be
inspected manually.

Reviewed-by: Marco Elver <el...@google.com>
Signed-off-by: Andrey Konovalov <andre...@google.com>

---

Changed v2->v3:
- Rebased onto 6.1-rc1

---
mm/kasan/kasan_test.c | 40 +++++++++++++++++++++++++++++-------
mm/kasan/kasan_test_module.c | 30 ---------------------------
2 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c
index 38bf6ed61cb8..e27591ef2777 100644
--- a/mm/kasan/kasan_test.c
+++ b/mm/kasan/kasan_test.c
@@ -1141,6 +1141,14 @@ static void kmalloc_double_kzfree(struct kunit *test)
KUNIT_EXPECT_KASAN_FAIL(test, kfree_sensitive(ptr));
}

+/*
+ * The two tests below check that Generic KASAN prints auxiliary stack traces
+ * for RCU callbacks and workqueues. The reports need to be inspected manually.
+ *
+ * These tests are still enabled for other KASAN modes to make sure that all
+ * modes report bad accesses in tested scenarios.
+ */
+
static struct kasan_rcu_info {
int i;
struct rcu_head rcu;
@@ -1155,13 +1163,6 @@ static void rcu_uaf_reclaim(struct rcu_head *rp)

((volatile struct kasan_rcu_info *)fp)->i;
}

-/*
- * Check that Generic KASAN prints auxiliary stack traces for RCU callbacks.
- * The report needs to be inspected manually.
- *
- * This test is still enabled for other KASAN modes to make sure that all modes
- * report bad accesses in tested scenarios.
- */
static void rcu_uaf(struct kunit *test)
{
struct kasan_rcu_info *ptr;
@@ -1177,6 +1178,30 @@ static void rcu_uaf(struct kunit *test)
rcu_barrier());
}

+static void workqueue_uaf_work(struct work_struct *work)
+{
+ kfree(work);
+}
+
+static void workqueue_uaf(struct kunit *test)
+{
+ struct workqueue_struct *workqueue;
+ struct work_struct *work;
+
+ workqueue = create_workqueue("kasan_workqueue_test");
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, workqueue);
+
+ work = kmalloc(sizeof(struct work_struct), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, work);
+
+ INIT_WORK(work, workqueue_uaf_work);
+ queue_work(workqueue, work);
+ destroy_workqueue(workqueue);
+
+ KUNIT_EXPECT_KASAN_FAIL(test,
+ ((volatile struct work_struct *)work)->data);

+}
+
static void vmalloc_helpers_tags(struct kunit *test)
{
void *ptr;

@@ -1509,6 +1534,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
KUNIT_CASE(kasan_bitops_tags),
KUNIT_CASE(kmalloc_double_kzfree),
KUNIT_CASE(rcu_uaf),
+ KUNIT_CASE(workqueue_uaf),

KUNIT_CASE(vmalloc_helpers_tags),
KUNIT_CASE(vmalloc_oob),
KUNIT_CASE(vmap_tags),
diff --git a/mm/kasan/kasan_test_module.c b/mm/kasan/kasan_test_module.c

index 4688cbcd722d..7be7bed456ef 100644

--- a/mm/kasan/kasan_test_module.c
+++ b/mm/kasan/kasan_test_module.c
@@ -62,35 +62,6 @@ static noinline void __init copy_user_test(void)
kfree(kmem);
}

-static noinline void __init kasan_workqueue_work(struct work_struct *work)
-{
- kfree(work);
-}
-
-static noinline void __init kasan_workqueue_uaf(void)
-{
- struct workqueue_struct *workqueue;
- struct work_struct *work;
-
- workqueue = create_workqueue("kasan_wq_test");
- if (!workqueue) {

- pr_err("Allocation failed\n");
- return;
- }

- work = kmalloc(sizeof(struct work_struct), GFP_KERNEL);
- if (!work) {

- pr_err("Allocation failed\n");

- return;
- }
-
- INIT_WORK(work, kasan_workqueue_work);
- queue_work(workqueue, work);
- destroy_workqueue(workqueue);
-
- pr_info("use-after-free on workqueue\n");
- ((volatile struct work_struct *)work)->data;
-}
-
static int __init test_kasan_module_init(void)
{
/*
@@ -101,7 +72,6 @@ static int __init test_kasan_module_init(void)

bool multishot = kasan_save_enable_multi_shot();

copy_user_test();

- kasan_workqueue_uaf();

kasan_restore_multi_shot(multishot);
return -EAGAIN;
--
2.25.1

[Peter Collingbourne]

My build script for a KASAN-enabled kernel does something like:

make defconfig
scripts/config -e CONFIG_KUNIT -e CONFIG_KASAN -e CONFIG_KASAN_HW_TAGS
-e CONFIG_KASAN_KUNIT_TEST
yes '' | make syncconfig

and after this change, the unit tests are no longer built. Should this
use "select TRACING" instead?

Peter

[Marco Elver]

I think we shouldn't select TRACING, which should only be selected by
tracers. You'd need CONFIG_FTRACE=y.

Since FTRACE is rather big, we probably also shouldn't implicitly
select it. Instead, at least when using kunit.py tool, we could add a
mm/kasan/.kunitconfig like:

CONFIG_KUNIT=y
CONFIG_KASAN=y
CONFIG_KASAN_KUNIT_TEST=y
# Additional dependencies.
CONFIG_FTRACE=y

Which mirrors the KFENCE mm/kfence/.kunitconfig. But that doesn't help
if you want to run it with something other than KUnit tool.

[Peter Collingbourne]

Doesn't CONFIG_FTRACE=y mean "function tracing", i.e. function
entry/exit tracing using compiler instrumentation? As far as I can
tell, the KASAN tests do not make use of this feature. They only use
the kernel tracepoint infrastructure to trace the "console" tracepoint
defined in include/trace/events/printk.h, which is not associated with
function entry/exit.

I have yet to find any evidence that TRACING ought to only be selected
by tracers. As far as I can tell, TRACING appears to be the minimal
config required in order for it to be possible to trace pre-defined
(i.e. defined with TRACE_EVENT) tracepoints, which is all that KASAN
needs. (I also tried selecting TRACEPOINTS, but this led to a number
of link failures.) If select TRACING is only used by tracers, it could
just mean that only tracers are making use of this functionality
inside the kernel. From that perspective the KASAN tests can
themselves be considered a "tracer" (albeit a very specialized one).

If I locally revert the change to lib/Kconfig.kasan and add the
TRACING select, the KASAN tests pass when using my kernel build
script, which suggests that TRACING is all that is needed.

> Since FTRACE is rather big, we probably also shouldn't implicitly
> select it. Instead, at least when using kunit.py tool, we could add a
> mm/kasan/.kunitconfig like:
>
> CONFIG_KUNIT=y
> CONFIG_KASAN=y
> CONFIG_KASAN_KUNIT_TEST=y
> # Additional dependencies.
> CONFIG_FTRACE=y
>
> Which mirrors the KFENCE mm/kfence/.kunitconfig. But that doesn't help
> if you want to run it with something other than KUnit tool.

In any case, I'm not sure I'm in favor of adding yet another config
that folks need to know to enable in order to avoid silently disabling
the unit tests. Many developers will maintain their own scripts for
kernel development if the existing ones do not meet their needs. It's
possible that kunit.py will work out for me now (when I looked at it
before, it was useless for me because it only supported UML, but it
looks like it supports QEMU now), but there's no guarantee that it
will, so I might stick with my scripts for a while.

Peter

[Marco Elver]

+Cc tracing maintainers

Yes, you are right, and it's something I've wondered how to do better
as well. Let's try to consult tracing maintainers on what the right
approach is.

[Peter Collingbourne]

On Wed, Feb 15, 2023 at 11:33 AM Steven Rostedt <ros...@goodmis.org> wrote:

>
> On Wed, 15 Feb 2023 09:57:40 +0100
> Marco Elver <el...@google.com> wrote:
>
> > Yes, you are right, and it's something I've wondered how to do better
> > as well. Let's try to consult tracing maintainers on what the right
> > approach is.
>

> I have to go and revisit the config options for CONFIG_FTRACE and
> CONFIG_TRACING, as they were added when this all started (back in
> 2008), and the naming was rather all misnomers back then.
>
> "ftrace" is really for just the function tracing, but CONFIG_FTRACE
> really should just be for the function tracing infrastructure, and
> perhaps not even include trace events :-/ But at the time it was
> created, it was for all the "tracers" (this was added before trace
> events).

It would be great to see this cleaned up. I found this aspect of how
tracing works rather confusing.

So do you think it makes sense for the KASAN tests to "select TRACING"
for now if the code depends on the trace event infrastructure?

Peter

[Peter Collingbourne]

Any thoughts? It looks like someone else got tripped up by this:
https://reviews.llvm.org/D144057

Peter

[Nick Desaulniers]

https://reviews.llvm.org/D144057#4311029
Peter, please triple check.
--
Thanks,
~Nick Desaulniers

[Paul Heidekrüger]

Hi all!

On 05.05.2023 09:58, Steven Rostedt wrote:

> On Mon, 1 May 2023 15:02:37 -0700
> Peter Collingbourne <p...@google.com> wrote:
>
> > > > "ftrace" is really for just the function tracing, but CONFIG_FTRACE
> > > > really should just be for the function tracing infrastructure, and
> > > > perhaps not even include trace events :-/ But at the time it was
> > > > created, it was for all the "tracers" (this was added before trace
> > > > events).
> > >
> > > It would be great to see this cleaned up. I found this aspect of how
> > > tracing works rather confusing.
> > >
> > > So do you think it makes sense for the KASAN tests to "select TRACING"
> > > for now if the code depends on the trace event infrastructure?
> >
> > Any thoughts? It looks like someone else got tripped up by this:
> > https://reviews.llvm.org/D144057
>

> Yeah, it really does need to get cleaned up, but unfortunately it's not
> going to be a trivial change. We need to make sure it's done in a way that
> an old .config still keeps the same things enabled with the new config
> settings. That takes some trickery in the dependency.
>
> I'll add this to my todo list, hopefully it doesn't fall into the abyss
> portion of that list :-p
>
> -- Steve

Just adding to Peter's concern re: CONFIG_KASAN_KUNIT_TEST's dependency on
CONFIG_TRACEPOINTS.

I'm having no luck running the KASan KUnit tests on arm64 with the following
.kunitconfig on v6.6.0:

CONFIG_KUNIT=y
CONFIG_KUNIT_ALL_TESTS=n
CONFIG_DEBUG_KERNEL=y
CONFIG_KASAN=y
CINFIG_KASAN_GENERIC=y
CONFIG_KASAN_KUNIT_TEST=y

CONFIG_TRACEPOINTS, which CONFIG_KASAN_TEST relies on since the patch this
thread is based on, isn't defined for arm64, AFAICT.

If I comment out the dependency on CONFIG_TRACEPOINTS, the tests appear to run,
but KUnit isn't picking up the KASan output.

If I revert the patch, the above .kunitconfig appears to work fine on arm64 and
the tests pass.

The above .kunitconfig works as intended on X86, no changes necessary.

Am I missing something?

Many thanks,
Paul

[Andrey Konovalov]

Hi Paul,

I've been successfully running KASAN tests with CONFIG_TRACEPOINTS
enabled on arm64 since this patch landed.

What happens when you try running the tests with .kunitconfig? Does
CONFIG_TRACEPOINTS or CONFIG_KASAN_KUNIT_TEST get disabled during
kernel building? Or tests just don't get executed?

Thanks!

[Paul Heidekrüger]

Hi Andrey!

Interesting ...

> What happens when you try running the tests with .kunitconfig? Does
> CONFIG_TRACEPOINTS or CONFIG_KASAN_KUNIT_TEST get disabled during
> kernel building?

Yes, exactly, that's what's happening.

Here's the output kunit.py is giving me. I replaced CONFIG_DEBUG_KERNEL with
CONFIG_TRACEPOINTS in my .kunitconfig. Otherwise, it's identical with the one I
posted above.

➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64
Configuring KUnit Kernel ...
Regenerating .config ...
Populating config with:
$ make ARCH=arm64 O=.kunit olddefconfig
ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
This is probably due to unsatisfied dependencies.
Missing: CONFIG_KASAN_KUNIT_TEST=y, CONFIG_TRACEPOINTS=y

Does CONFIG_TRACEPOINTS have some dependency I'm not seeing? I couldn't find a
reason why it would get disabled, but I could definitely be wrong.

> Or tests just don't get executed?
>
> Thanks!

Many thanks,
Paul

[Andrey Konovalov]

Does your .kunitconfig include CONFIG_TRACEPOINTS=y? I don't see it in
the listing that you sent earlier.

[Paul Heidekrüger]

Yes. For the kunit.py output from my previous email, I replaced
CONFIG_DEBUG_KERNEL=y with CONFIG_TRACEPOINTS=y. So, the .kunitconfig I used to
produce the output above was:

CONFIG_KUNIT=y
CONFIG_KUNIT_ALL_TESTS=n
CONFIG_TRACEPOINTS=y
CONFIG_KASAN=y
CONFIG_KASAN_GENERIC=y
CONFIG_KASAN_KUNIT_TEST=y

This more or less mirrors what mm/kfence/.kunitconfig is doing, which also isn't
working on my side; kunit.py reports the same error.

Many thanks,
Paul

[Marco Elver]

mm/kfence/.kunitconfig does CONFIG_FTRACE=y. TRACEPOINTS is not user
selectable. I don't think any of this has changed since the initial
discussion above, so CONFIG_FTRACE=y is still needed.

[Paul Heidekrüger]

Using CONFIG_FTRACE=y instead of CONFIG_TRACEPOINTS=y produces the same error
for me.

So

CONFIG_KUNIT=y
CONFIG_KUNIT_ALL_TESTS=n
CONFIG_FTRACE=y
CONFIG_KASAN=y
CONFIG_KASAN_GENERIC=y
CONFIG_KASAN_KUNIT_TEST=y

produces

➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64
Configuring KUnit Kernel ...
Regenerating .config ...
Populating config with:

$ make ARCH=arm64 O=.kunit olddefconfig CC=clang

ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
This is probably due to unsatisfied dependencies.
Missing: CONFIG_KASAN_KUNIT_TEST=y

By that error message, CONFIG_FTRACE appears to be present in the generated
config, but CONFIG_KASAN_KUNIT_TEST still isn't. Presumably,
CONFIG_KASAN_KUNIT_TEST is missing because of an unsatisfied dependency, which
must be CONFIG_TRACEPOINTS, unless I'm missing something ...

If I just generate an arm64 defconfig and select CONFIG_FTRACE=y,
CONFIG_TRACEPOINTS=y shows up in my .config. So, maybe this is kunit.py-related
then?

Andrey, you said that the tests have been working for you; are you running them
with kunit.py?

Many thanks,
Paul

[Andrey Konovalov]

No, I just run the kernel built with a config file that I put together
based on defconfig.

[Paul Heidekrüger]

Ah. I believe I've figured it out.

When I add CONFIG_STACK_TRACER=y in addition to CONFIG_FTRACE=y, it works.

CONFIG_STACK_TRACER selects CONFIG_FUNCTION_TRACER, CONFIG_FUNCTION_TRACER
selects CONFIG_GENERIC_TRACER, CONFIG_GENERIC_TRACER selects CONFIG_TRACING, and
CONFIG_TRACING selects CONFIG_TRACEPOINTS.

CONFIG_BLK_DEV_IO_TRACE=y also works instead of CONFIG_STACK_TRACER=y, as it
directly selects CONFIG_TRACEPOINTS.

CONFIG_FTRACE=y on its own does not appear suffice for kunit.py on arm64.

I believe the reason my .kunitconfig as well as the existing
mm/kfence/.kunitconfig work on X86 is because CONFIG_TRACEPOINTS=y is present in
an X86 defconfig.

Does this make sense?

Would you welcome a patch addressing this for the existing
mm/kfence/.kunitconfig?

I would also like to submit a patch for an mm/kasan/.kunitconfig. Do you think
that would be helpful too?

FWICT, kernel/kcsan/.kunitconfig might also be affected since
CONFIG_KCSAN_KUNIT_TEST also depends on CONFIG_TRACEPOITNS, but I would have to
test that. That could be a third patch.

What do you think?

Many thanks,
Paul

[Marco Elver]

CONFIG_FTRACE should be enough - maybe also check x86 vs. arm64 to debug more.

> CONFIG_STACK_TRACER selects CONFIG_FUNCTION_TRACER, CONFIG_FUNCTION_TRACER
> selects CONFIG_GENERIC_TRACER, CONFIG_GENERIC_TRACER selects CONFIG_TRACING, and
> CONFIG_TRACING selects CONFIG_TRACEPOINTS.
>
> CONFIG_BLK_DEV_IO_TRACE=y also works instead of CONFIG_STACK_TRACER=y, as it
> directly selects CONFIG_TRACEPOINTS.
>
> CONFIG_FTRACE=y on its own does not appear suffice for kunit.py on arm64.

When you build manually with just CONFIG_FTRACE, is CONFIG_TRACEPOINTS enabled?

> I believe the reason my .kunitconfig as well as the existing
> mm/kfence/.kunitconfig work on X86 is because CONFIG_TRACEPOINTS=y is present in
> an X86 defconfig.
>
> Does this make sense?
>
> Would you welcome a patch addressing this for the existing
> mm/kfence/.kunitconfig?
>
> I would also like to submit a patch for an mm/kasan/.kunitconfig. Do you think
> that would be helpful too?
>
> FWICT, kernel/kcsan/.kunitconfig might also be affected since
> CONFIG_KCSAN_KUNIT_TEST also depends on CONFIG_TRACEPOITNS, but I would have to
> test that. That could be a third patch.

I'd support figuring out the minimal config (CONFIG_FTRACE or
something else?) that satisfies the TRACEPOINTS dependency. I always
thought CONFIG_FTRACE ought to be the one config option, but maybe
something changed.

Also maybe one of the tracing maintainers can help untangle what's
going on here.

Thanks,
-- Marco

[Paul Heidekrüger]

See below.

> > CONFIG_STACK_TRACER selects CONFIG_FUNCTION_TRACER, CONFIG_FUNCTION_TRACER
> > selects CONFIG_GENERIC_TRACER, CONFIG_GENERIC_TRACER selects CONFIG_TRACING, and
> > CONFIG_TRACING selects CONFIG_TRACEPOINTS.
> >
> > CONFIG_BLK_DEV_IO_TRACE=y also works instead of CONFIG_STACK_TRACER=y, as it
> > directly selects CONFIG_TRACEPOINTS.
> >
> > CONFIG_FTRACE=y on its own does not appear suffice for kunit.py on arm64.
>
> When you build manually with just CONFIG_FTRACE, is CONFIG_TRACEPOINTS enabled?

When I add CONFIG_FTRACE and enter-key my way through the FTRACE prompts - I
believe because CONFIG_FTRACE is a menuconfig? - at the beginning of a build,
CONFIG_TRACEPOINTS does get set on arm64, yes.

On X86, the defconfig already includes CONIFG_TRACEPOINTS.

I also had a closer look at how kunit.py builds its configs.
I believe it does something along the following lines:

cp <path_to_kunitconfig> .kunit/.config
make ARCH=arm64 O=.kunit olddefconfig

On arm64, that isn't enough to set CONFIG_TRACEPOINTS; same behaviour when run
outside of kunit.py.

For CONFIG_TRACEPOINTS, `make ARCH=arm64 menuconfig` shows:

Symbol: TRACEPOINTS [=n]
Type : bool
Defined at init/Kconfig:1920
Selected by [n]:
- TRACING [=n]
- BLK_DEV_IO_TRACE [=n] && FTRACE [=y] && SYSFS [=y] && BLOCK [=y]

So, CONFIG_TRACING or CONFIG_BLK_DEV_IO_TRACE are the two options that prevent
CONFIG_TRACEPOINTS from being set on arm64.

For CONFIG_TRACING we have:

Symbol: TRACING [=n]
Type : bool
Defined at kernel/trace/Kconfig:157
Selects: RING_BUFFER [=n] && STACKTRACE [=y] && TRACEPOINTS [=n] && NOP_TRACER [=n] && BINARY_PRINTF [=n] && EVENT_TRACING [=n] && TRACE_CLOCK [=y] && TASKS_RCU [=n]
Selected by [n]:
- DRM_I915_TRACE_GEM [=n] && HAS_IOMEM [=y] && DRM_I915 [=n] && EXPERT [=n] && DRM_I915_DEBUG_GEM [=n]
- DRM_I915_TRACE_GTT [=n] && HAS_IOMEM [=y] && DRM_I915 [=n] && EXPERT [=n] && DRM_I915_DEBUG_GEM [=n]
- PREEMPTIRQ_TRACEPOINTS [=n] && (TRACE_PREEMPT_TOGGLE [=n] || TRACE_IRQFLAGS [=n])
- GENERIC_TRACER [=n]
- ENABLE_DEFAULT_TRACERS [=n] && FTRACE [=y] && !GENERIC_TRACER [=n]
- FPROBE_EVENTS [=n] && FTRACE [=y] && FPROBE [=n] && HAVE_REGS_AND_STACK_ACCESS_API [=y]
- KPROBE_EVENTS [=n] && FTRACE [=y] && KPROBES [=n] && HAVE_REGS_AND_STACK_ACCESS_API [=y]
- UPROBE_EVENTS [=n] && FTRACE [=y] && ARCH_SUPPORTS_UPROBES [=y] && MMU [=y] && PERF_EVENTS [=n]
- SYNTH_EVENTS [=n] && FTRACE [=y]
- USER_EVENTS [=n] && FTRACE [=y]
- HIST_TRIGGERS [=n] && FTRACE [=y] && ARCH_HAVE_NMI_SAFE_CMPXCHG [=y]

> > I believe the reason my .kunitconfig as well as the existing
> > mm/kfence/.kunitconfig work on X86 is because CONFIG_TRACEPOINTS=y is present in
> > an X86 defconfig.
> >
> > Does this make sense?
> >
> > Would you welcome a patch addressing this for the existing
> > mm/kfence/.kunitconfig?
> >
> > I would also like to submit a patch for an mm/kasan/.kunitconfig. Do you think
> > that would be helpful too?
> >
> > FWICT, kernel/kcsan/.kunitconfig might also be affected since
> > CONFIG_KCSAN_KUNIT_TEST also depends on CONFIG_TRACEPOITNS, but I would have to
> > test that. That could be a third patch.
>
> I'd support figuring out the minimal config (CONFIG_FTRACE or
> something else?) that satisfies the TRACEPOINTS dependency. I always
> thought CONFIG_FTRACE ought to be the one config option, but maybe
> something changed.

If we want a minimal config, setting CONFIG_BLK_DEV_IO_TRACE,
CONFIG_SYNTH_EVENTS or CONFIG_USER_EVENTS seem like viable options, for
instance. But AFAICT, setting them in the context of KASan doesn't really make
sense, and I might be missing an obvious choice here too.

What do you think?

> Also maybe one of the tracing maintainers can help untangle what's
> going on here.
>
> Thanks,
> -- Marco

Many thanks,
Paul

[Paul Heidekrüger]

Hi all,

Just giving this thread a polite bump, hoping that someone has some pointers.

The TL;DR is the following: I’m trying to run KASan KUnit tests with the
following local .kunitconfig:

CONFIG_KUNIT=y
CONFIG_KUNIT_ALL_TESTS=n
CONFIG_FTRACE=y
CONFIG_KASAN=y
CONFIG_KASAN_GENERIC=y
CONFIG_KASAN_KUNIT_TEST=y

The problem is that on arm64, this does not appear to be enough to set all of
CONFIG_KASAN_KUNIT_TEST’s dependencies, namely CONFIG_TRACEPOINTS.

An additional option is needed to enable CONFIG_TRACEPOINTS. As per `make
menuconfig`, this is either CONFIG_BLK_DEV_IO_TRACE or any (combination of)
option(s) that enable(s) CONFIG_TRACING. See the `make menuconfig` output in my
previous email for details.

Which option do you think is appropriate here? Or am I missing something?

For anyone wanting to reproduce, use:
./tools/testing/kunit/kunit.py run —kunitconfig=<path_to_above_kunitconfig> --arch=arm64

Many thanks,
Paul