lkml.org 
[lkml]   [2011]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] x86, AMD: Correct F15h IC aliasing issue
    On Sun, Jul 24, 2011 at 10:22 AM, Borislav Petkov <bp@amd64.org> wrote:
    >
    >> So at a MINIMUM, I would say that this is acceptable only when the
    >> process doing the allocation hasn't got ASLR disabled.
    >
    > I guess I could look at randomize_va_space before enabling it.

    That's not what I meant - I meant the per-process PF_RANDOMIZE and
    ADDR_NO_RANDOMIZE personality flags (although the global
    "randomize_va_space" thing obviously is one input to that too)

    In fact, if 99% of your problem is ASLR-induced, might I suggest just
    making the whole thing a tweak to ASLR instead, and not use ASLR for
    bits 14:12? That should be fundamentally much safer: it doesn't change
    any semantics at all, it just makes for slightly less random bits to
    be used.

    So I really think that you might be *much* better off just changing
    mmap_rnd(), and nothing else. Just make *that* mask off the three low
    bits of the random address, ie something like

    diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
    index 1dab5194fd9d..6b62ab5a5ae1 100644
    --- a/arch/x86/mm/mmap.c
    +++ b/arch/x86/mm/mmap.c
    @@ -90,6 +90,9 @@ static unsigned long mmap_rnd(void)
    rnd = (long)get_random_int() % (1<<8);
    else
    rnd = (long)(get_random_int() % (1<<28));
    +
    + if (avoid_aliasing_in_bits_14_12)
    + rnd &= ~7;
    }
    return rnd << PAGE_SHIFT;
    }

    would be fundamentally very safe - it would already take all our
    current anti-randomization code into account.

    No?

    > But this won't address the case where one of the processes was created
    > with ASLR off and the other with ASLR on and they map the same library
    > at VAs differing at bits [14:12].

    I wouldn't worry about some corner-case like that _nearly_ as much as
    worrying about the non-ASLR process working at all.

    > Yeah, I like the BITS() thing - will change. I actually have a similar
    > macro GENMASK(o, hi) in <drivers/edac/amd64_edac.h> - I should move it
    > to <linux/bitops.h> and rename it to BITS().

    So it may be that BITS() is much too generic a name, and will cause
    problems. A quick "git grep -w BITS" certainly finds a fair number of
    hits. So I don't think it's usable as-is, it was meant more as
    pseudo-code.

    >> Changing address space layout is not a small decision.
    >
    > I suspected as much - thus the boot option to disable it.

    I understand that the boot option is worth it, but since we _already_
    have a way to mark binaries as not wanting address space layout
    changes, I really think it should use that as the primary method. When
    that bit is set, I think it's a big hint that the process is "fragile"
    wrt address space changes.

    A boot option might be left as a last ditch thing, but I don't think
    it should be the primary model.

    Linus


    \
     
     \ /
      Last update: 2011-07-24 19:43    [W:0.023 / U:33.692 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site