Date: Tue, 5 Sep 2000 05:58:26 -0700 From: Don Lewis <Don.Lewis@tsc.tdk.com> To: Bruce Evans <bde@zeta.org.au>, Don Lewis <Don.Lewis@tsc.tdk.com> Cc: Robert Watson <rwatson@FreeBSD.ORG>, freebsd-arch@FreeBSD.ORG Subject: Re: Request for review: restructuring of per-uid resource limits Message-ID: <200009051258.FAA24311@salsa.gv.tsc.tdk.com> In-Reply-To: <Pine.BSF.4.21.0009051756180.23890-100000@besplex.bde.org> References: <Pine.BSF.4.21.0009051756180.23890-100000@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sep 5, 6:32pm, Bruce Evans wrote: } Subject: Re: Request for review: restructuring of per-uid resource limits } On Mon, 4 Sep 2000, Don Lewis wrote: } } > I'd like to get my patch tested and into the tree sooner rather than } > later, since I'm concerned about bugs in the existing implementation. } > I also want something that can be MFC'ed to 4-stable. } } The sbsize limit should be backed out of 4-stable until it is correctly } implemented. Recent changes probably increased the chance of a panic } by calling chgsbsize() in interrupt context. I think the previous } version was more likely to cause an effective limit of nearly } RLIM_INFINITY (even when the apparent limit is much lower) and a } resource leak than a panic, since the result of incrementing sb_hiwat } in interrupt context without incrementing ui_sbsize was to tend to } make ui_sbsize go negative when the socket is deallocated. There was } only a panic when ui_sbsize hit precisely 0 earlier than it should have } (and ui_proccnt was also 0). My patch removes all the dangerous operations from interrupt context. It also reference counts the uidinfo structure, which only gets freed when the reference count goes to zero. The hazards in the present implementation are what motivated me to work on this. } The bugs in chgsbsize() would probably have been detected earlier if } the sanity checks were unconditional and complete like they were for } ui_proccnt in rev.1.65 of kern_proc.c. The check for ui_proccnt going } negative was removed and the check for ui_proccnt going to precisely 0 } too early was made conditional (turned into a KASSERT()). The check } for ui_sbsize going negative never existed and the check for ui_sbsize } going to precisely 0 too early was always a KASSERT(). My patch has KASSERT checks that look for ui_sbsize or ui_proccnt going negative, or not being zero when freeing the uidinfo structure. I could be convinced to make this checks mandatory. I'm planning on commiting my patch to -current sometime today, assuming I can get a satisfactory review. I'd like to commit it to -stable after a few days exercise in -current. 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?200009051258.FAA24311>