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).

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.

Ask John and Robert about the permissability of changing
cred contents, and the NFS changes that resulted (fairly)

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.

> > Personally, I still do not understand the need to have a
> > cred reference per thread, the only thing that makes any
> > sense about that is to optimize the degenerate case of a
> > daemon that makes calls as another ID, on behalf of a lot
> > of users (or, sequentially, at least, different users).
> > One example of such a program would be SAMBA (but *not*
> > NFS, due to "access" semantics on objects based on path
> > component access exclusion by credential not being an
> > effective mechanism for NFS file handles).
> the cred that is in force at  the time that the syscall STARTS
> is used for the full syscall otherwise you could have
> one cred used for the first part of a syscall and a completely
> differnet one used for the secnd part of a syscall.

Again: the model permits dropping, but not gaining, of
priviledges.  THe worst case failure, then, is:

1)      A call starts on a thread without proper program
        level stall synchornization between threads.

2)      A subsequent thread drops priviledges, and this
        drop is unprotected by the stall.

3)      The call completes, but fails during completion
        for lack of privledges present when the call was
        started, because the drop was not stalled.

Frankly, I don't see this being a problem: badly written
code with a race condition will occasionally lose the race,
rather than being implicitly protected (an assumption that
is not portable, if it is depended upon), and the code will
end up being fixed by its author.

> > I think that you would need to have [gs]et[ug]id() be on a
> > per thread basis for this to be an efficiency, and I think
> > trying to do this pessimizes everything else.
> >
> > My gut tells me that creds should be per process, and
> > that the references to them should be taken sparingly,
> > and then only if a need can be justified, rather than
> > "just in case some day".
> 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.

> > Kirk at one time called vnodes "the structure that ate the
> > kernel"; he was wrong: it was creds.
> I believe it was Mike Karels.

Whatever.  It's "creds" now.

> > Perhaps this dicsussion is enough impetus to justify
> > revisiting the atomic_t type definitions, which would
> > be useful as reference counted hold/release mechanisms
> > that would obviate the need for locks here?  This would
> > at least let you defer getting rid of the per thread
> > cred instances until later.
> I've made that point before and I believe that jhb has said he would like
> such primatives.

I'm still not convinced that it's necessary to lock what
you are trying to lock, but I certainly agree on atomic_t.

-- Terry

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

Reply via email to