Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200524145034.10697-1-penguin-kernel@I-love.SAKURA.ne.jp> <d65ee15211aa69a815bdc7cc4fc9e7c2e1bcba43.camel@perches.com>
In-Reply-To: <d65ee15211aa69a815bdc7cc4fc9e7c2e1bcba43.camel@perches.com>
From:   Ondrej Mosnacek <omosnace@redhat.com>
Date:   Sun, 24 May 2020 21:18:38 +0200
Message-ID: <CAFqZXNthJE0a3KkgZFXYSFArwRs0H_20KjT6KfAkiMo6WTp1rw@mail.gmail.com>
Subject: Re: [PATCH] twist: allow converting pr_devel()/pr_debug() into printk(KERN_DEBUG)
To:     Joe Perches <joe@perches.com>
Cc:     Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux kernel mailing list <linux-kernel@vger.kernel.org>,
        Dmitry Vyukov <dvyukov@google.com>,
        Petr Mladek <pmladek@suse.com>,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, May 24, 2020 at 7:38 PM Joe Perches <joe@perches.com> wrote:
> On Sun, 2020-05-24 at 23:50 +0900, Tetsuo Handa wrote:
> > syzbot found a NULL pointer dereference bug inside mptcp_recvmsg() due to
> > ssock == NULL, but this bug manifested inside selinux_socket_recvmsg()
> > because pr_debug() was no-op [1].
> >
> >   pr_debug("fallback-read subflow=%p",
> >            mptcp_subflow_ctx(ssock->sk));
> >   copied = sock_recvmsg(ssock, msg, flags);
>
> > Since console loglevel used by syzkaller will not print KERN_DEBUG
> > messages to consoles, always evaluating pr_devel()/pr_debug() messages
> > will not cause too much console output. Thus, let's allow fuzzers to
> > always evaluate pr_devel()/pr_debug() messages.
>
> While I think this is rather unnecessary,
> what about dev_dbg/netdev_dbg/netif_dbg et al ?

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.

That said, I'm not strongly opposed to the change either, I just
wanted to state my opinion in case my reply to the syzbot report [1]
gave the impression that I considered the "misattribution" as
something that needs to be fixed :)

[1] https://lore.kernel.org/selinux/CAFqZXNvf+oJs9u4H97u7=jTL2Wo_Hkf4nZdZJLD7tNC_J0KDRg@mail.gmail.com/

--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.