| Message ID | CAG48ez0QRmU9fGV3x7nceGqBGADNTb66MnG4PW_xu+xnGUqg=g@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM64 suggestion: reduce the compat address limit (currently 0x100000000)? | expand |
Hi Jann, On Wed, Jan 30, 2019 at 05:32:00PM +0100, Jann Horn wrote: > At the moment, compat tasks running on ARM64 can allocate memory up to > 0x100000000 (TASK_SIZE_32). Testing on an Android device (with an > admittedly somewhat old kernel): [...] > ffff1000-100000000 rw-p 00000000 00:00 0 > > This means that mmap() allocations do not adhere to section 6.5.8 of > C99 ("If the > expression P points to an element of an array object and the > expression Q points to the > last element of the same array object, the pointer expression Q+1 > compares greater than > P.") if you treat mmap() allocations as returning an array. Oh, good point. > In practice, I've also seen code that does things like computing a > pointer that is out of bounds by a few bytes and then comparing it > against the end of the array; while this is UB according to C99, it > probably makes sense to try to avoid breaking such code. Agreed, and since the current behaviour isn't something you can portably rely on anyway, I think we're ok to change it. > X86-64's compat code uses the limit 0xFFFFe000 (IA32_PAGE_OFFSET), > which I think makes more sense. Would it make sense to do something > like the following (untested)? Can't we just go with 0x100000000 - PAGE_SIZE instead? Will
On Wed, Jan 30, 2019 at 7:22 PM Will Deacon <will.deacon@arm.com> wrote: > On Wed, Jan 30, 2019 at 05:32:00PM +0100, Jann Horn wrote: > > At the moment, compat tasks running on ARM64 can allocate memory up to > > 0x100000000 (TASK_SIZE_32). Testing on an Android device (with an > > admittedly somewhat old kernel): > > [...] > > > ffff1000-100000000 rw-p 00000000 00:00 0 > > > > This means that mmap() allocations do not adhere to section 6.5.8 of > > C99 ("If the > > expression P points to an element of an array object and the > > expression Q points to the > > last element of the same array object, the pointer expression Q+1 > > compares greater than > > P.") if you treat mmap() allocations as returning an array. > > Oh, good point. > > > In practice, I've also seen code that does things like computing a > > pointer that is out of bounds by a few bytes and then comparing it > > against the end of the array; while this is UB according to C99, it > > probably makes sense to try to avoid breaking such code. > > Agreed, and since the current behaviour isn't something you can portably > rely on anyway, I think we're ok to change it. > > > X86-64's compat code uses the limit 0xFFFFe000 (IA32_PAGE_OFFSET), > > which I think makes more sense. Would it make sense to do something > > like the following (untested)? > > Can't we just go with 0x100000000 - PAGE_SIZE instead? Ah, yeah, sounds good. I think x86 probably only chose 0xFFFFe000 to keep user allocations out of the way for the 32-bit vsyscall page.
===========
$ cat mmap_filler.c
#include <sys/mman.h>
#include <unistd.h>
#include <err.h>
#include <stdlib.h>
#include <fcntl.h>
int main(void) {
unsigned long size = 1UL<<31;
while (size >= 0x1000UL) {
void *res = mmap(NULL, size, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0);
if (res == MAP_FAILED) size >>= 1;
}
int fd = open("/proc/self/maps", O_RDONLY);
if (fd == -1) err(1, "open");
while (1) {
char buf[0x1000];
ssize_t res = read(fd, buf, sizeof(buf));
if (res == -1) err(1, "read");
if (res == 0) _exit(0);
ssize_t wres = write(1, buf, res);
if (wres != res) errx(1, "write failed");
}
}
$ arm-linux-gnueabi-gcc -static -o mmap_filler mmap_filler.c && adb
push mmap_filler /data/local/tmp/ && adb shell
/data/local/tmp/mmap_filler
mmap_filler: 1 file pushed. 16.3 MB/s (587008 bytes in 0.034s)
00008000-00010000 rw-p 00000000 00:00 0
00010000-00089000 r-xp 00000000 103:1d 2080772
/data/local/tmp/mmap_filler
00089000-00098000 rw-p 00000000 00:00 0
00098000-0009a000 rw-p 00078000 103:1d 2080772
/data/local/tmp/mmap_filler
0009a000-0009b000 rw-p 00000000 00:00 0
0009b000-00752000 rw-p 00000000 00:00 0 [heap]
00752000-00774000 rw-p 00000000 00:00 0 [heap]
00774000-f081b000 rw-p 00000000 00:00 0 [heap]
f081b000-f081c000 r--p 00000000 00:00 0 [vvar]
f081c000-f081e000 r-xp 00000000 00:00 0 [vdso]
f081e000-ff710000 rw-p 00000000 00:00 0
ff810000-ff831000 rw-p 00000000 00:00 0 [stack]
ff831000-ffff0000 rw-p 00000000 00:00 0
ffff0000-ffff1000 r-xp 00000000 00:00 0
[kuserhelpers]
ffff1000-100000000 rw-p 00000000 00:00 0
===========
This means that mmap() allocations do not adhere to section 6.5.8 of
C99 ("If the
expression P points to an element of an array object and the
expression Q points to the
last element of the same array object, the pointer expression Q+1
compares greater than
P.") if you treat mmap() allocations as returning an array.
In practice, I've also seen code that does things like computing a
pointer that is out of bounds by a few bytes and then comparing it
against the end of the array; while this is UB according to C99, it
probably makes sense to try to avoid breaking such code.
X86-64's compat code uses the limit 0xFFFFe000 (IA32_PAGE_OFFSET),
which I think makes more sense. Would it make sense to do something
like the following (untested)?
==============
diff --git a/arch/arm64/include/asm/processor.h
b/arch/arm64/include/asm/processor.h
index f1a7ab18faf3..a0c7eb1358dd 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -57,7 +57,7 @@
#define TASK_SIZE_64 (UL(1) << vabits_user)
#ifdef CONFIG_COMPAT
-#define TASK_SIZE_32 UL(0x100000000)
+#define TASK_SIZE_32 UL(0xFFFFe000)
#define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \
TASK_SIZE_32 : TASK_SIZE_64)
#define TASK_SIZE_OF(tsk) (test_tsk_thread_flag(tsk, TIF_32BIT) ? \