linux-kernel - Re: [PATCH next v1 1/3] printk: track/limit recursion

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: <YFh770T97jwQLRKu@google.com>
Date:   Mon, 22 Mar 2021 20:13:51 +0900
From:   Sergey Senozhatsky <senozhatsky@...omium.org>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH next v1 1/3] printk: track/limit recursion

On (21/03/22 11:53), John Ogness wrote:
> On 2021-03-21, Sergey Senozhatsky <senozhatsky@...omium.org> wrote:
> >> @@ -2055,6 +2122,9 @@ int vprintk_store(int facility, int level,
> >>  	 */
> >>  	ts_nsec = local_clock();
> >>  
> >> +	if (!printk_enter_irqsave(&irqflags))
> >> +		return 0;
> >
> > I guess it can be interesting to somehow signal us that we had
> > printk() recursion overflow, and how many messages we lost.
> 
> Honestly, if we hit 3 levels of recursion, we are probably dealing with
> an infinite recursion issue.

I tend to agree.

> I do not see the value of counting the overflows in that case. The
> logged messages at that recursion level would ben enough to point
> us to the problem.
>
> > 3 levels of recursion seem like reasonable limit, but I maybe wouldn't
> > mind one extra level.
>
> With 3 levels, we will see all the messages of:
>
>     printk -> WARN_ON -> WARN_ON -> WARN_ON

Well, not necessarily this simple.

printk
 vsprintf
  handle_foo_specifier
   printk
    call_console_drivers
     timekeeping
      printk
       vsprintf

We saw in the past that enabling CONFIG_DEBUG_OBJECTS (if I'm not
mistaken) can add quite a bit of extra printk recursion paths.

We also have other CONFIG_DEBUG_* config options that can pop up as
recursive printk-s here and there. For instance, from vsprintf::foo_specifier()
where we escape from printk() to various kernel subsystems: net, block,
etc.

Maybe sometimes on level 3+ we'll see something interesting,
but I've no strong opinion on this.

> Keep in mind that each additional level causes the reading of the logs
> to be significantly more complex. Each level increases the output
> exponentially:

Yes, I realize that. That's why I suggested that maybe recursive
printk-s can have some special extra prefix. Recursive printk-s
will interleave with whatever is being printed on this_cpu, so
prefix might be helpful.

	-ss