From owner-freebsd-hackers Sat Jun 10 11: 3:31 2000 Delivered-To: freebsd-hackers@freebsd.org Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20]) by hub.freebsd.org (Postfix) with ESMTP id 6B22937BE19; Sat, 10 Jun 2000 11:03:26 -0700 (PDT) (envelope-from bright@fw.wintelcom.net) Received: (from bright@localhost) by fw.wintelcom.net (8.10.0/8.10.0) id e5AI3QC07329; Sat, 10 Jun 2000 11:03:26 -0700 (PDT) Date: Sat, 10 Jun 2000 11:03:26 -0700 From: Alfred Perlstein To: Brian Fundakowski Feldman Cc: hackers@FreeBSD.ORG Subject: Re: uidinfo has many race conditions. Message-ID: <20000610110326.R18462@fw.wintelcom.net> References: <20000609170758.O18462@fw.wintelcom.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2i In-Reply-To: ; from green@FreeBSD.ORG on Sat, Jun 10, 2000 at 12:09:18PM -0400 Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG * Brian Fundakowski Feldman [000610 09:13] wrote: > On Fri, 9 Jun 2000, Alfred Perlstein wrote: > > > * Alfred Perlstein [000609 16:45] wrote: > > > hi, > > > > > > Is it just me or does the fact that uidinfo structures (see > > > kern/kern_proc.c) are allocated with M_WAITOK after finding them > > > fails and then inserted into the uidhash structure a race condition? > > > > > > Index: kern_proc.c > > > =================================================================== > > > > Yes, I know i forgot to put the created ones back into the list, I was > > just a bit flusteres after reading over the code. I'll have some more > > later. > > With regard to sbsize itself, the test-and-branch conditions do not have > to be atomic. It really isn't that important. The incrementing does, > though, and to fix that a very lightweight mutex should be used. How > about a simplelock? That should work perfectly. Well if we get an atomic_t it could be done that way, even if we fail the race for updating and go beyond our rlimit slightly it's much cheaper than a spinlock from my PoV. The only problem is that afaik on some archs atomic_t can be quite small, we'd have to watch for overflow, perhaps a spinlock is a better idea however only if the next thing I mention here is realized: I'm somewhat upset now that I understand sbsize; every operation on a socket involves a lookup of the uidinfo and manipulating it. You could optimize it by having a flag in the socket struct that says "created with a non-infinity RLIMIT", which means that chgsbsize needs to be called when the size changes, but for the common case it's not done. Do you think you can do that? > As for uidinfo itself, I feel it should be done with a mutex over > uihashtbl. It should be grabbed if the hash for the uid is not > found, tested again (to see if we lost the race), and allocated. > I can't see a way to poke holes in that, and it would be quite > efficient. A mutex on each of the hash entrie heads would work, I really don't like the manual inlining of the lookup and insert functions, seperating them would be a good idea, it really reduces the complexity of the code. -- -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org] "I have the heart of a child; I keep it in a jar on my desk." To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message