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

[dodell at sitetronics.com]

Original Message:
-----------------
From: Eivind Eklund eivind at FreeBSD.org
Date: Thu, 15 Apr 2004 11:06:22 +0000
To: dodell at sitetronics.com, jilles at stack.nl, freebsd-arch at FreeBSD.org
Subject: Re: [patch] lockf(3) user-exploitable kernel panic


On Wed, Apr 14, 2004 at 01:51:03PM +0200, Devon H. O'Dell wrote:
>> 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.
>
>Most of the sys/* files are really owned by the implementation, and it
>is usually OK to introduce forward declarations into them.  We try to
>avoid namespace poisoning (introducing unknown variables) for the
>official files, but that also happens sometimes.
>
>Also, many of the files (including sys/ucred.h) has an #ifdef _KERNEL
>section.  This section is totally owned by the implementation, and it is
>(almost) always OK to add forward declarations.
>
>The list of official sys/ includes can be fetched at 
>http://www.opengroup.org/onlinepubs/007904975/basedefs/sys/

Thanks for the clarification here, Eivind. When implementing the 
declaration in my current patch, I noticed that sys/ucred.h had done
this as well with struct thread.

>> [snip]
>> 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.
>
>Please do that separately from the main patch.  We try quite hard to not
>mix stylistic and functional changes in a single patch, to make it easy
>to use the version history (and easy for people to review the patches).

I was aware of this policy. I'll make aesthetic changes to my current
patch code and styleize the other code in a separate patch.

>> 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. 

>> 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.
>
>It'd be nice to be compatible with Linux here, as it means just a define
>is needed for making apps work with it on FreeBSD (it may even
>automatically happen due to configure scripts.)

My patch implements a per-user setting for this, since I thought that
a malicious user might be able to spawn a large number of 
processes, each eating up n locks. However, considering that POSIX
locks are not inherited between processes and the 
kern.maxprocperuid sysctl, if it's desirable to be compatible with
Linux in this case, I can certainly back out the per-user check and
make it per-process. This would get rid of a good bit of code,
including the setuid()-necessary changes.

What would be a sane value for a per-process setting? Should I 
calculate this value based on available system resources (as 
kern.maxfiles is set)?

>Eivind.

Thanks for the input!

Kind regards,

Devon H. O'Dell

--------------------------------------------------------------------
mail2web - Check your email from the web at
http://mail2web.com/ .