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: <20210104232123.31378-1-stephen.s.brennan@oracle.com>
 <20210105055935.GT3579531@ZenIV.linux.org.uk> <20210105165005.GV3579531@ZenIV.linux.org.uk>
 <20210105195937.GX3579531@ZenIV.linux.org.uk> <87a6tnge5k.fsf@stepbren-lnx.us.oracle.com>
 <CAHC9VhQnQW8RvTzyb4MTAvGZ7b=AHJXS8PzD=egTcpdDz73Yzg@mail.gmail.com> <20210106003803.GA3579531@ZenIV.linux.org.uk>
In-Reply-To: <20210106003803.GA3579531@ZenIV.linux.org.uk>
From:   Paul Moore <paul@paul-moore.com>
Date:   Tue, 5 Jan 2021 21:43:13 -0500
Message-ID: <CAHC9VhQyZOewT5nQ5fqqx-tvSx1kt62i26ruF_Unk5K_iFQTKA@mail.gmail.com>
Subject: Re: [PATCH v4] proc: Allow pid_revalidate() during LOOKUP_RCU
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     Stephen Brennan <stephen.s.brennan@oracle.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Alexey Dobriyan <adobriyan@gmail.com>,
        James Morris <jmorris@namei.org>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        linux-security-module@vger.kernel.org,
        Stephen Smalley <stephen.smalley.work@gmail.com>,
        Eric Paris <eparis@parisplace.org>, selinux@vger.kernel.org,
        Casey Schaufler <casey@schaufler-ca.com>,
        Eric Biederman <ebiederm@xmission.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        Matthew Wilcox <willy@infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Jan 5, 2021 at 7:38 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Tue, Jan 05, 2021 at 07:00:59PM -0500, Paul Moore wrote:

...

> > I would expect the problem here to be the currently allocated audit
> > buffer isn't large enough to hold the full audit record, in which case
> > it will attempt to expand the buffer by a call to pskb_expand_head() -
> > don't ask why audit buffers are skbs, it's awful - using a gfp flag
> > that was established when the buffer was first created.  In this
> > particular case it is GFP_ATOMIC|__GFP_NOWARN, which I believe should
> > be safe in that it will not sleep on an allocation miss.
> >
> > I need to go deal with dinner, so I can't trace the entire path at the
> > moment, but I believe the potential audit buffer allocation is the
> > main issue.
>
> Nope.  dput() in dump_common_audit_data(), OTOH, is certainly not
> safe.

My mistake.  My initial reaction is to always assume audit is the
problem; I should have traced everything through before commenting.

> OTTH, it's not really needed there - see vfs.git #work.audit
> for (untested) turning that sucker non-blocking.  I hadn't tried
> a followup that would get rid of the entire AVC_NONBLOCKING thing yet,
> but I suspect that it should simplify the things in there nicely...

It would be nice to be able to get rid of the limitation on when we
can update the AVC and do proper auditing.  I doubt the impact is
anything that anyone notices, but I agree that it should make things
much cleaner.  Thanks Al.

-- 
paul moore
www.paul-moore.com