lkml.org 
[lkml]   [2012]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC] c/r: prctl: Add ability to set new mm_struct::exe_file v3
    On Fri, Mar 09, 2012 at 03:26:20PM +0100, Oleg Nesterov wrote:
    > >
    > > To be sure it's not increased somewhere else before
    > > down_write taken.
    >
    > Who can do this? Only another CLONE_VM thread. And _only_ if we
    > already have the bug in mm_exe accounting logic. And only if that
    > thread does something to trigger the bug in the small window
    > between.

    ok, agreed.

    > > > into removed_exe_file_vma().
    > >
    > > This one looks like a good idea for me -- it's cheap and
    > > not a hot path.
    >
    > But not in this patch, please.
    >

    Sure.

    > > > But imho your WARN looks like "OK, I checked it lockless but I
    > > > am not sure this is correct".
    > >
    > > Oleg, I bet if someone will be changing num_exe_file_vmas overall
    > > idea -- this prctl code will be fixed at last moment (if ever) only
    > > because it's very specific, so I wanted to not miss such moment
    > > and add some check that the rest of the kernel is in a good state.
    > > This test is cheap but may prevent potential problem if one day
    > > mm::exe_file concept will be reworked.
    >
    > The test is cheap indeed. If you mean performance-wise.
    >
    > But it looks confusing, imho. I do not care about a couple of CPU
    > cycles. The code should be optimized for the reading in the first
    > place, not for executing ;) Imho, of course.
    >
    > And once again. Following your logic you need another WARN_ON()
    > right after we drop mmap_sem. Why? To be sure it's not increased
    > somewhere else _after_ down_write taken. And another one after
    > fput.
    >
    > Sure, bugs are possible. And yes, in theory this WARN_ON() can
    > catch some problem. But there is tradeoff. Given that you need
    > another thread to trigger the (potential) bug and the window is
    > tiny, how high do you estimate the probability it can help?
    >
    > > Sure I can simply drop this WARN_ON ;)
    >
    > Oh, keep it if you like it ;)
    >
    > Yes I hate it, but you are the author and this is almost cosmetic.

    OK, Oleg, can't argue, you've convinced me ;) I'll drop this WARN_ON.
    Would it be enough for your Reviewed-by tag? /me hides

    Cyrill


    \
     
     \ /
      Last update: 2012-03-09 15:45    [W:0.023 / U:0.200 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site