Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <0000000000003cbb40059f4e0346@google.com> <CAHC9VhQVXk5ucd3=7OC=BxEkZGGLfXv9bESX67Mr-TRmTwxjEg@mail.gmail.com>
 <17916d0509978e14d9a5e9eb52d760fa57460542.camel@redhat.com>
 <CAHC9VhQnbdJprbdTa_XcgUJaiwhzbnGMWJqHczU54UMk0AFCtw@mail.gmail.com>
 <CACT4Y+azQXLcPqtJG9zbj8hxqw4jE3dcwUj5T06bdL3uMaZk+Q@mail.gmail.com>
 <CAHC9VhRRDJzyene2_40nhnxRV_ufgyaU=RrFxYGsnxR4Z_AWWw@mail.gmail.com>
 <55b362f2-9e6b-2121-ad1f-61d34517520b@i-love.sakura.ne.jp>
 <CAHC9VhT51-xezOmy1SM4eP_jFH9A8Tc05wY=cwDg7oC=FgYbYQ@mail.gmail.com>
 <CACT4Y+YgoyBCoPYxXOb8oQjXYc+Q-cZLPi6y1Yrx_mnfzOQafQ@mail.gmail.com> <CAHC9VhTsKCJf8bjOT+cxWZEX1y4c57KcVz0Y2c3vRGnJJQA4pA@mail.gmail.com>
In-Reply-To: <CAHC9VhTsKCJf8bjOT+cxWZEX1y4c57KcVz0Y2c3vRGnJJQA4pA@mail.gmail.com>
From:   Dmitry Vyukov <dvyukov@google.com>
Date:   Mon, 2 Mar 2020 15:25:35 +0100
Message-ID: <CACT4Y+bjB1TWZgrQiq0m57NL8T6rJX4b_vPRsoYTY0Lwqh2qVw@mail.gmail.com>
Subject: Re: kernel panic: audit: backlog limit exceeded
To:     Paul Moore <paul@paul-moore.com>
Cc:     Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
        syzbot <syzbot+9a5e789e4725b9ef1316@syzkaller.appspotmail.com>,
        LKML <linux-kernel@vger.kernel.org>,
        syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
        syzkaller <syzkaller@googlegroups.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Mar 2, 2020 at 2:43 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Mon, Mar 2, 2020 at 3:47 AM Dmitry Vyukov <dvyukov@google.com> wrote:
> > On Fri, Feb 28, 2020 at 2:09 PM Paul Moore <paul@paul-moore.com> wrote:
> > > On Fri, Feb 28, 2020 at 5:03 AM Tetsuo Handa
> > > <penguin-kernel@i-love.sakura.ne.jp> wrote:
> > > > On 2020/02/28 9:14, Paul Moore wrote:
> > > > > We could consider adding a fuzz-friendly build time config which would
> > > > > disable the panic failsafe, but it probably isn't worth it at the
> > > > > moment considering the syzbot's pid namespace limitations.
> > > >
> > > > I think adding a fuzz-friendly build time config does worth. For example,
> > > > we have locations where printk() emits "BUG:" or "WARNING:" and fuzzer
> > > > misunderstands that a crash occurred. PID namespace is irrelevant.
> > > > I proposed one at
> > > > https://lkml.kernel.org/r/20191216095955.9886-1-penguin-kernel@I-love.SAKURA.ne.jp .
> > > > I appreciate your response.
> > >
> > > To be clear, I was talking specifically about the intentional panic in
> > > audit_panic().  It is different from every other panic I've ever seen
> > > (perhaps there are others?) in that it doesn't indicate a serious
> > > error condition in the kernel, it indicates that audit records were
> > > dropped.  It seems extreme to most people, but some use cases require
> > > that the system panic rather than lose audit records.
> > >
> > > My suggestion was that we could introduce a Kconfig build flag that
> > > syzbot (and other fuzzers) could use to make the AUDIT_FAIL_PANIC case
> > > in audit_panic() less panicky.  However, as syzbot isn't currently
> > > able to test the kernel's audit code due to it's pid namespace
> > > restrictions, it doesn't make much sense to add this capability.  If
> > > syzbot removes that restriction, or when we get to the point that we
> > > support multiple audit daemons, we can revisit this.
> >
> > Yes, we need some story for both panic and pid ns.
> >
> > We also use a separate net ns, but allow fuzzer to create some sockets
> > in the init net ns to overcome similar limitations. This is done using
> > a pseudo-syscall hack:
> > https://github.com/google/syzkaller/blob/4a4e0509de520c7139ca2b5606712cbadc550db2/executor/common_linux.h#L1546-L1562
> >
> > But the pid ns is different and looks a bit harder as we need it
> > during send of netlink messages.
> >
> > As a strawman proposal: the comment there says "for now":
> >
> > /* Only support auditd and auditctl in initial pid namespace
> >  * for now. */
> > if (task_active_pid_ns(current) != &init_pid_ns)
> >   return -EPERM;
> >
> > What does that mean? Is it a kind of TODO? I mean if removing that
> > limitation is useful for other reasons, then maybe we could kill 2
> > birds with 1 stone.
>
> Long story made short - the audit subsystem doesn't handle namespaces
> or containers as well as it should.  Work is ongoing to add the
> necessary support, but it isn't there yet and I don't want us to just
> start removing restrictions until we have the proper support in place
> (this what I alluded to with my "... when we get to the point that we
> support multiple audit daemons, we can revisit this").

I see. Thanks for context.

FTR we've started collecting such cases
(panic-but-working-as-intended-and-hard-to-selectively-filter-out) in
https://github.com/google/syzkaller/issues/1622. So that they are not
lost in future.