Date: Tue, 11 Jun 2002 04:36:41 -0400 (EDT) From: John Baldwin <jhb@FreeBSD.org> To: Mike Makonnen <makonnen@pacbell.net> Cc: hiten@uk.FreeBSD.org, jrh@lab.it.uc3m.es, freebsd-current@FreeBSD.ORG Subject: Re: Fixing "could sleeep..." was (Re: ../../../vm/uma_core.c:132 Message-ID: <XFMail.20020611043641.jhb@FreeBSD.org> In-Reply-To: <200206100932.g5A9WPe1008406@kokeb.ambesa.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On 10-Jun-2002 Mike Makonnen wrote: >> Well, the real solution probably involves changing where we dink with >> uidinfo structs so we bump the reference count on teh new one before > we> grab the proc lock, change over to the new one while holding the > proc lock,> then release the reference to the old one after we are done. >> > > Well... this is basically what happens > > setuid - creates a new ucred > - locks p > - calls change_ruid() > > change_ruid - calls uifind() > > uifind - does MALLOC w/ M_WAITOK > > After thinking about it for a while, this is the solution I came up > with: > > Each new struct ucred will carry an array of pointers to struct uidinfo. > This will be an array of 3 elements (a spare for cr_ruidinfo, > cr_uidinfo, null). Obviously, it gets added after ->cr_endcopy. > > When crget() is called it calls a function whose job it is to create an > array of pointers to struct uidinfo and allocate the memory for them. > > When uifind() is called it will be given an array of pointers to uidinfo > structs (the ones from ucred), in addition to the uid it is to lookup. > If it already has a uidinfo for that uid, then it returns that to the > calling function. If it can't find the uid, then it unhooks (copies the > address, and deletes it from the array) the last struct uidinfo from > the array, initializes it, inserts it into the hashtable and returns it > to the caller. > > When crfree is called it calls a function that deallocates the spare > structs uidinfo. > > This solution has the advantage that the only code that has to change is > the ucred and setuid/gid helper functions that already know about the > struct uidinfo functions. In fact only three functions not related to > uidinfo(9) need to be touched: proc0_init(), change_ruid(), > change_uid(). The disadvantage is the memory bloat and a small amount of > code complexity (but as I said, this is localized, and not very complex > either). > > Do you like it? > Should I go ahead and implement a patch? > Anything I overlooked? It won't work if you have to change a uidinfo more than once. I still prefer just doing the uifind() at the beginning of the function, passing in the uidinfo pointer to the chnage_fooid() functions, and adding cleanup uifree's in case of failure. -- 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.20020611043641.jhb>