Date: Mon, 11 Feb 2002 23:12:53 -0500 (EST) From: John Baldwin <jhb@FreeBSD.org> To: Terry Lambert <tlambert2@mindspring.com> Cc: current@freebsd.org, bde@freebsd.org, Alfred Perlstein <bright@mu.org>, Julian Elischer <julian@elischer.org> Subject: Re: ucred holding patch, BDE version Message-ID: <XFMail.020211231253.jhb@FreeBSD.org> In-Reply-To: <3C68831A.12A67EAB@mindspring.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.020211231253.jhb>