mmap(2): fix MAP_32BIT when ASLR is disabled

[kib]

Summary

After the specified revision, vm_map_object() with fitit == true passes
min_addr equal to the end of the data segment, to vm_map_find_min().
This breaks MAP_32BIT since min_addr > max_addr. Make MAP_32BIT a
special case for kern_mmap() by setting min_addr to vm_map_min().

Reported by:    dchagin
Fixes:  d8e6f4946cec0b84a6997d62e791b8cf993741b2

[markj]

sys/vm/vm_mmap.c

1628 ↗

(On Diff #125057)

This computation is thrown away in the MAP_32BIT case. You could add an else clause to avoid that.

[kib]

Add the else statement.

[alc]

I'd like to suggest this patch instead:

diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
index 444e09986d4e..eb607d519247 100644
--- a/sys/vm/vm_map.c
+++ b/sys/vm/vm_map.c
@@ -2255,10 +2255,10 @@ vm_map_find_min(vm_map_t map, vm_object_t object, vm_ooffset_t offset,
        int rv;
 
        hint = *addr;
-       if (hint == 0)
+       if (hint == 0) {
                cow |= MAP_NO_HINT;
-       if (hint < min_addr)
                *addr = hint = min_addr;
+       }
        for (;;) {
                rv = vm_map_find(map, object, offset, addr, length, max_addr,
                    find_space, prot, max, cow);

This patch also restores the ability of non-MAP_32BIT mmap()s to allocate starting from hints below the text segment.

[kib]

In D41159#937487, @alc wrote:

I'd like to suggest this patch instead:

diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
index 444e09986d4e..eb607d519247 100644
--- a/sys/vm/vm_map.c
+++ b/sys/vm/vm_map.c
@@ -2255,10 +2255,10 @@ vm_map_find_min(vm_map_t map, vm_object_t object, vm_ooffset_t offset,
        int rv;
 
        hint = *addr;
-       if (hint == 0)
+       if (hint == 0) {
                cow |= MAP_NO_HINT;
-       if (hint < min_addr)
                *addr = hint = min_addr;
+       }
        for (;;) {
                rv = vm_map_find(map, object, offset, addr, length, max_addr,
                    find_space, prot, max, cow);

This patch also restores the ability of non-MAP_32BIT mmap()s to allocate starting from hints below the text segment.

Please go ahead with your fix.

[alc]

sys/vm/vm_mmap.c

362 ↗

(On Diff #125091)

Shouldn't this expression include the size? Otherwise, if the given address is close to vm_taddr, such that there isn't enough space between the given address and vm_taddr, the find space code will select an unused address range within the legacy heap region.

[kib]

sys/vm/vm_mmap.c

362 ↗

(On Diff #125091)

Isn't it the reverse? If the hint falls into the legacy heap, we must move it.

If addr + size is compared with the text address, instead of add, we might end up allowing allocation below text, for sufficiently large size.

[alc]

sys/vm/vm_mmap.c

362 ↗

(On Diff #125091)

No, I don't think that I have it backwards. Setting aside the possibility of overflow by addr + size for the moment, if addr > vm_taddr, then so is addr + size. However, suppose that vm_taddr is 0x200000, addr is 0x1ff000, and size is 8192. Today, I believe that would result in the allocation of an address range in the legacy heap.

[kib]

sys/vm/vm_mmap.c

362 ↗

(On Diff #125091)

I agree.