linux-kernel - Re: [PATCH] twist: allow converting pr_devel()/pr_debug() into printk(KERN

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: <2818b93e-3ebb-72ce-feb9-b0768ccb60d8@i-love.sakura.ne.jp>
Date:   Thu, 28 May 2020 23:13:17 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, Dmitry Vyukov <dvyukov@...gle.com>,
        Ondrej Mosnacek <omosnace@...hat.com>,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH] twist: allow converting pr_devel()/pr_debug() into
 printk(KERN_DEBUG)

On 2020/05/28 21:14, Petr Mladek wrote:
>> how to handle
>>
>>>>   #define no_printk(fmt, ...)                             \
>>>>   ({                                                      \
>>>>           if (0)                                          \
>>>>                   printk(fmt, ##__VA_ARGS__);             \
>>>>           0;                                              \
>>>>   })
>>
>> part used by e.g. pr_devel() ? Since this macro is not using dynamic debug
>> interface, vprintk_store() will not be called from the beginning. Are you
>> suggesting that we should convert no_printk() to use dynamic debug interface ?
> 
> OK, this is one more path that would need special handling. Two paths
> are much better than 15.

OK. That can avoid needlessly increasing dynamic debug locations.
But I believe that your suggestion is much worse than 15. ;-(

Let's go back to "Add twist into vprintk_store().". The condition is not as simple as

  #if TWIST
    return text_len;
  #endif

because we need to check whether the caller wants to print this message or not.
Since we need to print all messages that the caller asked to print, the condition
needs to be

  #if TWIST
    if (!this_message_should_be_stored_into_logbuf(arg))
      return text_len;
  #endif

and where does the "arg" come? It is not as simple as loglevel. Like you said

  It might get more complicated if you would actually want to see
  pr_debug() messages for a selected module in the fuzzer.

, we want to conditionally store KERN_DEBUG messages into logbuf.

We sometimes don't want to store into logbuf due to ratelimit, due to
dynamic debug. But we DO want to evaluate arguments passed to printk().

Oh, if we try to add twist into vprintk_store(), we will have to modify
all printk() callers in order to pass "arg" only for telling whether we
want to store that messages into logbuf. What a nightmare!