regd. thread ucred update in kern_accessat()

[Mateusz Guzik]

On Fri, Nov 01, 2013 at 11:31:05AM -0700, Vijay Singh wrote:
> Hi hackers. In kern_accessat() [I'm looking at 8.2 code], for the case
> where flags doesn't have AT_EACCESS set, we make a local copy of the thread
> ucred. This is then passed in to vn_access(). My question is why we update
>  td->td_ucred with this temporary ucred?
> 

/me takes nitpicky hat on

For starters you should have checked newer tree - maybe this code would
be gone and you would get an answer from respective commit.

/me takes the hat off

>                 tmpcred = crdup(cred);
>                 tmpcred->cr_uid = cred->cr_ruid;
>                 tmpcred->cr_groups[0] = cred->cr_rgid;
> ==>          td->td_ucred = tmpcred;
> 
> At this point p->p_ucred != td->td_ucred. Couldn't this cause issues?

Can you elaborate what you have in mind?

This is fine. In fact, when you do setuid() thread credentials are
updated only on syscall boundary. In worst case syscall that is being
performed at the moment is using old credentials which is ok -
whatever you wanted to do with it you can do before calling setuid() :)

Other case would be when you drop some privs and suddenly something is
allowed to ptrace you. Even if that was the case, already executing
syscall cannot be altered and credentials are changed for another one.

> Wouldn't it just suffice to use the tempcred as an argument to vn_access()
> and leave the thread's ucred intact?
> 

Before vn_access you can find namei call and it uses thread credentials.
Specifically ndp->ni_cnd.cn_thread->td_ucred, as populated by
NDINIT_ATRIGHTS.

Maybe creating another macro which allows one to supply credentials
would be ok, but one would have to go first through the code from namei
and vn_access down the stack and check for td_ucred users (this should be
fine, but just in case).

-- 
Mateusz Guzik <mjguzik gmail.com>