[RFC PATCH 0/3] Improved ALSR

[Topi Miettinen]

On 23.11.2020 18.41, Szabolcs Nagy wrote:
> The 11/23/2020 18:06, Topi Miettinen via Libc-alpha wrote:
>> No comments at all? I see several implementation options here:
>>
>> 1. Always use mmap() instead of sbrk(), delete any uses of sbrk()
>>
>> I have hard time thinking why sbrk() would ever be the preferred choice over
>> mmap(), especially considering security. There may be some bytes wasted, so
> 
> i'm not against using mmap instead brk in malloc
> but the latter has more overhead so such change
> should be measured.

This test shows 48% increase when using mmap() vs. sbrk():

$ cat malloc-vs-sbrk.c
#include <sys/mman.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#define ROUNDS 1000000
#define SIZES 4
#define SIZE_FACTOR 4

int main(int argc, char **argv) {
         if (argc == 2) {
                 for (int i = 0; i < ROUNDS; i++) {
                         for (int j = 0; j < SIZES; j++) {
                                 size_t s = 4096 * (1 << (j * SIZE_FACTOR));

                                 void *ptr = mmap(NULL, s, PROT_READ | 
PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
                                 if (ptr == MAP_FAILED) {
                                         fprintf(stderr, "mmap() failed, 
size %zu iter %d\n", s, i);
                                         return 1;
                                 }
                                 munmap(ptr, s);
                         }
                 }
         } else {
                 for (int i = 0; i < ROUNDS; i++) {
                         for (int j = 0; j < SIZES; j++) {
                                 size_t s = 4096 * (1 << (j * SIZE_FACTOR));

                                 void *ptr = sbrk(s);
                                 if (ptr == (void *) -1) {
                                         fprintf(stderr, "sbrk() failed, 
size %zu iter %d\n", s, i);
                                         return 1;
                                 }
                                 sbrk(-s);
                         }
                 }
         }

         return 0;
}
$ time ./malloc-vs-sbrk

real    0m1.923s
user    0m0.160s
sys     0m1.762s
$ time ./malloc-vs-sbrk 1

real    0m2.847s
user    0m0.176s
sys     0m2.669s

>> 2. Conditionally use mmap() instead of sbrk()
>>
>> Something like `#define USE_SBRK`, enabled by `configure` or a header file.
> 
> i think configure time option is not a good idea,
> but e.g. it can be a runtime tunable.

The runtime option needs to be available very early in the dynamic 
loader, before errno and malloc() are available. Would getenv() work?

>> I've been using a patched glibc for a month without seeing problems. I
>> enabled audit logging for the brk() system call and installed a global
>> seccomp filter (in initrd) which returns ENOSYS to catch any uses. So far
>> I've only noticed that cpp (used by X11 startup in addition to compiling)
>> calls sbrk() to check memory usage. Perhaps it should use official malloc
>> statistics interface instead, since malloc() may use mmap() for other
>> reasons and then sbrk() won't return true data.
> 
> sbrk should continue to work even if glibc itself
> does not use it internally, that's public api/abi.

Yes, the patches don't remove the API/ABI.

-Topi