linux-kernel - [GIT PULL] tracing: Have trace event string test handle zero length strings

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]

Message-ID: <20220324190339.399932f7@gandalf.local.home>
Date:   Thu, 24 Mar 2022 19:03:39 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Brian Foster <bfoster@...hat.com>
Subject: [GIT PULL] tracing: Have trace event string test handle zero length
 strings



Linus,

Trace event fix of string verifier

- The run time string verifier that checks all trace event formats
  as they are read from the tracing file to make sure that the %s
  pointers are not reading something that no longer exists, failed
  to account for %*.s where the length given is zero, and the string
  is NULL. It incorrectly flagged it as a null pointer dereference and
  gave a WARN_ON().


Please pull the latest trace-v5.18-1 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v5.18-1

Tag SHA1: 2eb99512607710b43f383ddd44fd009827c8460e
Head SHA1: eca344a7362e0f34f179298fd8366bcd556eede1


Steven Rostedt (Google) (1):
      tracing: Have trace event string test handle zero length strings

----
 kernel/trace/trace.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
---------------------------
commit eca344a7362e0f34f179298fd8366bcd556eede1
Author: Steven Rostedt (Google) <rostedt@...dmis.org>
Date:   Wed Mar 23 10:32:51 2022 -0400

    tracing: Have trace event string test handle zero length strings
    
    If a trace event has in its TP_printk():
    
     "%*.s", len, len ? __get_str(string) : NULL
    
    It is perfectly valid if len is zero and passing in the NULL.
    Unfortunately, the runtime string check at time of reading the trace sees
    the NULL and flags it as a bad string and produces a WARN_ON().
    
    Handle this case by passing into the test function if the format has an
    asterisk (star) and if so, if the length is zero, then mark it as safe.
    
    Link: https://lore.kernel.org/all/YjsWzuw5FbWPrdqq@bfoster/
    
    Cc: stable@...r.kernel.org
    Reported-by: Brian Foster <bfoster@...hat.com>
    Tested-by: Brian Foster <bfoster@...hat.com>
    Fixes: 9a6944fee68e2 ("tracing: Add a verifier to check string pointers for trace events")
    Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index eb44418574f9..96265a717ca4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3663,12 +3663,17 @@ static char *trace_iter_expand_format(struct trace_iterator *iter)
 }
 
 /* Returns true if the string is safe to dereference from an event */
-static bool trace_safe_str(struct trace_iterator *iter, const char *str)
+static bool trace_safe_str(struct trace_iterator *iter, const char *str,
+			   bool star, int len)
 {
 	unsigned long addr = (unsigned long)str;
 	struct trace_event *trace_event;
 	struct trace_event_call *event;
 
+	/* Ignore strings with no length */
+	if (star && !len)
+		return true;
+
 	/* OK if part of the event data */
 	if ((addr >= (unsigned long)iter->ent) &&
 	    (addr < (unsigned long)iter->ent + iter->ent_size))
@@ -3854,7 +3859,7 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt,
 		 * instead. See samples/trace_events/trace-events-sample.h
 		 * for reference.
 		 */
-		if (WARN_ONCE(!trace_safe_str(iter, str),
+		if (WARN_ONCE(!trace_safe_str(iter, str, star, len),
 			      "fmt: '%s' current_buffer: '%s'",
 			      fmt, show_buffer(&iter->seq))) {
 			int ret;