2020-04-10 16:45:35

by Marco Elver

Subject: [PATCH 1/2] kcsan: Fix function matching in report

Pass string length as returned by scnprintf() to strnstr(), since
strnstr() searches exactly len bytes in haystack, even if it contains a
NUL-terminator before haystack+len.

Signed-off-by: Marco Elver <[email protected]>
---
kernel/kcsan/report.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c
index ddc18f1224a4..cf41d63dd0cd 100644
--- a/kernel/kcsan/report.c
+++ b/kernel/kcsan/report.c
@@ -192,11 +192,11 @@ skip_report(enum kcsan_value_change value_change, unsigned long top_frame)
* maintainers.
*/
char buf[64];
+ int len = scnprintf(buf, sizeof(buf), "%ps", (void *)top_frame);

- snprintf(buf, sizeof(buf), "%ps", (void *)top_frame);
- if (!strnstr(buf, "rcu_", sizeof(buf)) &&
- !strnstr(buf, "_rcu", sizeof(buf)) &&
- !strnstr(buf, "_srcu", sizeof(buf)))
+ if (!strnstr(buf, "rcu_", len) &&
+ !strnstr(buf, "_rcu", len) &&
+ !strnstr(buf, "_srcu", len))
return true;
}

@@ -262,15 +262,15 @@ static const char *get_thread_desc(int task_id)
static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries)
{
char buf[64];
+ int len;
int skip = 0;

for (; skip < num_entries; ++skip) {
- snprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skip]);
- if (!strnstr(buf, "csan_", sizeof(buf)) &&
- !strnstr(buf, "tsan_", sizeof(buf)) &&
- !strnstr(buf, "_once_size", sizeof(buf))) {
+ len = scnprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skip]);
+ if (!strnstr(buf, "csan_", len) &&
+ !strnstr(buf, "tsan_", len) &&
+ !strnstr(buf, "_once_size", len))
break;
- }
}
return skip;
}
--
2.26.0.110.g2183baf09c-goog

2020-04-10 16:46:44

by Marco Elver

Subject: [PATCH 2/2] kcsan: Make reporting aware of KCSAN tests

Reporting hides KCSAN runtime functions in the stack trace, with
filtering done based on function names. Currently this included all
functions (or modules) that would match "kcsan_". Make the filter aware
of KCSAN tests, which contain "kcsan_test", and are no longer skipped in
the report.

This is in preparation for adding a KCSAN test module.

Signed-off-by: Marco Elver <[email protected]>
---
kernel/kcsan/report.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c
index cf41d63dd0cd..ac5f8345bae9 100644
--- a/kernel/kcsan/report.c
+++ b/kernel/kcsan/report.c
@@ -262,16 +262,32 @@ static const char *get_thread_desc(int task_id)
static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries)
{
char buf[64];
- int len;
- int skip = 0;
+ char *cur;
+ int len, skip;

- for (; skip < num_entries; ++skip) {
+ for (skip = 0; skip < num_entries; ++skip) {
len = scnprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skip]);
- if (!strnstr(buf, "csan_", len) &&
- !strnstr(buf, "tsan_", len) &&
- !strnstr(buf, "_once_size", len))
- break;
+
+ /* Never show tsan_* or {read,write}_once_size. */
+ if (strnstr(buf, "tsan_", len) ||
+ strnstr(buf, "_once_size", len))
+ continue;
+
+ cur = strnstr(buf, "kcsan_", len);
+ if (cur) {
+ cur += sizeof("kcsan_") - 1;
+ if (strncmp(cur, "test", sizeof("test") - 1))
+ continue; /* KCSAN runtime function. */
+ /* KCSAN related test. */
+ }
+
+ /*
+ * No match for runtime functions -- @skip entries to skip to
+ * get to first frame of interest.
+ */
+ break;
}
+
return skip;
}

--
2.26.0.110.g2183baf09c-goog

2020-04-10 20:39:26

by Paul E. McKenney

Subject: Re: [PATCH 1/2] kcsan: Fix function matching in report

On Fri, Apr 10, 2020 at 06:44:17PM +0200, Marco Elver wrote:
> Pass string length as returned by scnprintf() to strnstr(), since
> strnstr() searches exactly len bytes in haystack, even if it contains a
> NUL-terminator before haystack+len.
>
> Signed-off-by: Marco Elver <[email protected]>

I queued both for testing and review, thank you, Marco!

Thanx, Paul

> ---
> kernel/kcsan/report.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c
> index ddc18f1224a4..cf41d63dd0cd 100644
> --- a/kernel/kcsan/report.c
> +++ b/kernel/kcsan/report.c
> @@ -192,11 +192,11 @@ skip_report(enum kcsan_value_change value_change, unsigned long top_frame)
> * maintainers.
> */
> char buf[64];
> + int len = scnprintf(buf, sizeof(buf), "%ps", (void *)top_frame);
>
> - snprintf(buf, sizeof(buf), "%ps", (void *)top_frame);
> - if (!strnstr(buf, "rcu_", sizeof(buf)) &&
> - !strnstr(buf, "_rcu", sizeof(buf)) &&
> - !strnstr(buf, "_srcu", sizeof(buf)))
> + if (!strnstr(buf, "rcu_", len) &&
> + !strnstr(buf, "_rcu", len) &&
> + !strnstr(buf, "_srcu", len))
> return true;
> }
>
> @@ -262,15 +262,15 @@ static const char *get_thread_desc(int task_id)
> static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries)
> {
> char buf[64];
> + int len;
> int skip = 0;
>
> for (; skip < num_entries; ++skip) {
> - snprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skip]);
> - if (!strnstr(buf, "csan_", sizeof(buf)) &&
> - !strnstr(buf, "tsan_", sizeof(buf)) &&
> - !strnstr(buf, "_once_size", sizeof(buf))) {
> + len = scnprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skip]);
> + if (!strnstr(buf, "csan_", len) &&
> + !strnstr(buf, "tsan_", len) &&
> + !strnstr(buf, "_once_size", len))
> break;
> - }
> }
> return skip;
> }
> --
> 2.26.0.110.g2183baf09c-goog
>