Re: kernel BUG at kernel/cred.c:434!

[Paul Moore <paul@paul-moore.com>]

On Tue, Apr 23, 2019 at 12:08 AM Yang Yingliang
<yangyingliang@huawei.com> wrote:
> On 2019/4/23 3:48, Paul Moore wrote:
> > On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang <yangyingliang@huawei.com> wrote:
> >> I'm not sure you got my point.
> > I went back and looked at your previous emails again to try and
> > understand what you are talking about, and I'm a little confused by
> > some of the output ...
> >
> >> --- a/kernel/acct.c
> >> +++ b/kernel/acct.c
> >> @@ -481,6 +481,7 @@ static void do_acct_process(struct bsd_acct_struct
> >> *acct)
> >>           flim = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
> >>           current->signal->rlim[RLIMIT_FSIZE].rlim_cur = RLIM_INFINITY;
> >>           /* Perform file operations on behalf of whoever enabled
> >> accounting */
> >> +       pr_info("task:%px new cred:%px real cred:%px cred:%px\n",
> >> current, file->f_cred, current->real_cred, current->cred);
> >>           orig_cred = override_creds(file->f_cred);
> > Okay, with this patch applied we should the task/cred info when
> > do_acct_process is called.  Got it.
> >
> >> Messages:
> >> [   56.643298] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
> >> cred:ffff88841ae450c0 cred:ffff88841ae450c0    //They are same.
> > Okay, it looks like do_acct_process() was called and f_cred,
> > real_cred, and cred are all the same.
>
> This is a original message, without patch applied.

The patch I am referring to is your pr_info patch (above).  I'm not
talking about any other patches at the moment; I just want to
understand the example dmesg output you copied into your email.

With that in mind, the message above seems to indicate that
do_acct_process() has been invoked with f_cred, real_cred, and cred
all pointing to the same credentials struct, yes?

> >> [   56.646609] Process accounting resumed
> > It looks like do_acct_process() has called check_free_space() now.  So
> > far so good.

...

> >> [   56.649943] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
> >> cred:ffff88841c96c300 cred:ffff88841ae450c0
> > Wait a minute ... why are we seeing this again?  Looking at the task
> > pointer and the timestamp, this is the same task exiting and trying to
> > write to the accounting file, yes?  This output is particularly
> > curious since it appears that real_cred has changed; where is this
> > happening?
>
> This is the message when the BUG_ON was triggered without applying any
> fix patch.

The only place in the code that generates this message is the bit of
code that you patches in using the pr_info() patch (above), yes?  If
so, that would seem to indicate that the same task is calling
do_acct_process() twice, yes?  I may be fundamentally misunderstanding
something about process accounting, but I though do_acct_process()
would only be called once for a given task - while it was exiting.
Yes, no?

> If we apply this patch "proc: prevent changes to overridden
> credentials", program
> runs like this:

I'd like to focus on understanding the dmesg output you shared first,
because it doesn't seem to make sense to me.

-- 
paul moore
www.paul-moore.com