Courtesy of JeremyRand, there appears to be an issue on ppc64el systems when using fixed vm.mmap_rnd_bits and vm.mmap_rnd_compat:
A solution is proposed in this PR:
It works normally on x64, however, I do not have access to ppc64el or x86 machines to test compatibility.
Overall, besides the inclusion of very minor additional comments, I think this is a good idea. Thoughts?
Hi @raja , thanks for testing on x86_64. Happy to answer any questions about the fix that might facilitate review. For reference, x86 32-bit, ARM 32-bit, and ARM 64-bit are (per my reading of the Linux source code) also broken in the status quo, and should be fixed by this PR (but I wasn’t able to test any of them with or without this PR). See the following values of ARCH_MMAP_RND_BITS_MAX:
I didn’t look at the kernel source for rarer arches such as RISC-V, SPARC, etc, but it’s likely that they’re broken in the status quo and will be fixed by this PR too.
/usr/libexec/security-misc/mmap-rnd-bits
/usr/libexec/security-misc/mmap-rnd-bits || true
This is to avoid the package installing just because of an issue in that script which does not seem important enough to break the package management as a warning if that happens.
if CONFIG=$(ls -1 -t /boot/config-* | head -n 1)
This needs some more defensive programming to avoid an unhanded error message by ls if there are no files in /boot/config-* which can happen on some systems such as Qubes (or maybe OpenVZ, other containers) were the kernel is provided through other means such as on the command line.
Also even if the file is there it might not be readable (test -r) and even if readable its contents my be corrupted / garbage.
BITS_MAX and COMPAT_BITS_MAX need to be guaranteed to always have a useful default value even if grep fails and finds nothing. So if the variable is empty, not an integer or too large, please fall back to a built-in default (+ warning message?).
echo "${SYSCTL}" > /etc/sysctl.d/30_security-misc_aslr-mmap.conf
echo "${SYSCTL}" | tee /etc/sysctl.d/30_security-misc_aslr-mmap.conf >/dev/null
This is to have a proper pipe that would fail if the file is unwriteable (which could happen in some corner cases. Sometimes I have witnessed devices being re-mounted read-only on hardware issues, maybe some implementations of live mode, or if that file was set to immutable for some reason).
Good review, thanks @Patrick . I’ll look into making the requested changes, may take me a few days.
Updated the PR. I think I’ve addressed @Patrick’s review, sorry this took so long. Let me know if I missed anything.
As an aside: it would be cool if there were a way to do the kernel config detection at boot time rather than install time. I was unable to find a way to do this, but if there are any kernel wizards here who know a way to do it, that would be very welcome. (I don’t think this should block the PR I submitted though.)
Merged. Thanks!
if compgen -G "/boot/config-*" > /dev/null && CONFIG=$(ls -1 -t /boot/config-* | head -n 1) ; then
And then showing an error if no Linux config files exist is an issue because there are environments where no Linux kernel is installed. (Such as chroot, OpenVZ and whatnot.) Therefore it cannot be a strong error message in that case. Therefore downgraded to an INFO level message.
Added a few commits on top.
Also in Qubes Templates and App Qubes there’s in in-VM kernel by default at time of writing. So this is a common case.
Forgot about README.md. Could you document this functionality in security-misc/README.md at master · Kicksecure/security-misc · GitHub please?
The existing docs say this:
The bits of entropy used for mmap ASLR are increased, therefore improving its effectiveness.
Which seems to still be accurate, though maybe not as detailed as they could be. Do you want me to expand that bullet point to clarify that it’s increased to the maximum allowed by the kernel config? Or is there some other edit you’re looking for?
Would be nice to have a style comparable to other readme entries which reference all the file names which implement this.
Does this do what you want? README: Document mmap-rnd-bits by JeremyRand · Pull Request #124 · Kicksecure/security-misc · GitHub
Reviewed, here are the changes I made.
https://bitbucket.org/uncut-grass/security-misc/commits/f176677e18fc437a0c3cf15528bdf702d47fc874
Broken link.