linux-kernel - Re: printk: what is going on with additional newlines?

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]

Date:   Wed, 30 Aug 2017 02:33:14 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Pavel Machek <pavel@....cz>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>, Jan Kara <jack@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jiri Slaby <jslaby@...e.com>, Andreas Mohr <andi@...as.de>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: printk: what is going on with additional newlines?

Hello,

On (08/29/17 10:00), Linus Torvalds wrote:
> On Tue, Aug 29, 2017 at 6:40 AM, Sergey Senozhatsky
> <sergey.senozhatsky.work@...il.com> wrote:
> > Pavel reported that
> >         printk("foo"); printk("bar");
> >
> > now does not produce a single continuation "foobar" line, but
> > instead produces two lines
> >                 foo
> >                 bar
> 
> And that's the *correct* behavior.

ok. thanks for taking a look.

> Stop trying to fix that. Fix the printk's instead.
> 
> In particular, the
> 
>     printk("bar");
> 
> could have come from an interrupt, and have nothing what-so-ever to do
> with "foo".
> 
> If you want continuations, you
> 
>  (a) make sure the first one doesn't end in a newline
> 
>  (b) make sure the second printk has a KERN_CONT
> 
>  (c) even after that, ask yourself how much you _really_ want
> continuations, because there are going to be situations where it still
> doesn't work.

yes, continuations are not really welcomed. I thought that this
particular case could be considered a regression. but your position
is pretty clear.

> I refuse to help those things. We mis-designed things, and the
> continuations were a mistake to begin with, but they were a mistake
> that was understandable in the timeframe they happened. But it's not
> something we should support, and it's most definitely is not something
> we should then say "oh, you were broken shit that didn't even bother
> to add the KERN_CONT, let me help your crap".
> 
> No.
> 
> Only acceptable use of continuations is basically boot-time testing,
> when you do things like
> 
>      printk("Testing feature XYZ..");
>      this_may_blow_up_because_of_hw_bugs();
>      printk(KERN_CONT " ... ok\n");
>
> and anything else you should seriously try to marshal the data
> *before* doing a printk(), and not expect printk() to marshal it for
> you.

ok. that's something several people asked for -- some sort of buffered
printk mode; but people don't want to use a buffer allocated on the stack
(or kmalloc-ed, etc.) to do sprintf() on it and then feed it to printk("%s"),
because this adds some extra cost:

	void foo(void)
	{
		char cont_string[256];
		size_t sz;

		sz = sprintf(cont_string + sz, "%xxxx", data1...);
		do_abc()
		sz += sprintf(cont_string + sz, "%xxxx", data1...);

		....

		printk("%s\n", cont_string)   // does "sprintf" again
					      // and then memcpy
	}


I thought about re-using printk-safe per-CPU buffers for that purpose.
this saves us memory, because printk-safe buffers are always there, but
it has some disadvantages. namely, to use printk-safe buffer we need to
disable local interrupts. so something like this

	printk_buffered_mode_begin();   // disables local irq

	printk()	// appends data to the per-CPU buffer
	printk()
	printk()

	printk_buffered_mode_end();  // append messages to consequent logbuf
				     // entries
				     // enable local irqs.

... not sure, how usable this will end up to be.
probably not usable at all.

> But for legacy reasons, we do end up trying to support KERN_CONT.
> Just barely.
> 
> I'd really like to get rid of it entirely, because the whole log-based
> structure really really doesn't work well for it (what if somebody has
> already read the partial line from the logs?)
> 
> Our printk stuff didn't used to be log-based. It was just a plain
> character-based circular buffer. Back then that KERN_CONT made a whole
> lot more sense.

	-ss