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

[Yang Yingliang <yangyingliang@huawei.com>]

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.
>
>> [   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() ?