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