From owner-freebsd-current Mon Feb 11 20:43:27 2002 Delivered-To: freebsd-current@freebsd.org Received: from mail12.speakeasy.net (mail12.speakeasy.net [216.254.0.212]) by hub.freebsd.org (Postfix) with ESMTP id 96C5537BDEB for ; Mon, 11 Feb 2002 20:13:08 -0800 (PST) Received: (qmail 5523 invoked from network); 12 Feb 2002 04:13:03 -0000 Received: from unknown (HELO laptop.baldwin.cx) ([65.91.152.149]) (envelope-sender ) by mail12.speakeasy.net (qmail-ldap-1.03) with SMTP for ; 12 Feb 2002 04:13:03 -0000 Message-ID: X-Mailer: XFMail 1.4.0 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: <3C68831A.12A67EAB@mindspring.com> Date: Mon, 11 Feb 2002 23:12:53 -0500 (EST) From: John Baldwin To: Terry Lambert Subject: Re: ucred holding patch, BDE version Cc: current@freebsd.org, bde@freebsd.org, Alfred Perlstein , Julian Elischer Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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 <>< 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