Re: [PATCH v3 04/11] arm64, mm: Move generic mmap layout functions to mm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/18/19 10:19 AM, Kees Cook wrote:
On Thu, Apr 18, 2019 at 12:55 AM Alex Ghiti <alex@xxxxxxxx> wrote:
Regarding the help text, I agree that it does not seem to be frequent to
place
comment above config like that, I'll let Christoph and you decide what's
best. And I'll
add the possibility for the arch to define its own STACK_RND_MASK.
Yeah, I think it's very helpful to spell out the requirements for new
architectures with these kinds of features in the help text (see
SECCOMP_FILTER for example).

I think CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT should select
CONFIG_ARCH_HAS_ELF_RANDOMIZE. It would mean moving

I don't think we should link those 2 features together: an architecture
may want
topdown mmap and don't care about randomization right ?
Given that the mmap randomization and stack randomization are already
coming along for the ride, it seems weird to make brk randomization an
optional feature (especially since all the of the architectures you're
converting include it). I'd also like these kinds of security features
to be available by default. So, I think one patch to adjust the MIPS
brk randomization entropy and then you can just include it in this
move.

Actually, I had to add those ifdefs for mmap_rnd_compat_bits, not
is_compat_task.
Oh! In that case, use CONFIG_HAVE_ARCH_MMAP_RND_BITS. :) Actually,
what would be maybe cleaner would be to add mmap_rnd_bits_min/max
consts set to 0 for the non-CONFIG_HAVE_ARCH_MMAP_RND_BITS case at the
top of mm/mmap.c.

I really like this clean-up! I think we can move x86 to it too without
too much pain. :)

Hi,

Just a short note to indicate that while working on v4, I realized this series had a some issues:

- I broke the case ARCH_HAS_ELF_RANDOMIZE selected but not
  ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT (which can happen on arm without MMU for example) - I use mmap_rnd_bits unconditionnally whereas it might not be defined (it works for all arches I moved though)

The only clean solution I found for the first problem is to propose a common implementation for arch_randomize_brk and arch_mmap_rnd, which is another series on its own and another good cleanup since every architecture uses
the same functions (except ppc, but that can be workarounded easily).
Just like moving x86 deserves its own series since it adds up 8/9 commits.
I am on vacations for 2 weeks, so I won't have time to submit another patchset before, sorry about that.

Thanks,

Alex




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux