linux-kernel - Re: [GIT PULL] ftrace: Fixes for v6.13

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wh3cUC2a=yJv42HTjDLCp6VM+GTky+q65vV_Q33BeoxAg@mail.gmail.com>
Date: Sat, 14 Dec 2024 20:11:50 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 
	Al Viro <viro@...iv.linux.org.uk>, Michal Simek <monstr@...str.eu>
Subject: Re: [GIT PULL] ftrace: Fixes for v6.13

On Sat, 14 Dec 2024 at 20:06, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> And I think test_event_printk() could probably be expanded to handle
> at least some cases of '%s' (because, as you say, pointers to various
> static allocations are fine for it).

.. and I think we can at least protect against kernel faults in the
generic '%s' handling. We already do some of that in
check_pointer_msg, and it could be expanded upon with something like

  --- a/lib/vsprintf.c
  +++ b/lib/vsprintf.c
  @@ -695,13 +695,22 @@ static char *error_string(char *buf, ...
    */
   static const char *check_pointer_msg(const void *ptr)
   {
  +       unsigned char val;
  +
          if (!ptr)
                  return "(null)";

          if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr))
                  return "(efault)";

  +       pagefault_disable();
  +       __get_kernel_nofault(&val, (const char *)ptr, u8, Efault);
  +       pagefault_enable();
          return NULL;
  +
  +Efault:
  +       pagefault_enable();
  +       return "(bad address)";
   }

which is admittedly

 (a) ENTIRELY UNTESTED
 (b) not exactly pretty
 (c) only handles the first byte of the string

but it's at least very simple and cheap and fits in with the checks we
already do.

No, that vsprintf code wouldn't deal with any tracing-specific sanity
checks that say "the pointer is in the trace ring buffer", but as
mentioned, *that* code is entirely separate from the thing I'm
objecting to anyway.

               Linus