Re: CVS commit: src/sys

[David Laight <david%l8s.co.uk@localhost>]

On Sun, Jun 24, 2007 at 11:30:10AM -0700, Bill Stouder-Studenmund wrote:
> Setting groups:
> 
> 3) We pass out the memory array, let the caller scribble on it, then 
> process it. This seems like a bad idea. I think what we should do is pass 
> in an array and have kauth set the group list to that. I think changing 
> the group set should be atomic. Either it happens or it doesn't. Other 
> users (other threads in the same process for instance) should either see 
> the old group set or the new group set. Nothing else.
> 
> The current code, however, isn't atomic. We set the number of groups in
> the kauth_cred_setngroups() call. We then let the caller do things. We 
> then kauth_authorize_process() the operation, and if it fails, we error 
> out.

Actually the intent is that this code runs immediately after kauth_cred_alloc()
so the cred structure has no contents prior to the call.

> What's really unclear to me is how we are supposed to go back to the 
> original group set at this point. The caller has directly modified the 
> group set, and I see no way to restore the old contents.

You don't the only user is the code trying to initialise the structure.
on error it is freed.
 
> 4) Since we change the # groups before we finish changing the groups, we 
> can expose concurrent access to group IDs that aren't set yet. Say we 
> raise the count. Before we finish updating the array, other threads can 
> see garbage gids. That's bad.

No only owner is the calling function.
 
> 6) In kauth_proc_setgroups(), there are two calls to proc_crmod_leave() 
> that have "ncred" and "cred" passed as different arguements. Is that 
> correct?

Correct.

        David

-- 
David Laight: david%l8s.co.uk@localhost