linux-kernel - Re: [PATCH] printk: inject caller information into the body of message

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:   Mon, 24 Sep 2018 17:11:22 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitriy Vyukov <dvyukov@...gle.com>,
        kbuild test robot <fengguang.wu@...el.com>,
        syzkaller <syzkaller@...glegroups.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] printk: inject caller information into the body of
 message

On 2018/09/19 20:02, Tetsuo Handa wrote:
> On 2018/09/14 21:22, Sergey Senozhatsky wrote:
>> The "SMP-safe" comment becomes a bit tricky when pr_line is used with a
>> static buffer. Either we need to require synchronization - umm... and
>> document it - or to provide some means of synchronization in pr_line().
>> Let's think what pr_line API should do about it.
>>
>> Any thoughts?
>>
> 
> I'm inclined to propose a simple one shown below, similar to just having
> several "struct cont" for concurrent printk() users.
> What Linus has commented is that implicit context is bad, and below one
> uses explicit context.
> After almost all users are converted to use below one, we might be able
> to get rid of KERN_CONT support.

The reason of using statically preallocated global buffers is that I think
that it is inconvenient for KERN_CONT users to calculate necessary bytes
only for avoiding message truncation. The pr_line might be passed to deep
into the callchain and adjusting buffer size whenever the content's possible
max length changes is as much painful as changing printk() to accept only
one "const char *" argument. Even if we guarantee that any context can
allocate buffer from kernel stack, we cannot guarantee that many concurrent
printk() won't trigger lockup. Thus, I think that trying to allocate from
finite static buffers with a fallback to unbuffered printk() upon failure
is sufficient.



By the way, kbuild test robot told me that I forgot to drop asmlinkage keyword.

 include/linux/printk.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 889491b..3347442 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -178,7 +178,7 @@ asmlinkage __printf(1, 2) __cold
 void flush_printk_buffer(struct printk_buffer *ptr);
 __printf(2, 3)
 int buffered_printk(struct printk_buffer *ptr, const char *fmt, ...);
-asmlinkage __printf(2, 0)
+__printf(2, 0)
 int buffered_vprintk(struct printk_buffer *ptr, const char *fmt, va_list args);
 void put_printk_buffer(struct printk_buffer *ptr);