From owner-freebsd-arch@FreeBSD.ORG Wed Apr 14 04:54:47 2004 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3AC4B16A4CE for ; Wed, 14 Apr 2004 04:54:47 -0700 (PDT) Received: from smtp1.euronet.nl (smtp1.euronet.nl [194.134.35.133]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0673243D5D for ; Wed, 14 Apr 2004 04:54:47 -0700 (PDT) (envelope-from dodell@sitetronics.com) Received: from sitetronics.com (zp-c-13e65.mxs.adsl.euronet.nl [81.69.92.101]) by smtp1.euronet.nl (Postfix) with ESMTP id E1D8567199; Wed, 14 Apr 2004 13:54:45 +0200 (MEST) Message-ID: <407D25A7.8090502@sitetronics.com> Date: Wed, 14 Apr 2004 13:51:03 +0200 From: "Devon H. O'Dell" User-Agent: Mozilla Thunderbird 0.5 (Windows/20040207) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Jilles Tjoelker References: <407CF5B8.2060909@sitetronics.com> <20040414112800.GA69649@stack.nl> In-Reply-To: <20040414112800.GA69649@stack.nl> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit cc: freebsd-arch@freebsd.org Subject: Re: [patch] lockf(3) user-exploitable kernel panic X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Apr 2004 11:54:47 -0000 Jilles Tjoelker wrote: > e) add a line 'struct proc;' to sys/ucred.h Thanks for this suggestion; I wasn't aware that this was reasonably possible from an architectural standpoint. >>3) Does this work justify my going through the modified files and doing >>style(9) changes on them? I'm willing to do this; mux@ has encouraged >>it; style(9) suggests that I do it if my code comprises 50% or more of >>the new files (which it doesn't). Again, if this is useful, I'll >>certainly do this. > > > Some of the files have a mixture of K&R-style and ANSI function > definitions. I'll look into implementing style(9) changes then. I know my patch fails a style(9) check in some contexts, so I'll go a general cleanup as well. >>8) Are any of the modifications I've made too intrusive to the >>[proc|advlock|rlimit|sysctl] subsystem(s)? > > > Rather a lot of functions and programs (setusercontext(3) in libutil, > limits(1), rlimit-related builtins in all shells) have knowledge of all > the rlimits built into them. This is already a bit of a problem, for > example bash doesn't support the socket buffer size rlimit. Also note > that those programs often use single letters for the rlimits. My fix for sh's builtin(1) ulimit feature does implement a single letter feature for limiting locks. In my reading of the sh ulimit code, I saw that these limits are really not dynamic at all... one is thus unable to create a generic interface for use in all shells, although it appears that libutil tries to provide this interface. It's a pity that it's not used in the contexts it should be. >>9) What (extra) suggestions would you have for my patches for relevant >>manpages? > > >>10) Have I missed any userland utilities that don't use libutil to >>check/set classes/limits (perhaps there are some in ports that I can >>patch as well)? > > > limits(1), all shells. sh has been fixed. I was under the impression that csh used libutil for this (libutil has been fixed). I'll take a deeper look into shells in base and in ports and figure out what changes I need to make there. While I'm at it, I don't think it'd be a bad idea to go ahead and build in the RLIMIT_SBSIZE to bash and bash2. I'm not entirely sure what information I need to list in all the manpages, but I'll get that sorted out. >>This patch is against April 13th -CURRENT but backporting it is very >>simple since the main affected subsystem doesn't change much >>architecturally / structurally. However, this also brings into light >>that this problem may also affect the other BSDs (Dragonfly, Net, Open, >>Ekko). I cannot verify this as I do not have much experience with these >>other BSDs and do not know if they impose any limits on the amount of >>kernel memory a user can have or any other limits which would disallow >>this to exploit to ``work''. Should they be affected, what do I need to >>do to alert them of this? > > > Limiting the number of locked regions is not uncommon, e.g. Solaris does > it (the manpage seems to indicate a per-system limitation only, though). > > Interesting part from Linux getrlimit(2) manpage: > RLIMIT_LOCKS > A limit on the combined number of flock() locks and fcntl() > leases that this process may establish (Linux 2.4 and later). > > Per-user instead of per-process limits are harder to implement but > more effective. Ok. I was not aware that Linux had this fix / feature already. I'll take a look into the CVS repos of the other BSDs and see if it's something I can suggest a patch for in those worlds. The reason I asked was because I don't have access to many boxes of different architectures or operating systems. Indeed, my patch implements per-user limits, but keeps track per-process for the purpose of removing locks on a setuid() call. In the beginning, I thought that it would be necessary for process termination, but since fdfree() is called in kern_exit.c and the locks are released sequentially across the whole file (the whole file is unlocked and the for (;;) code in kern_lockf.c will unlock this), I see that it is unnecessary for this purpose. Are there ideas on how I could implement the lock transfer between users without intruding on the process structure, or is this something that's reasonable? >>[snip] > Refer to fcntl(2) in preference to lockf(3). While lockf(3) locks > typically are implemented using fcntl(2), SUSv3 doesn't say anything > about interaction between the two. Also, lockf(3) is marked XSI, but > fcntl(2) locking is not. Point taken. > The sysctl(3) and sysctl(8) manpages haven't been updated, but I'm not > sure whether that's useful. Right. I'll need to list my new sysctl. Thanks for the reminder. Thanks for the feedback, Jilles. I really appreciate the architectural help and explinations you've given to me both here on-list and on Freenode. I'll let you guys know when I've an updated patch incorporating these changes. Kind regards, Devon H. O'Dell