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

[Devon H. O'Dell]

Brian F. Feldman wrote:
> "dodell at sitetronics.com" <dodell at sitetronics.com> wrote:
> 
>>>>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.
>>>
>>>If it is easy, it might be worthwhile to patch the shells to use
>>>libutil and submit those patches back to the maintainers.
>>
>>There are a huge number of shells to do this with. This subsystem
>>looks like somewhat of a kludge to me in this respect; the
>>functionality is plainly provided in libutil, while every shell (sh
>>and tcsh included) have their own implementations. limits(1)
>>even has statically compiled information about the limits for
>>every shell it is aware of (including sh, csh, tcsh, bash/bash2
>>and a good few others). I'll take a look at these later. 
> 
> 
> Thanks for doing this work, Devon!  The most important part is for
> /etc/login.conf to allow you to configure the maximum limits -- all the 
> shell stuff is really secondary.
> 

Hrm, it seems that my last email went to /dev/null, so I'll write it 
again. :)

I'm glad to have done this work, and I hope I can help out in the future 
with squashing more bugs :)

I don't know who's taken a look at the patch, but it's available at 
http://freebsd0.sitetronics.com/~dodell/patches/lockfix.tar.gz. 
login.conf limits are already taken care of; so are libutil, limit(1), 
tcsh and sh.

Regarding Linux compatibility: it seems to me that Linux limits the 
number of flock-style locks as well. This seems unnecessary as that is 
effectively limited by the maximum open files rlimit (since these types 
of locks are one-per-file). Still, if we wish to be compatible, the 
patch can be modified to affect locks of all types, though not easily. 
BSD-style locks (flock(2)) don't contain process information in the 
lf_id field, unlike POSIX locks, which means that keeping track of them 
per-process can get difficult. Since they're limited by the 
maxfilesperproc and maxprocesses anyway, it seems a bit overkill to 
introduce a manner to track these locks on a per-process basis. As long 
as an administrator keeps these limits to sane values, there is no 
reason that flock(2)-style locks should pose a problem.

OTOH, the lockf(3) (POSIX-style) locks can easily be limited per 
process; this would simply remove the per-user checks and counts in my 
code (and fix the fact that change_ruid() needs a struct proc *). Extra 
sanity checks for fork(2) calls are unnecessary as POSIX locks aren't 
inherited.

Again, any and all feedback would be appreciated. What do I need to do 
to get this all squared away and ready for commitment. (I'll generate 
patches for all non-EOLed systems from the final patch.) :)

This has been a fun experience and I hope to continue to be able to 
contribute to the project again soon :)

Kind regards,

Devon H. O'Dell