Subject: Re: CVS commit: src/sys
To: Bill Stouder-Studenmund <wrstuden@netbsd.org>
From: David Laight <david@l8s.co.uk>
List: source-changes
Date: 06/24/2007 19:36:01
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