lkml.org 
[lkml]   [2019]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: kernel BUG at kernel/cred.c:434!
From
Date


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



\
 
 \ /
  Last update: 2019-04-23 06:09    [W:0.530 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site