Re: [PATCH] audit,io_uring: io_uring openat triggers audit reference count underflow

[Paul Moore]

On Fri, Oct 13, 2023 at 12:22 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> On Fri, Oct 13, 2023 at 11:56:08AM -0400, Paul Moore wrote:
> > On Fri, Oct 13, 2023 at 11:44 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > >
> > > On Thu, 12 Oct 2023 14:55:18 -0700, Dan Clash wrote:
> > > > An io_uring openat operation can update an audit reference count
> > > > from multiple threads resulting in the call trace below.
> > > >
> > > > A call to io_uring_submit() with a single openat op with a flag of
> > > > IOSQE_ASYNC results in the following reference count updates.
> > > >
> > > > These first part of the system call performs two increments that do not race.
> > > >
> > > > [...]
> > >
> > > Picking this up as is. Let me know if this needs another tree.
> >
> > Whoa.  A couple of things:
> >
> > * Please don't merge patches into an upstream tree if all of the
> > affected subsystems haven't ACK'd the patch.  I know you've got your
> > boilerplate below about ACKs *after* the merge, which is fine, but I
> > find it breaks decorum a bit to merge patches without an explicit ACK
> > or even just a "looks good to me" from all of the relevant subsystems.
>
> I simply read your mail:
>
> X-Date: Fri, 13 Oct 2023 17:43:54 +0200
> X-URI: https://lore.kernel.org/lkml/CAHC9VhQcSY9q=wVT7hOz9y=o3a67BVUnVGNotgAvE6vK7WAkBw@xxxxxxxxxxxxxx
>
> "I'm not too concerned, either approach works for me, the important bit
>  is moving to an atomic_t/refcount_t so we can protect ourselves
>  against the race.  The patch looks good to me and I'd like to get this
>  fix merged."
>
> including that "The patch looks good to me [...]" part before I sent out
> the application message:

Some of this is likely due to email races, or far faster than normal
responses.  When I was writing the email you reference above ("This
patch looks good to me...") the last email I had from you was asking
for changes to the patch; since you were suggesting a change I made
the assumption (which arguably one shouldn't assume things) that you
were not planning to merge the patch.

> X-Date: Fri, 13 Oct 2023 17:44:36 +0200
> X-URI: https://lore.kernel.org/lkml/20231013-karierte-mehrzahl-6a938035609e@brauner
>
> > Regardless, as I mentioned in my last email (I think our last emails
> > raced a bit), I'm okay with this change, please add my ACK.
>
> It's before the weekend and we're about to release -rc6. This thing
> needs to be in -next, you said it looks good to you in a prior mail. I'm
> not sure why I'm receiving this mail apart from the justified
> clarification about -stable although that was made explicit in your
> prior mail as well.

I hope I explained the intent in my last email a bit more clearly with
the explanation above.  Regardless, I think the lessons to be learned
is that I won't assume that your suggestion of changes and merging a
patch are mutually exclusive, and just to be on the safe side I would
ask that you not merge audit, LSM, or SELinux related patches without
an explicit ACK from those subsystems.  Hopefully that should prevent
things like this from happening again.

-- 
paul-moore.com