Julian Elischer wrote:
> In the KSE code I have:
>  in trap(), and syscall()
>         if (td->td_ucred != p->p_ucred) {
>                 PROC_LOCK(p);
>                 if (td->td_ucred) {
>                         crfree(td->td_ucred);
>                         td->td_ucred = NULL;
>                 }
>                 if (p->p_ucred != NULL) {
>                         td->td_ucred = crhold(p->p_ucred);
>                 }
>                 PROC_UNLOCK(p);
>         }
> THis is actually not dependent on KSE (though I originally needed it
> for KSE I have since decided that it would be a good idea anyhow).
> Do those who deal in ucreds (probably jhb and Robert W)
> agree or disagree.. (if so I'll happily commit it as it shrinks the KSE
> patches a bit more :-)

You can't have a process with a thread without a ucred, right?

If so, then the idea is sound, but the code is broken.

In the case that they are not equal, you free it, null
the pointer, and don't allocate a replacement.

What is the default state of td->td_ucred?

In theory, it should be initialized on creation, and then
left initialized (you are effectively lazy binding the cred

Since the reference count is positive in both cases, an
unlocked pointer comparison is fine.  In the case where
there is a race on a credential change in the unlocked
value, that race exists in the calling code, anyway.  The
failure case in a change for->to instead of to->from is
also safe, since the pointers will be inequal during the
update, the lock will be held during the update, so the
subsequent release and regrab with the increment never
dropping below 1.

If that's the case, then the code should be:

        if (td->td_ucred != p->p_ucred) {
                if (td->td_ucred) {
                td->td_ucred = crhold(p->p_ucred);

Is a thread lock required?  I think it's guaranteed to
be non-reentrant, so the answer should be no...

-- Terry

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

Reply via email to