[lkml]   [2012]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 1/9] exec: add a global execve counter
    On Sun, Mar 11, 2012 at 04:42:37PM -0700, Linus Torvalds wrote:
    > On Sun, Mar 11, 2012 at 4:32 PM, Djalal Harouni <> wrote:
    > >>
    > >> Just increment the mm_count for the thing, and hold a reference to it,
    > >> and now you're all done.
    > > Please Linus have you checked the:
    > > [PATCH 9/9] proc: improve and clean up /proc/<pid>/mem protection
    > >
    > > That keeping the mm struct wont work, since it will eat memory and the
    > > OOM-killer will kill some innocent processes, and the abuse can only be
    > > catched by the VFS.
    > That's the point. I made the mistake of using mm_users initially, but
    > ysing mm_count - which is what I said to use (and what Oleg fixed
    > things to in commit 6d08f2c71397) should *not* have that problem. It
    > just keeps the 'struct mm_struct' itself around.
    And that mm_struct will explode and only the VFS will catch it.

    Given 1024 processes * (RLIMIT_NOFILE 1024 - 3) == ~1020000

    more than 1020000 mm structs (all of dead processes ?)

    A quick test on a default ubuntu:
    cat /proc/sys/fs/file-max

    So we are able to keep around 388411 dead mm_struct in memory, just try it.

    Our embedded devices will suffer, serial login will be killed, getty, ...
    ssh root owned ... I've experienced this.

    I'll try to post more numbers.

    > > What's your opinion on it ?
    > What's the advantage? You replace it with *another* allocation, and a
    That allocation is much smaller and it can be used to protect all
    /proc/<pid>/* files in a unified way. Consistency seems the right thing
    to do, even the /proc/<pid>/{syscall,stack,...} that do not operate on mm
    structs will be protected. I don't know how you will protect these files ?

    We can even remove the allocation and just reference the exec_id or use
    other fields of the file struct.

    I'm must admit that grsecurity just did the check in the seq files, they
    did not add anything, this can leave some other files unprotected, but it
    is cheaper.

    > 64-bit thing that is much less useful.
    Well, it protect as from wraps.
    From the previous discussions it seems that perhaps we can work around it
    or have an agreement.

    > The size of the patch also speaks for itself:
    > fs/proc/base.c | 99 ++++++++++++++++++++++++++++++++++++++++++++------------
    That's not fair since most of the logic of /proc/<pid>/* files is in
    fs/proc/base.c and we can clean the code, there are some duplicated
    functions in my patch, which we can improve.

    > and it's more complex and uses more memory on average (the refcount
    > thing is *free* for usual cases).
    > I do agree that it would be nicer if mm_struct was a bit smaller, but
    > at the same time, I really don't see the advantage of replacing it
    > with another allocation entirely that makes the code just more
    > complicated.
    As I've said we can improve it.

    I'll take all the feedback I got and re-work the patches, in the meantime
    some of these sensitive files need to be protected especially the

    I'll re-submit.

    Thanks for the feedback.

    > Linus
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to
    > More majordomo info at
    > Please read the FAQ at


     \ /
      Last update: 2012-03-12 01:25    [W:0.023 / U:324.888 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site