[PATCH v3 0/3] Add error_report_end tracepoint to KFENCE and KASAN

[PATCH v3 0/3] Add error_report_end tracepoint to KFENCE and KASAN

[Alexander Potapenko]

This patchset adds a tracepoint, error_repor_end, that is to be used by
KFENCE, KASAN, and potentially other bug detection tools, when they
print an error report.
One of the possible use cases is userspace collection of kernel error
reports: interested parties can subscribe to the tracing event via
tracefs, and get notified when an error report occurs.

v3:
 - dropped the sysfs interface for log collection
 - dropped error_report_start tracepoint

v2:
 - added ABI documentation for /sys/kernel/error_report/
 - changed error_report_start and error_report end tracepoints to take
   a fixed set of values for the error detector


Alexander Potapenko (3):
  tracing: add error_report_end trace point
  kfence: use error_report_end tracepoint
  kasan: use error_report_end tracepoint

 include/trace/events/error_report.h | 74 +++++++++++++++++++++++++++++
 kernel/trace/Makefile               |  1 +
 kernel/trace/error_report-traces.c  | 12 +++++
 mm/kasan/report.c                   |  8 ++--
 mm/kfence/report.c                  |  2 +
 5 files changed, 94 insertions(+), 3 deletions(-)
 create mode 100644 include/trace/events/error_report.h
 create mode 100644 kernel/trace/error_report-traces.c

-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v3 1/3] tracing: add error_report_end trace point

[Alexander Potapenko]

Introduce error_report_end tracepoint. It can be used in debugging tools
like KASAN, KFENCE, etc. to provide extensions to the error reporting
mechanisms (e.g. allow tests hook into error reporting, ease error report
collection from production kernels).
Another benefit would be making use of ftrace for debugging or
benchmarking the tools themselves.

Should we need it, the tracepoint name leaves us with the possibility to
introduce a complementary error_report_start tracepoint in the future.

Suggested-by: Marco Elver <elver@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Marco Elver <elver@google.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: linux-mm@kvack.org
Cc: linux-api@vger.kernel.org
Signed-off-by: Alexander Potapenko <glider@google.com>

---
v3: - delete error_report_start that is unlikely to be unused in
      the nearest future
    - add missing copyright headers

v2: - change error_report_start and error_report_end prototypes
      to accept enum error_detector instead of char*
      (as suggested by Steven Rostedt)
---
 include/trace/events/error_report.h | 74 +++++++++++++++++++++++++++++
 kernel/trace/Makefile               |  1 +
 kernel/trace/error_report-traces.c  | 12 +++++
 3 files changed, 87 insertions(+)
 create mode 100644 include/trace/events/error_report.h
 create mode 100644 kernel/trace/error_report-traces.c

diff --git a/include/trace/events/error_report.h b/include/trace/events/error_report.h
new file mode 100644
index 000000000000..96f64bf218b2
--- /dev/null
+++ b/include/trace/events/error_report.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Declarations for error reporting tracepoints.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM error_report
+
+#if !defined(_TRACE_ERROR_REPORT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_ERROR_REPORT_H
+
+#include <linux/tracepoint.h>
+
+#ifndef __ERROR_REPORT_DECLARE_TRACE_ENUMS_ONCE_ONLY
+#define __ERROR_REPORT_DECLARE_TRACE_ENUMS_ONCE_ONLY
+
+enum error_detector {
+	ERROR_DETECTOR_KFENCE,
+	ERROR_DETECTOR_KASAN
+};
+
+#endif /* __ERROR_REPORT_DECLARE_TRACE_ENUMS_ONCE_ONLY */
+
+#define error_detector_list	\
+	EM(ERROR_DETECTOR_KFENCE, "kfence")	\
+	EMe(ERROR_DETECTOR_KASAN, "kasan")
+/* Always end the list with an EMe. */
+
+#undef EM
+#undef EMe
+
+#define EM(a, b)	TRACE_DEFINE_ENUM(a);
+#define EMe(a, b)	TRACE_DEFINE_ENUM(a);
+
+error_detector_list
+
+#undef EM
+#undef EMe
+
+#define EM(a, b) { a, b },
+#define EMe(a, b) { a, b }
+
+#define show_error_detector_list(val) \
+	__print_symbolic(val, error_detector_list)
+
+DECLARE_EVENT_CLASS(error_report_template,
+		    TP_PROTO(enum error_detector error_detector, unsigned long id),
+		    TP_ARGS(error_detector, id),
+		    TP_STRUCT__entry(__field(enum error_detector, error_detector)
+					     __field(unsigned long, id)),
+		    TP_fast_assign(__entry->error_detector = error_detector;
+				   __entry->id = id;),
+		    TP_printk("[%s] %lx",
+			      show_error_detector_list(__entry->error_detector),
+			      __entry->id));
+
+/**
+ * error_report_end - called after printing the error report
+ * @error_detector:	short string describing the error detection tool
+ * @id:			pseudo-unique descriptor identifying the report
+ *			(e.g. the memory access address)
+ *
+ * This event occurs right after a debugging tool finishes printing the error
+ * report.
+ */
+DEFINE_EVENT(error_report_template, error_report_end,
+	     TP_PROTO(enum error_detector error_detector, unsigned long id),
+	     TP_ARGS(error_detector, id));
+
+#endif /* _TRACE_ERROR_REPORT_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 7e44cea89fdc..b28d3e5013cd 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_SYNTH_EVENTS) += trace_events_synth.o
 obj-$(CONFIG_HIST_TRIGGERS) += trace_events_hist.o
 obj-$(CONFIG_BPF_EVENTS) += bpf_trace.o
 obj-$(CONFIG_KPROBE_EVENTS) += trace_kprobe.o
