Date: Wed, 14 Apr 2004 13:51:03 +0200 From: "Devon H. O'Dell" <dodell@sitetronics.com> To: Jilles Tjoelker <jilles@stack.nl> Cc: freebsd-arch@freebsd.org Subject: Re: [patch] lockf(3) user-exploitable kernel panic Message-ID: <407D25A7.8090502@sitetronics.com> In-Reply-To: <20040414112800.GA69649@stack.nl> References: <407CF5B8.2060909@sitetronics.com> <20040414112800.GA69649@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?407D25A7.8090502>