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]

Date:   Mon, 25 May 2020 09:38:28 +0200
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Ondrej Mosnacek <omosnace@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        syzkaller <syzkaller@...glegroups.com>
Subject: Re: [PATCH] twist: allow converting pr_devel()/pr_debug() into printk(KERN_DEBUG)

On Mon, May 25, 2020 at 8:07 AM Joe Perches <joe@...ches.com> wrote:
>
> On Mon, 2020-05-25 at 14:03 +0900, Tetsuo Handa wrote:
> > On 2020/05/25 4:18, Ondrej Mosnacek wrote:
> > > I'm also not sure if this is really worth it... It would help localize
> > > the bug in this specific case, but there is nothing systematic about
> > > it. Are there that many debug print statements that dereference
> > > pointers that are later passed to functions, but not dereferenced
> > > otherwise? Maybe yes, but it seems to be quite an optimistic
> > > assumption... I don't consider it such a big problem that a bug in
> > > function X only manifests itself deeper in the callchain. There will
> > > always be such bugs, no matter how many moles you whack.
> >
> > There are about 1400 pr_debug() callers. About 1000 pr_debug() callers seem
> > to pass plain '%p' (which is now likely useless for debugging purpose due to
> > default ptr_to_id() conversion inside pointer()), and about 400 pr_debug()
> > callers seem to pass '%p[a-zA-Z]' (which does some kind of dereference inside
> > pointer()). Thus, we might find some bugs by evaluating '%p[a-zA-Z]'.
> >
> >
> >
> > On Sun, May 24, 2020 at 7:38 PM Joe Perches <joe@...ches.com> wrote:
> > > While I think this is rather unnecessary,
> > > what about dev_dbg/netdev_dbg/netif_dbg et al ?
> >
> > Maybe a good idea, for there are about 24000 *dev_dbg() callers, and
> > 479 callers pass '%p[a-zA-Z]'. But we can defer to another patch, in
> > case this patch finds crashes before fuzz testing process starts.
>
> There are a bunch more than that.
> Some use other macros, some are functions.


I think this is a good idea overall and I don't mind enabling it on syzbot.
It's not only about %p, even %d can crash kernel or leak sensitive
info (if it happens after-free/out-of-bounds/uninit). Overall it
increases code coverage and allows to catch more bugs earlier. That
was the reason for enabling dynamic debug, but I wasn't aware that
debug level is not included.