Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: kernel BUG at kernel/cred.c:434! | From | Yang Yingliang <> | Date | Tue, 23 Apr 2019 12:08:36 +0800 |
| |
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() ?
| ![\](/images/icornerr.gif) |