lkml.org 
[lkml]   [2021]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 5.10 036/103] ucounts: Increase ucounts reference counter before the security hook
On Thu, Sep 02, 2021 at 09:04:09AM -0400, Sasha Levin wrote:
> On Wed, Sep 01, 2021 at 12:26:10PM -0500, Eric W. Biederman wrote:
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >
> > > On Wed, Sep 01, 2021 at 09:25:25AM -0500, Eric W. Biederman wrote:
> > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > > >
> > > > > From: Alexey Gladkov <legion@kernel.org>
> > > > >
> > > > > [ Upstream commit bbb6d0f3e1feb43d663af089c7dedb23be6a04fb ]
> > > > >
> > > > > We need to increment the ucounts reference counter befor security_prepare_creds()
> > > > > because this function may fail and abort_creds() will try to decrement
> > > > > this reference.
> > > >
> > > > Has the conversion of the rlimits to ucounts been backported?
> > > >
> > > > Semantically the code is an improvement but I don't know of any cases
> > > > where it makes enough of a real-world difference to make it worth
> > > > backporting the code.
> > > >
> > > > Certainly the ucount/rlimit conversions do not meet the historical
> > > > criteria for backports. AKA simple obviously correct patches.
> > > >
> > > > The fact we have been applying fixes for the entire v5.14 stabilization
> > > > period is a testament to the code not quite being obviously correct.
> > > >
> > > > Without backports the code only affects v5.14 so I have not been
> > > > including a Cc stable on any of the commits.
> > > >
> > > > So color me very puzzled about what is going on here.
> > >
> > > Sasha picked this for some reason, but if you think it should be
> > > dropped, we can easily do so.
> >
> > My question is what is the reason Sasha picked this up?
> >
> > If this patch even applies to v5.10 the earlier patches have been
> > backported. So we can't just drop this patch. Either the earlier
> > backports need to be reverted, or we need to make certain all of the
> > patches are backported.
> >
> > I really am trying to understand what is going on and why.
>
> I'll happily explain. The commit message is telling us that:
>
> 1. There is an issue uncovered by syzbot which this patch fixes:
>
> "Reported-by: syzbot"
>
> 2. The issue was introduced in 905ae01c4ae2 ("Add a reference to ucounts
> for each cred"):
>
> "Fixes: 905ae01c4ae2"
>
> Since 905ae01c4ae2 exist in 5.10, and this patch seemed to fix an issue,
> I've queued it up.

I think Eric's question was about why this commit got into 5.10 because
the whole c54b245d0118 patchset was not a bug fix ?

Personally, I don't see anything wrong with moving this patchset to 5.10,
but it's not clear why it was decided to backport it.

> In general, if we're missing backports, backported something only
> partially and should revert it, or anything else that might cause an
> issue, we'd be more than happy to work with you to fix it up.
>
> All the patches we queue up get multiple rounds of emails and reviews,
> if there is a better way to solicit reviews so that we won't up in a
> place where you haven't noticed something going in earlier we'd be more
> than happy to improve that process too.
>
> --
> Thanks,
> Sasha
>

--
Rgrds, legion

\
 
 \ /
  Last update: 2021-09-02 16:29    [W:0.218 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site