MREMAP_FIXED unmaps dest on error

MREMAP_FIXED unmaps dest on error

[stsp]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

Hello.

The attached test-case demonstrates a
bug in mremap(). If MREMAP_FIXED is used
over an existing mapping and mremap() fails,
destination area gets unmapped.
AFAIK the failed syscall should have no
observable effects.

There is also another bug demonstrated by
the same test-case. Namely, it does 2 subsequent
mprotect()s on the same page, changing the
protection and restoring it back. But VMAs
are not merged, so the subsequent mremap()
fails (and exhibits a bug by unmapping dest).

[-- Attachment #2: mrtst2.c --]
[-- Type: text/x-csrc, Size: 1594 bytes --]

#define _GNU_SOURCE
#include <sys/mman.h>
#include <assert.h>
#include <stdlib.h>
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <signal.h>

#ifndef PAGE_SIZE
#define PAGE_SIZE 4096
#endif

static void my_segv(int sig)
{
    printf ("Test FAILED\n");
    _exit (EXIT_FAILURE);
}

int main(void)
{
    const char *file = "mrtst2.c";
    char *addr, *addr2, *addr3;
    int fd;

    fd = open (file, O_RDONLY);
    if (fd == -1)
        return EXIT_FAILURE;
    addr = mmap (NULL, PAGE_SIZE * 2, PROT_READ, MAP_PRIVATE, fd, 0);
    close(fd);
    if (addr == MAP_FAILED) {
        perror("mmap()");
        return EXIT_FAILURE;
    }
    addr2 = mmap (NULL, PAGE_SIZE * 2, PROT_READ | PROT_WRITE,
                  MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
    if (addr2 == MAP_FAILED) {
        perror("mmap()");
        return EXIT_FAILURE;
    }
    /* split VMA to fail mremap() */
    mprotect (addr, PAGE_SIZE, PROT_READ | PROT_WRITE);
    /* VMAs won't be merged, bug */
    mprotect (addr, PAGE_SIZE, PROT_READ);
    /* make sure dest is here and writable */
    addr2[0] = 3;
    /* expose the bug */
    addr3 = mremap (addr, PAGE_SIZE * 2, PAGE_SIZE * 2,
                    MREMAP_MAYMOVE | MREMAP_FIXED | MREMAP_DONTUNMAP,
                    addr2);
    if (addr3 == addr2) {
        printf ("Even VMAs merged? Excellent!\n");
        return 0;
    }
    assert (addr3 == MAP_FAILED);
    signal (SIGSEGV, my_segv);
    /* see if dest address unmapped */
    addr2[0] = 5;
    assert (addr2[0] == 5);
    printf ("Test PASSED\n");
    return 0;
}

Re: MREMAP_FIXED unmaps dest on error

[David Hildenbrand]

On 30.03.23 17:48, stsp wrote:
> Hello.
> 
> The attached test-case demonstrates a
> bug in mremap(). If MREMAP_FIXED is used
> over an existing mapping and mremap() fails,
> destination area gets unmapped.
> AFAIK the failed syscall should have no
> observable effects.

I remember that holds for various mapping-related syscalls: if something 
goes wrong, the end result is not guaranteed to be what we had before 
the syscall.

For example, if you use mmap(MAP_FIXED) to replace part of an exiting 
mapping, we first munmap what's there and then try to mmap the new 
mapping. If something goes wrong while doing that, we cannot simple undo 
what we already did.

Long story short: the semantics of these syscalls has never been to 
leave the system in the state as it was before in case anything goes wrong.


As another example, if you do an mprotect() that covers multiple VMAS, 
and there is an issue with the last VMA, all but the last VMA will have 
their permissions changed.

-- 
Thanks,

David / dhildenb

Re: MREMAP_FIXED unmaps dest on error

[stsp]

Hi,

03.04.2023 16:58, David Hildenbrand пишет:
> On 30.03.23 17:48, stsp wrote:
>> Hello.
>>
>> The attached test-case demonstrates a
>> bug in mremap(). If MREMAP_FIXED is used
>> over an existing mapping and mremap() fails,
>> destination area gets unmapped.
>> AFAIK the failed syscall should have no
>> observable effects.
>
> I remember that holds for various mapping-related syscalls: if 
> something goes wrong, the end result is not guaranteed to be what we 
> had before the syscall.
>
> For example, if you use mmap(MAP_FIXED) to replace part of an exiting 
> mapping, we first munmap what's there and then try to mmap the new 
> mapping. If something goes wrong while doing that, we cannot simple 
> undo what we already did.
>
> Long story short: the semantics of these syscalls has never been to 
> leave the system in the state as it was before in case anything goes 
> wrong.
>
>
> As another example, if you do an mprotect() that covers multiple VMAS, 
> and there is an issue with the last VMA, all but the last VMA will 
> have their permissions changed.
>
Thanks for info.
Is this documented in a man page?
I wonder how do you deal with mmap() and
mprotect() on such occasions. mremap()
is an extension, but mmap() and mprotect()
are from posix, so is it a compliant impl?

Also my example shows another bug
that the VMAs are not merged after I
restore the protection of one of them,
allowing them to merge.