linux-kernel - [PATCH] tracing: silence GCC 9 array bounds warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 May 2019 11:25:02 +0200
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>
Cc:     linux-kernel@...r.kernel.org
Subject: [PATCH] tracing: silence GCC 9 array bounds warning

Starting with GCC 9, -Warray-bounds detects cases when memset is called
starting on a member of a struct but the size to be cleared ends up
writing over further members.

Such a call happens in the trace code to clear, at once, all members
after and including `seq` on struct trace_iterator:

    In function 'memset',
        inlined from 'ftrace_dump' at kernel/trace/trace.c:8914:3:
    ./include/linux/string.h:344:9: warning: '__builtin_memset' offset
    [8505, 8560] from the object at 'iter' is out of the bounds of
    referenced subobject 'seq' with type 'struct trace_seq' at offset
    4368 [-Warray-bounds]
      344 |  return __builtin_memset(p, c, size);
          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

In order to avoid GCC complaining about it, we compute the address
ourselves by adding the offsetof distance instead of referring
directly to the member.

Since there are two places doing this clear (trace.c and trace_kdb.c),
take the chance to move the workaround into a single place in
the internal header.

Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
---
 kernel/trace/trace.c     |  7 +------
 kernel/trace/trace.h     | 14 ++++++++++++++
 kernel/trace/trace_kdb.c |  7 +------
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ca1ee656d6d8..37990532351b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8627,12 +8627,7 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)
 
 		cnt++;
 
-		/* reset all but tr, trace, and overruns */
-		memset(&iter.seq, 0,
-		       sizeof(struct trace_iterator) -
-		       offsetof(struct trace_iterator, seq));
-		iter.iter_flags |= TRACE_FILE_LAT_FMT;
-		iter.pos = -1;
+		trace_iterator_reset(&iter);
 
 		if (trace_find_next_entry_inc(&iter) != NULL) {
 			int ret;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index d80cee49e0eb..80ad656f43eb 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1964,4 +1964,18 @@ static inline void tracer_hardirqs_off(unsigned long a0, unsigned long a1) { }
 
 extern struct trace_iterator *tracepoint_print_iter;
 
+/* reset all but tr, trace, and overruns */
+static __always_inline void trace_iterator_reset(struct trace_iterator * iter)
+{
+	/*
+	 * Equivalent to &iter->seq, but avoids GCC 9 complaining about
+	 * overwriting more members than just iter->seq (-Warray-bounds)
+	 */
+	memset((char *)(iter) + offsetof(struct trace_iterator, seq), 0,
+	       sizeof(struct trace_iterator) -
+	       offsetof(struct trace_iterator, seq));
+	iter->iter_flags |= TRACE_FILE_LAT_FMT;
+	iter->pos = -1;
+}
+
 #endif /* _LINUX_KERNEL_TRACE_H */
diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c
index 810d78a8d14c..0a2a166ee716 100644
--- a/kernel/trace/trace_kdb.c
+++ b/kernel/trace/trace_kdb.c
@@ -41,12 +41,7 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file)
 
 	kdb_printf("Dumping ftrace buffer:\n");
 
-	/* reset all but tr, trace, and overruns */
-	memset(&iter.seq, 0,
-		   sizeof(struct trace_iterator) -
-		   offsetof(struct trace_iterator, seq));
-	iter.iter_flags |= TRACE_FILE_LAT_FMT;
-	iter.pos = -1;
+	trace_iterator_reset(&iter);
 
 	if (cpu_file == RING_BUFFER_ALL_CPUS) {
 		for_each_tracing_cpu(cpu) {
-- 
2.17.1