[PATCH v2] linux-user: Fix executable page of /proc/self/maps

[PATCH v2] linux-user: Fix executable page of /proc/self/maps

[Nicolas Surbayrole]

The guest binary and libraries are not always map with the
executable bit in the host process. The guest may read a
/proc/self/maps with no executable address range. The
perm fields should be based on the guest permission inside
Qemu.

Signed-off-by: Nicolas Surbayrole <nsurbayrole@quarkslab.com>
---
 linux-user/syscall.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 389ec09764..0bbb2ff9c7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7888,9 +7888,9 @@ static int open_self_maps(void *cpu_env, int fd)
             count = dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
                             " %c%c%c%c %08" PRIx64 " %s %"PRId64,
                             h2g(min), h2g(max - 1) + 1,
-                            e->is_read ? 'r' : '-',
-                            e->is_write ? 'w' : '-',
-                            e->is_exec ? 'x' : '-',
+                            (flags & PAGE_READ) ? 'r' : '-',
+                            (flags & PAGE_WRITE_ORG) ? 'w' : '-',
+                            (flags & PAGE_EXEC) ? 'x' : '-',
                             e->is_priv ? 'p' : '-',
                             (uint64_t) e->offset, e->dev, e->inode);
             if (path) {
-- 
2.30.1

Re: [PATCH v2] linux-user: Fix executable page of /proc/self/maps

[Laurent Vivier]

Le 08/03/2021 à 10:19, Nicolas Surbayrole a écrit :
> The guest binary and libraries are not always map with the
> executable bit in the host process. The guest may read a
> /proc/self/maps with no executable address range. The
> perm fields should be based on the guest permission inside
> Qemu.
> 
> Signed-off-by: Nicolas Surbayrole <nsurbayrole@quarkslab.com>
> ---
>  linux-user/syscall.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 389ec09764..0bbb2ff9c7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7888,9 +7888,9 @@ static int open_self_maps(void *cpu_env, int fd)
>              count = dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
>                              " %c%c%c%c %08" PRIx64 " %s %"PRId64,
>                              h2g(min), h2g(max - 1) + 1,
> -                            e->is_read ? 'r' : '-',
> -                            e->is_write ? 'w' : '-',
> -                            e->is_exec ? 'x' : '-',
> +                            (flags & PAGE_READ) ? 'r' : '-',
> +                            (flags & PAGE_WRITE_ORG) ? 'w' : '-',
> +                            (flags & PAGE_EXEC) ? 'x' : '-',
>                              e->is_priv ? 'p' : '-',
>                              (uint64_t) e->offset, e->dev, e->inode);
>              if (path) {
> 

It looks good.

Alex, you wrote this code, any comment?
Should we move this directly in read_self_maps() to have the guest values in MapInfo?

Thanks,
Laurent

Re: [PATCH v2] linux-user: Fix executable page of /proc/self/maps

[Richard Henderson]

On 3/8/21 2:47 AM, Laurent Vivier wrote:
> Should we move this directly in read_self_maps() to have the guest values in MapInfo?

No, because we also need read_self_maps() to create the guest in the first place.


r~

Re: [PATCH v2] linux-user: Fix executable page of /proc/self/maps

[Alex Bennée]

Laurent Vivier <laurent@vivier.eu> writes:

> Le 08/03/2021 à 10:19, Nicolas Surbayrole a écrit :
>> The guest binary and libraries are not always map with the
>> executable bit in the host process. The guest may read a
>> /proc/self/maps with no executable address range. The
>> perm fields should be based on the guest permission inside
>> Qemu.
>> 
>> Signed-off-by: Nicolas Surbayrole <nsurbayrole@quarkslab.com>
>> ---
>>  linux-user/syscall.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 389ec09764..0bbb2ff9c7 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7888,9 +7888,9 @@ static int open_self_maps(void *cpu_env, int fd)
>>              count = dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
>>                              " %c%c%c%c %08" PRIx64 " %s %"PRId64,
>>                              h2g(min), h2g(max - 1) + 1,
>> -                            e->is_read ? 'r' : '-',
>> -                            e->is_write ? 'w' : '-',
>> -                            e->is_exec ? 'x' : '-',
>> +                            (flags & PAGE_READ) ? 'r' : '-',
>> +                            (flags & PAGE_WRITE_ORG) ? 'w' : '-',
>> +                            (flags & PAGE_EXEC) ? 'x' : '-',
>>                              e->is_priv ? 'p' : '-',
>>                              (uint64_t) e->offset, e->dev, e->inode);
>>              if (path) {
>> 
>
> It looks good.
>
> Alex, you wrote this code, any comment?

I just checked to see if we need a Fixes but it seems we've been
ignoring flags for anything other than page_check_range since the
original parsing code so I think we are good.

Acked-by: Alex Bennée <alex.bennee@linaro.org>

> Should we move this directly in read_self_maps() to have the guest
> values in MapInfo?

Nope for the reasons Richard said. read_self_maps() is a QEMU internal
function which we use elsewhere and needs to know the "real" truth ;-)

>
> Thanks,
> Laurent


-- 
Alex Bennée

Re: [PATCH v2] linux-user: Fix executable page of /proc/self/maps

[Richard Henderson]

On 3/8/21 1:19 AM, Nicolas Surbayrole wrote:
> The guest binary and libraries are not always map with the
> executable bit in the host process. The guest may read a
> /proc/self/maps with no executable address range. The
> perm fields should be based on the guest permission inside
> Qemu.
> 
> Signed-off-by: Nicolas Surbayrole<nsurbayrole@quarkslab.com>
> ---

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v2] linux-user: Fix executable page of /proc/self/maps

[Laurent Vivier]

Le 08/03/2021 à 10:19, Nicolas Surbayrole a écrit :
> The guest binary and libraries are not always map with the
> executable bit in the host process. The guest may read a
> /proc/self/maps with no executable address range. The
> perm fields should be based on the guest permission inside
> Qemu.
> 
> Signed-off-by: Nicolas Surbayrole <nsurbayrole@quarkslab.com>
> ---
>  linux-user/syscall.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 389ec09764..0bbb2ff9c7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7888,9 +7888,9 @@ static int open_self_maps(void *cpu_env, int fd)
>              count = dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
>                              " %c%c%c%c %08" PRIx64 " %s %"PRId64,
>                              h2g(min), h2g(max - 1) + 1,
> -                            e->is_read ? 'r' : '-',
> -                            e->is_write ? 'w' : '-',
> -                            e->is_exec ? 'x' : '-',
> +                            (flags & PAGE_READ) ? 'r' : '-',
> +                            (flags & PAGE_WRITE_ORG) ? 'w' : '-',
> +                            (flags & PAGE_EXEC) ? 'x' : '-',
>                              e->is_priv ? 'p' : '-',
>                              (uint64_t) e->offset, e->dev, e->inode);
>              if (path) {
> 

Applied to my linux-user-for-6.0 branch.

Thanks,
Laurent