linux-user: Fix stale tbs after mmap

If we execute linux-user code that does the following:

  * A = mmap()
  * execute code in A
  * munmap(A)
  * B = mmap(), but mmap returns the same address as A
  * execute code in B

we end up executing a stale cached tb that contains translated code
from A, while we want new code from B.

This patch adds a TB flush for mmap'ed regions, before we return them,
avoiding the whole issue.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/mmap.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

On 7 May 2012 10:30, Alexander Graf <agraf@suse.de> wrote:
> @@ -587,6 +587,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
>     page_dump(stdout);
>     printf("\n");
>  #endif
> +    tb_invalidate_phys_page_range(start, start + len, 0);
>     mmap_unlock();
>     return start;

The comment at the top of tb_invalidate_phys_page_range() says
"start and end must refer to the same physical page" -- is it
out of date or does that not apply to user-mode?

Do you need to also invalidate the range on munmap() and
mprotect-to-not-executable in order to correctly fault on
the case of:
  map something
  execute it
  unmap it
  try to execute it again

? (haven't tested that case but it seems like it might be an issue)

-- PMM

On 07.05.2012, at 12:37, Peter Maydell wrote:

> On 7 May 2012 10:30, Alexander Graf <agraf@suse.de> wrote:
>> @@ -587,6 +587,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
>>     page_dump(stdout);
>>     printf("\n");
>>  #endif
>> +    tb_invalidate_phys_page_range(start, start + len, 0);
>>     mmap_unlock();
>>     return start;
> 
> The comment at the top of tb_invalidate_phys_page_range() says
> "start and end must refer to the same physical page" -- is it
> out of date or does that not apply to user-mode?

:(

No, you're right. It only flushes the first page.

> Do you need to also invalidate the range on munmap() and
> mprotect-to-not-executable in order to correctly fault on
> the case of:
>  map something
>  execute it
>  unmap it
>  try to execute it again
> 
> ? (haven't tested that case but it seems like it might be an issue)

I'm not sure. But it's an unrelated issue either way, right? :)
Could you please try to quickly write a test case for this one while I fix the patch?


Alex

On 07.05.2012, at 12:37, Peter Maydell wrote:

> On 7 May 2012 10:30, Alexander Graf <agraf@suse.de> wrote:
>> @@ -587,6 +587,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
>>     page_dump(stdout);
>>     printf("\n");
>>  #endif
>> +    tb_invalidate_phys_page_range(start, start + len, 0);
>>     mmap_unlock();
>>     return start;
> 
> The comment at the top of tb_invalidate_phys_page_range() says
> "start and end must refer to the same physical page" -- is it
> out of date or does that not apply to user-mode?
> 
> Do you need to also invalidate the range on munmap() and
> mprotect-to-not-executable in order to correctly fault on
> the case of:
>  map something
>  execute it
>  unmap it
>  try to execute it again
> 
> ? (haven't tested that case but it seems like it might be an issue)

Yeah, the issue does exist:

#include <stdio.h>
#include <sys/mman.h>
#include <string.h>
#include <unistd.h>
#include <stdlib.h>

static int foo(void)
{
    return 5;
}

int main(int argc, char **argv)
{
    void *p;
    int x;
    int (*f)(void);

    p = mmap(NULL,0x1000,PROT_EXEC|PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,0,0);
    if (!p) {
        printf("Error: mmap returned failure\n");
        exit(1);
    }
    memcpy(p, (void*)foo, 0x10);
    f = p;
    x = f();
    printf("returned %d\n", x);
    munmap(p, 0x1000);
    x = f();
    printf("returned %d\n", x);
}

-----

baur:/> ./test
returned 5
returned 5