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

[Yang Yingliang]

On 2019/4/23 3:48, Paul Moore wrote:
> On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang <yangyingliang@xxxxxxxxxx> 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.
> > [   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.


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

1. As print message shows, before overriden, the pointer has the 
following value:
    real_cread=cred=0xffff88841ae450c0, f_cred=0xffff88841ae450c0
    override_creds() is called in do_acct_process():
    ...
    /* Perform file operations on behalf of whoever enabled accounting */
    orig_cred = override_creds(file->f_cred);
    ...


2. After override_creds(), if (current_cred() != current_real_cred()) is 
not work here,
we will call commit_creds()  in security_setprocattr().
    ...
    /* Prevent changes to overridden credentials. */
        if (current_cred() != current_real_cred()) {
            rcu_read_unlock();
        return -EBUSY;
    }
    ...


3. After commit_creds(), we have new cred and real_cred.
    security_setprocattr()    //commit_creds is called here

4. revert_creds() is called in in do_acct_process(), the cred
is reverted to the old value(0xffff88841ae450c0)
    ...
    current->signal->rlim[RLIMIT_FSIZE].rlim_cur = flim;
    revert_creds(orig_cred);

5. After reverting, cred and real_cred are not equal.
If it has a risk to trigger the BUG_ON, when doing another
commit_creds() ?