+obj-$(CONFIG_TRACEPOINTS) += error_report-traces.o
 obj-$(CONFIG_TRACEPOINTS) += power-traces.o
 ifeq ($(CONFIG_PM),y)
 obj-$(CONFIG_TRACEPOINTS) += rpm-traces.o
diff --git a/kernel/trace/error_report-traces.c b/kernel/trace/error_report-traces.c
new file mode 100644
index 000000000000..632c8c7ff079
--- /dev/null
+++ b/kernel/trace/error_report-traces.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Error reporting trace points.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/error_report.h>
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(error_report_end);
+
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH 2/3] kfence: use error_report_end tracepoint

[Alexander Potapenko]

Make it possible to trace KFENCE error reporting. A good usecase is
watching for trace events from the userspace to detect and process
memory corruption reports from the kernel.

Suggested-by: Marco Elver <elver@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Marco Elver <elver@google.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: linux-mm@kvack.org
Signed-off-by: Alexander Potapenko <glider@google.com>

---
v3:
 - dropped error_report_start

v2:
 - change error_report_start and error_report_end prototypes
   to accept enum error_detector instead of char*
   (as suggested by Steven Rostedt)
---
 mm/kfence/report.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/kfence/report.c b/mm/kfence/report.c
index 901bd7ee83d8..f9fc93b2fe2e 100644
--- a/mm/kfence/report.c
+++ b/mm/kfence/report.c
@@ -14,6 +14,7 @@
 #include <linux/seq_file.h>
 #include <linux/stacktrace.h>
 #include <linux/string.h>
+#include <trace/events/error_report.h>
 
 #include <asm/kfence.h>
 
@@ -246,6 +247,7 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r
 		show_regs(regs);
 	else
 		dump_stack_print_info(KERN_ERR);
+	trace_error_report_end(ERROR_DETECTOR_KFENCE, address);
 	pr_err("==================================================================\n");
 
 	lockdep_on();
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH 3/3] kasan: use error_report_end tracepoint

[Alexander Potapenko]

Make it possible to trace KASAN error reporting. A good usecase is
watching for trace events from the userspace to detect and process
memory corruption reports from the kernel.

Suggested-by: Marco Elver <elver@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Marco Elver <elver@google.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: linux-mm@kvack.org
Signed-off-by: Alexander Potapenko <glider@google.com>

---
v3:
 - dropped error_report_start

v2:
 - change error_report_start and error_report_end prototypes
   to accept enum error_detector instead of char*
   (as suggested by Steven Rostedt)
---
 mm/kasan/report.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 234f35a84f19..87b271206163 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/sched/task_stack.h>
 #include <linux/uaccess.h>
+#include <trace/events/error_report.h>
 
 #include <asm/sections.h>
 
