Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 13 Dec 2000 10:03:26 -0800 (PST)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Terry Lambert <tlambert@primenet.com>
Cc:        arch@FreeBSD.org
Subject:   Re: Can !curproc touch
Message-ID:  <XFMail.001213100326.jhb@FreeBSD.org>
In-Reply-To: <200012130813.BAA25955@usr08.primenet.com>

next in thread | previous in thread | raw e-mail | index | archive | help

>> >> I've got a question about p_cred in proc, specifically
>> >> p_cred->pc_ucred.  In several VOP's and other places we
>> >> use p_cred->pc_ucred (aka p_ucred) as the credentials
>> >> if we don't already have one.  The problem arises if
>> >> another process can crfree() that ucred either by a
>> >> crcopy() or a direct crfree() of p_ucred.
> 
> This was incorrectly attributed to me.

??  I wrote that. :)

>> We already share ucred's, and we already do reference counting
>> for ucred structures.  xref crfree()/crcopy()/crhold(), etc.
>> Really, Terry, reading the code and reading the questions in
>> detail would help here.
> 
> Please read my statements in the contect of the question.  In
> particular, I think that "crfree" should be hidden in crunref()
> (or whatever the inverse of "crhold" is) on the 1->0 reference
> decrement, and "crcopy" needs to die.

crfree() is the opposite of crhold().  The actual FREE() of a ucred is hidden 
in crfree() on the 1->0 reference decrement. :)  xref src/sys/kern/kern_prot.c

>> My question is about protecting the p_ucred pointer that is part
>> of struct proc.  I'm not trying to lock the ucred itself, I'm
>> trying to figure out how to ensure that the pointer to a ucred
>> I get out of proc is valid when I pass it to a VOP and that it
>> _stays_ valid the entire time.  I know that if need be I can do
>> it by protecting the p_ucred pointer with the proc lock and
>> bumping the refcount before passing it down the VOP stack, but
>> if I can get away w/o having to do that I'd like to avoid the
>> cost.
> 
> My point was that holding the proc lock is implicit in the
> addition of the reference and the assignment of the pointer
> in the proc structure initially.

Erm, here, let me try to describe this better:

Process P             Process Q

 <start>                            process P gets it's inital ucred from fork
 VOP_FOO()                          P passes its ucred in as the credentionals
                                     for the VOP.  The VOP blocks.
 <asleep>
                      setgroups(P)  process Q changes P's ucred.  The ucred
                                     the VOP is referencing has now either
                                     changed, has been corrupted, or now
                                     is used by some other process.

(This is hypothetical of course.)  This can only happen if more than one
process can read/write P's p_ucred.  This problem occurs after process
creation, not during.
 
> I was under the impression that your function that you are
> entering for the purpose of dereferencing the value out of
> the proc structure could hold _its own_ reference to the
> credential on entry, releasing it on exit.

This is my proposed change, to grab the proc lock to protect the p_ucred
pointer from changing out from under me, then use a local variable to add
another reference to the ucred and bump the refcount, and then release the proc
lock.  After the ucred consumer (e.g. VOP) returns (it will run w/o the proc
lock), it can lower the refcount.

> Given the approach I outlined, this would prevent the 1->0
> reansition, since there would be a positive reference which
> could not go away.  Therefore the credential you entered
> with is valid _at least_ until you exit.

Yes, that is what I can guarantee with the above idea.  However, if p_ucred is
only ever accessed by curproc, then I don't actually need to protect p_ucred
with the proc lock as it will never be contended.

> Does my suggested approach make more sense, and appear more
> distinct from the current implementation, now?

I think we are saying the same thing.

-- 

John Baldwin <jhb@FreeBSD.org> -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.Baldwin.cx/~john/pgpkey.asc
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.001213100326.jhb>