[patch] lockf(3) user-exploitable kernel panic

[Devon H. O'Dell]

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