2022-10-18 18:11:52

by andrey.konovalov

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

From: Andrey Konovalov <[email protected]>

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 <[email protected]>
Signed-off-by: Andrey Konovalov <[email protected]>

---

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 <[email protected]>
*/

+#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


2022-10-18 18:12:10

by andrey.konovalov

[permalink] [raw]
Subject: [PATCH v3 3/3] kasan: migrate workqueue_uaf test to kunit

From: Andrey Konovalov <[email protected]>

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 <[email protected]>
Signed-off-by: Andrey Konovalov <[email protected]>

---

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

2023-02-14 01:21:41

by Peter Collingbourne

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Tue, Oct 18, 2022 at 10:17 AM <[email protected]> wrote:
>
> From: Andrey Konovalov <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Andrey Konovalov <[email protected]>
>
> ---
>
> 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

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

2023-02-14 06:08:08

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Tue, 14 Feb 2023 at 02:21, Peter Collingbourne <[email protected]> wrote:
>
> On Tue, Oct 18, 2022 at 10:17 AM <[email protected]> wrote:
> >
> > From: Andrey Konovalov <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Andrey Konovalov <[email protected]>
> >
> > ---
> >
> > 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
>
> 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?

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.

2023-02-15 02:56:12

by Peter Collingbourne

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Mon, Feb 13, 2023 at 10:08 PM Marco Elver <[email protected]> wrote:
>
> On Tue, 14 Feb 2023 at 02:21, Peter Collingbourne <[email protected]> wrote:
> >
> > On Tue, Oct 18, 2022 at 10:17 AM <[email protected]> wrote:
> > >
> > > From: Andrey Konovalov <[email protected]>
> > >
> > > 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 <[email protected]>
> > > Signed-off-by: Andrey Konovalov <[email protected]>
> > >
> > > ---
> > >
> > > 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
> >
> > 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?
>
> I think we shouldn't select TRACING, which should only be selected by
> tracers. You'd need CONFIG_FTRACE=y.

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

2023-02-15 08:58:25

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

+Cc tracing maintainers

On Wed, 15 Feb 2023 at 03:56, Peter Collingbourne <[email protected]> wrote:
>
> On Mon, Feb 13, 2023 at 10:08 PM Marco Elver <[email protected]> wrote:
> >
> > On Tue, 14 Feb 2023 at 02:21, Peter Collingbourne <[email protected]> wrote:
> > >
> > > On Tue, Oct 18, 2022 at 10:17 AM <[email protected]> wrote:
> > > >
> > > > From: Andrey Konovalov <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > Signed-off-by: Andrey Konovalov <[email protected]>
> > > >
> > > > ---
> > > >
> > > > 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
> > >
> > > 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?
> >
> > I think we shouldn't select TRACING, which should only be selected by
> > tracers. You'd need CONFIG_FTRACE=y.
>
> 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.

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 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

2023-02-15 19:33:16

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Wed, 15 Feb 2023 09:57:40 +0100
Marco Elver <[email protected]> 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).

-- Steve

2023-02-24 06:45:26

by Peter Collingbourne

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Wed, Feb 15, 2023 at 11:33 AM Steven Rostedt <[email protected]> wrote:
>
> On Wed, 15 Feb 2023 09:57:40 +0100
> Marco Elver <[email protected]> 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

2023-05-01 22:05:13

by Peter Collingbourne

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Thu, Feb 23, 2023 at 10:45 PM Peter Collingbourne <[email protected]> wrote:
>
> On Wed, Feb 15, 2023 at 11:33 AM Steven Rostedt <[email protected]> wrote:
> >
> > On Wed, 15 Feb 2023 09:57:40 +0100
> > Marco Elver <[email protected]> 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?

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

Peter

2023-05-01 22:31:28

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Mon, May 1, 2023 at 3:02 PM Peter Collingbourne <[email protected]> wrote:
>
> On Thu, Feb 23, 2023 at 10:45 PM Peter Collingbourne <[email protected]> wrote:
> >
> > On Wed, Feb 15, 2023 at 11:33 AM Steven Rostedt <[email protected]> wrote:
> > >
> > > On Wed, 15 Feb 2023 09:57:40 +0100
> > > Marco Elver <[email protected]> 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?
>
> Any thoughts? It looks like someone else got tripped up by this:
> https://reviews.llvm.org/D144057

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

2023-05-05 14:40:59

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Mon, 1 May 2023 15:02:37 -0700
Peter Collingbourne <[email protected]> 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

2023-12-11 16:45:33

by Paul Heidekrüger

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

Hi all!

On 05.05.2023 09:58, Steven Rostedt wrote:
> On Mon, 1 May 2023 15:02:37 -0700
> Peter Collingbourne <[email protected]> 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

2023-12-11 17:50:37

by Andrey Konovalov

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Mon, Dec 11, 2023 at 5:37 PM Paul Heidekrüger
<[email protected]> wrote:
>
> Hi all!
>
> On 05.05.2023 09:58, Steven Rostedt wrote:
> > On Mon, 1 May 2023 15:02:37 -0700
> > Peter Collingbourne <[email protected]> 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?

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!

2023-12-11 18:59:37

by Paul Heidekrüger

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

Hi Andrey!

