linux-kernel - [PATCH] tracing: Add __string_src() helper to help compilers not to get confused

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: Thu, 14 Mar 2024 23:27:54 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: LKML <linux-kernel@...r.kernel.org>, Linux trace kernel
 <linux-trace-kernel@...r.kernel.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Alison Schofield
 <alison.schofield@...el.com>
Subject: [PATCH] tracing: Add __string_src() helper to help compilers not to
 get confused

From: "Steven Rostedt (Google)" <rostedt@...dmis.org>

The __string() helper macro of the TRACE_EVENT() macro is used to
determine how much of the ring buffer needs to be allocated to fit the
given source string. Some trace events have a string that is dependent on
another variable that could be NULL, and in those cases the string is
passed in to be NULL.

The __string() macro can handle being passed in a NULL pointer for which
it will turn it into "(null)". It does that with:

  strlen((src) ? (const char *)(src) : "(null)") + 1

But if src itself has the same conditional type it can confuse the
compiler. That is:

  __string(r ? dev(r)->name : NULL)

Would turn into:

 strlen((r ? dev(r)->name : NULL) ? (r ? dev(r)->name : NULL) : "(null)" + 1

For which the compiler thinks that NULL is being passed to strlen() and
gives this kind of warning:

/include/trace/stages/stage5_get_offsets.h:50:21: warning: argument 1 null where non-null expected [-Wnonnull]
   50 |                     strlen((src) ? (const char *)(src) : "(null)") + 1)

Instead, create a static inline function that takes the src string and
will return the string if it is not NULL and will return "(null)" if it
is. This will then make the strlen() line:

 strlen(__string_src(src)) + 1

Where the compiler can see that strlen() will not end up with NULL and
does not warn about it.

Note that this depends on commit 51270d573a8d ("tracing/net_sched: Fix
tracepoints that save qdisc_dev() as a string") being applied, as passing
the qdisc_dev() into __string_src() will give an error.

Link: https://lore.kernel.org/all/ZfNmfCmgCs4Nc+EH@aschofie-mobl2/

Reported-by: Alison Schofield <alison.schofield@...el.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
---
 include/trace/stages/stage5_get_offsets.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/trace/stages/stage5_get_offsets.h b/include/trace/stages/stage5_get_offsets.h
index e6b96608f452..c6a62dfb18ef 100644
--- a/include/trace/stages/stage5_get_offsets.h
+++ b/include/trace/stages/stage5_get_offsets.h
@@ -9,6 +9,16 @@
 #undef __entry
 #define __entry entry

+#ifndef __STAGE5_STRING_SRC_H
+#define __STAGE5_STRING_SRC_H
+static inline const char *__string_src(const char *str)
+{
+       if (!str)
+	       return EVENT_NULL_STR;
+       return str;
+}
+#endif /* __STAGE5_STRING_SRC_H */
+
 /*
  * Fields should never declare an array: i.e. __field(int, arr[5])
  * If they do, it will cause issues in parsing and possibly corrupt the
@@ -47,7 +57,7 @@

 #undef __string
 #define __string(item, src) __dynamic_array(char, item,			\
-		    strlen((const char *)(src) ? : EVENT_NULL_STR) + 1)	\
+		    strlen(__string_src(src)) + 1)			\
 	__data_offsets->item##_ptr_ = src;

 #undef __string_len
@@ -70,7 +80,7 @@

 #undef __rel_string
 #define __rel_string(item, src) __rel_dynamic_array(char, item,		\
-		    strlen((const char *)(src) ? : EVENT_NULL_STR) + 1)	\
+		    strlen(__string_src(src)) + 1)			\
 	__data_offsets->item##_ptr_ = src;

 #undef __rel_string_len
-- 
2.43.0