@@ -84,8 +85,9 @@ static void start_report(unsigned long *flags)
 	pr_err("==================================================================\n");
 }
 
-static void end_report(unsigned long *flags)
+static void end_report(unsigned long *flags, unsigned long addr)
 {
+	trace_error_report_end(ERROR_DETECTOR_KASAN, addr);
 	pr_err("==================================================================\n");
 	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
 	spin_unlock_irqrestore(&report_lock, *flags);
@@ -355,7 +357,7 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
 	print_address_description(object, tag);
 	pr_err("\n");
 	print_memory_metadata(object);
-	end_report(&flags);
+	end_report(&flags, (unsigned long)object);
 }
 
 static void __kasan_report(unsigned long addr, size_t size, bool is_write,
@@ -401,7 +403,7 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 		dump_stack();
 	}
 
-	end_report(&flags);
+	end_report(&flags, addr);
 }
 
 bool kasan_report(unsigned long addr, size_t size, bool is_write,
-- 
2.30.0.296.g2bfb1c46d8-goog

Re: [PATCH v3 0/3] Add error_report_end tracepoint to KFENCE and KASAN

[Petr Mladek]

On Thu 2021-01-21 14:19:12, Alexander Potapenko wrote:
> This patchset adds a tracepoint, error_repor_end, that is to be used by
> KFENCE, KASAN, and potentially other bug detection tools, when they
> print an error report.
> One of the possible use cases is userspace collection of kernel error
> reports: interested parties can subscribe to the tracing event via
> tracefs, and get notified when an error report occurs.
> 
> v3:
>  - dropped the sysfs interface for log collection
>  - dropped error_report_start tracepoint

Just for record. This approach looks reasonable to me.
Thanks for removing the sysfs interface. It would have been
a potential can of worms.

I do not feel eligible to add any tag. I am neither familiar
with the tracepoint nor the KFENCE or KASAN code.

Best Regards,
Petr

Re: [PATCH v3 0/3] Add error_report_end tracepoint to KFENCE and KASAN

[Alexander Potapenko]

On Fri, Jan 22, 2021 at 9:32 AM Petr Mladek <pmladek@suse.com> wrote:
>
> On Thu 2021-01-21 14:19:12, Alexander Potapenko wrote:
> > This patchset adds a tracepoint, error_repor_end, that is to be used by
> > KFENCE, KASAN, and potentially other bug detection tools, when they
> > print an error report.
> > One of the possible use cases is userspace collection of kernel error
> > reports: interested parties can subscribe to the tracing event via
> > tracefs, and get notified when an error report occurs.
> >
> > v3:
> >  - dropped the sysfs interface for log collection
> >  - dropped error_report_start tracepoint
>
> Just for record. This approach looks reasonable to me.
> Thanks for removing the sysfs interface. It would have been
> a potential can of worms.

Thanks for the input!
At least it was premature to touch prink just for the sake of
collecting some particular crashes.
Perhaps we can revisit this topic when stronger arguments arise :)

> I do not feel eligible to add any tag. I am neither familiar
> with the tracepoint nor the KFENCE or KASAN code.
>
> Best Regards,
> Petr



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Re: [PATCH v3 0/3] Add error_report_end tracepoint to KFENCE and KASAN

[Petr Mladek]

On Fri 2021-01-22 09:41:54, Alexander Potapenko wrote:
> On Fri, Jan 22, 2021 at 9:32 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> > On Thu 2021-01-21 14:19:12, Alexander Potapenko wrote:
> > > This patchset adds a tracepoint, error_repor_end, that is to be used by
> > > KFENCE, KASAN, and potentially other bug detection tools, when they
> > > print an error report.
> > > One of the possible use cases is userspace collection of kernel error
> > > reports: interested parties can subscribe to the tracing event via
> > > tracefs, and get notified when an error report occurs.
> > >
> > > v3:
> > >  - dropped the sysfs interface for log collection
> > >  - dropped error_report_start tracepoint
> >
> > Just for record. This approach looks reasonable to me.
> > Thanks for removing the sysfs interface. It would have been
> > a potential can of worms.
> 
> Thanks for the input!
> At least it was premature to touch prink just for the sake of
> collecting some particular crashes.
> Perhaps we can revisit this topic when stronger arguments arise :)

Sure :-)

Best Regards,
Petr