On 11.12.2023 18:50, Andrey Konovalov wrote:
> On Mon, Dec 11, 2023 at 5:37 PM Paul Heidekrüger
> <[email protected]> wrote:
> >
> > Hi all!
> >
> > On 05.05.2023 09:58, Steven Rostedt wrote:
> > > On Mon, 1 May 2023 15:02:37 -0700
> > > Peter Collingbourne <[email protected]> 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?
>
> Hi Paul,
>
> I've been successfully running KASAN tests with CONFIG_TRACEPOINTS
> enabled on arm64 since this patch landed.

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

2023-12-11 20:51:24

by Andrey Konovalov

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Mon, Dec 11, 2023 at 7:59 PM Paul Heidekrüger
<[email protected]> wrote:
>
> > Hi Paul,
> >
> > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS
> > enabled on arm64 since this patch landed.
>
> 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.

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

2023-12-11 22:48:55

by Paul Heidekrüger

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On 11.12.2023 21:51, Andrey Konovalov wrote:
> On Mon, Dec 11, 2023 at 7:59 PM Paul Heidekrüger
> <[email protected]> wrote:
> >
> > > Hi Paul,
> > >
> > > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS
> > > enabled on arm64 since this patch landed.
> >
> > 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.
>
> Does your .kunitconfig include CONFIG_TRACEPOINTS=y? I don't see it in
> the listing that you sent earlier.

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

2023-12-11 22:57:29

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Mon, 11 Dec 2023 at 23:48, Paul Heidekrüger <[email protected]> wrote:
>
> On 11.12.2023 21:51, Andrey Konovalov wrote:
> > On Mon, Dec 11, 2023 at 7:59 PM Paul Heidekrüger
> > <[email protected]> wrote:
> > >
> > > > Hi Paul,
> > > >
> > > > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS
> > > > enabled on arm64 since this patch landed.
> > >
> > > 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.
> >
> > Does your .kunitconfig include CONFIG_TRACEPOINTS=y? I don't see it in
> > the listing that you sent earlier.
>
> 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.

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.

2023-12-11 23:37:55

by Paul Heidekrüger

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On 11.12.2023 23:56, Marco Elver wrote:
> On Mon, 11 Dec 2023 at 23:48, Paul Heidekrüger <[email protected]> wrote:
> >
> > On 11.12.2023 21:51, Andrey Konovalov wrote:
> > > On Mon, Dec 11, 2023 at 7:59 PM Paul Heidekrüger
> > > <[email protected]> wrote:
> > > >
> > > > > Hi Paul,
> > > > >
> > > > > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS
> > > > > enabled on arm64 since this patch landed.
> > > >
> > > > 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.
> > >
> > > Does your .kunitconfig include CONFIG_TRACEPOINTS=y? I don't see it in
> > > the listing that you sent earlier.
> >
> > 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.
>
> 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.

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

2023-12-11 23:38:42

by Andrey Konovalov

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger
<[email protected]> wrote:
>
> 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?

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

2023-12-12 09:20:27

by Paul Heidekrüger

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On 12.12.2023 00:37, Andrey Konovalov wrote:
> On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger
> <[email protected]> wrote:
> >
> > 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?
>
> No, I just run the kernel built with a config file that I put together
> based on defconfig.

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

2023-12-12 09:33:39

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On Tue, 12 Dec 2023 at 10:19, Paul Heidekrüger <[email protected]> wrote:
>
> On 12.12.2023 00:37, Andrey Konovalov wrote:
> > On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger
> > <[email protected]> wrote:
> > >
> > > 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?
> >
> > No, I just run the kernel built with a config file that I put together
> > based on defconfig.
>
> Ah. I believe I've figured it out.
>
> When I add CONFIG_STACK_TRACER=y in addition to CONFIG_FTRACE=y, it works.

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

2024-01-07 18:30:44

by Paul Heidekrüger

[permalink] [raw]
Subject: Re: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

On 12.12.2023 10:32, Marco Elver wrote:
> On Tue, 12 Dec 2023 at 10:19, Paul Heidekrüger <[email protected]> wrote:
> >
> > On 12.12.2023 00:37, Andrey Konovalov wrote:
> > > On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger
> > > <[email protected]> wrote:
> > > >
> > > > 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?
> > >
> > > No, I just run the kernel built with a config file that I put together
> > > based on defconfig.
> >
> > Ah. I believe I've figured it out.
> >
> > When I add CONFIG_STACK_TRACER=y in addition to CONFIG_FTRACE=y, it works.
>
> CONFIG_FTRACE should be enough - maybe also check x86 vs. arm64 to debug more.

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


2024-02-05 11:34:48

by Paul Heidekrüger

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints



On 7 Jan 2024, at 19:22, Paul Heidekrüger wrote:

> On 12.12.2023 10:32, Marco Elver wrote:
>> On Tue, 12 Dec 2023 at 10:19, Paul Heidekrüger <[email protected]> wrote:
>>>
>>> On 12.12.2023 00:37, Andrey Konovalov wrote:
>>>> On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger
>>>> <[email protected]> wrote:
>>>>>
>>>>> 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?
>>>>
>>>> No, I just run the kernel built with a config file that I put together
>>>> based on defconfig.
>>>
>>> Ah. I believe I've figured it out.
>>>
>>> When I add CONFIG_STACK_TRACER=y in addition to CONFIG_FTRACE=y, it works.
>>
>> CONFIG_FTRACE should be enough - maybe also check x86 vs. arm64 to debug more.
>
> 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

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