Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
From:   Keno Fischer <keno@juliacomputing.com>
Date:   Wed, 16 Dec 2020 20:29:30 -0500
Message-ID: <CABV8kRwoHAAdez8k60O+AJ9E3g5_PM0F6tpbpB9dC115_FD3Eg@mail.gmail.com>
Subject: brk checks in PR_SET_MM code
To:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Cc:     gorcunov@openvz.org, Andrew Morton <akpm@linux-foundation.org>,
        mkoutny@suse.com, ktkhai@virtuozzo.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi all,

The code in prctl(PR_SET_MM, ...) performs a number of sanity checks,
among them

```
/*
 * @brk should be after @end_data in traditional maps.
 */
if (prctl_map->start_brk <= prctl_map->end_data ||
    prctl_map->brk <= prctl_map->end_data)
goto out;
```

The original commit that introduces this check
(f606b77f1a9e362451aca8f81d8f36a3a112139e) says:

```
4) As in regular Elf loading procedure we require that @start_brk and
   @brk be greater than @end_data.
```

However, it does not appear that this invariant is actually
enforced during regular ELF loading. In particular, at least on my
linux distribution, it does not appear to be satisfied when
invoking the dynamic linker directly.
For example, consider the following test application:

```
#include <sys/prctl.h>
#include <unistd.h>
#include <assert.h>

int main(void) {
    int err = prctl(PR_SET_MM, PR_SET_MM_BRK, sbrk(0), 0, 0);
    assert(err == 0);
    return 0;
}
```
```
$ su
# ./a.out
# /lib64/ld-linux-x86-64.so.2 ./a.out
a.out: test.c:7: main: Assertion `err == 0' failed.
Aborted
```

I don't understand this code well enough to know what the
intended behavior is, but unfortunately this causes some
processes to be non-restorable using the PR_SET_MM
mechanism, which defeats the whole purpose of that API.
Could somebody clarify whether this situation is indeed
supposed to be impossible and if not whether said checks
in PR_SET_MM are actually supposed to be there?
I suppose this is also technically a regression when the
old PR_SET_MM commands were refactored to use this
new validation. Previously only the commands that changed
the brk validated this invariant, but these days it tries
to validate the entire structure at once, so all the PR_SET_MM
calls will fail in a process whose layout violates the sanity
check.

Thanks,
Keno