On 12-Feb-02 Terry Lambert wrote:
> Julian Elischer wrote:
>> > The "multiple threads" argument is bogus, since the calls
>> > to [gs]et[ug]id() are on a per process, not a per thread
>> > basis.
>> there is no such thing as a per process syscall.
>> Two threads can always do the same syscall at the same time.
>> there needs to be a proc-lock to stop it from becoming
>> chaotic in there. In actual fact, since you cannot alter a cred
>> but only replace that which the process points to it's not
>> quite that bad, but you need to either lock it or have atomic
>> reference-counting that can handle the possibility that
>> the cred could have bee decremented to 0 by another thread just before
>> you checked it.
> I would argue that:
> 1)    There are a small number of system calls where this
>       is true.
> 2)    It's worth doing the synchornization in-kernel for
>       the process alone, where this is the case.
> The problem I have with the crd locking is that each process
> does not do a gratuitous clone of the active cred before doing
> its own thing (if you will remember, I suggested this in the
> context of the cred reference count overflow bug, back when I
> found the problem last year).

... which has nothing to do with the problem at hand as far as I can tell. 
Well, if we did, say, embed a cred in each process rather than sharing them,
then we wouldn't need to protect p_ucred with a lock, but now we would need to
protect all the contents of the cred with a lock, or go with the IPI scheme
(yuck, IPI's are very uncheap).

> The upshot of this is that the lock will stall anyone using
> that cred.  This argues that creds should be, minimally,
> per process, if not per CPU, instead of shared references
> smeared all over the place, and locked on each reference,
> even though it's not possible for a cred to be changed at
> all out from under you -- only replaced wholesale, since
> once instanced, a cred may not be changed.

What in the world are you talking about Terry.  Have you read the code?
The lock _only_ protects the reference count, nothing else!  Once I commit the
code to make sure that we use p_ucred properly when updating credentials then I
can commit the code to chagne all of suser(), p_can*() to use the per-thread
ucred.  Since teh per-thread ucred is immutable, and since td_ucred is only
ever touchced by curthread, thsi involves NO LOCKS for ANY calls to suser() or
p_can*() EXCEPT in the few syscalls that update credentials.

We went over this _months_ ago.  I direct you to the archives of -arch please.

> Personally, I think that cred changes are rare enough,
> even in the degenerate case, that it's worth taking the
> synchronization event as an intraprocess global IPI,
> rather than using a lock.

Egads.  That still doesn't fix the problem of some syscall changing its creds
to get weird behavior halfway through a syscall.  You still need locks if two
threads call setuid() at the same time.   Sure it's a program bug but we can't
have the kernel panic over it.

>> creads can only be changed per process but the threads only pick
>> up the change on next syscall startup.
> I think this is the fundamental misunderstanding: creds
> never change.  The can only be replaced, and then only
> with a cred of equal or lesser priviledge.

Well, Robert wasn't very comfortable with that change, plus it either requires
a horribly expensive and ugly IPI, or it requires lots of lock acquires to read
p_ucred that we wouldn't need otherwise.


John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to