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

dodell_at_sitetronics.com
Date: 04/15/04

  • Next message: Dag-Erling Smørgrav: "installing multiple kernels"
    To: eivind@freebsd.org, dodell@sitetronics.com, jilles@stack.nl, freebsd-arch@freebsd.org
    Date: Thu, 15 Apr 2004 08:43:12 -0400
    
    

    Original Message:
    -----------------
    From: Eivind Eklund eivind@FreeBSD.org
    Date: Thu, 15 Apr 2004 11:06:22 +0000
    To: dodell@sitetronics.com, jilles@stack.nl, freebsd-arch@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/ .

    _______________________________________________
    freebsd-arch@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-arch
    To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"


  • Next message: Dag-Erling Smørgrav: "installing multiple kernels"

    Relevant Pages