Re: [PATCH] linux-user: mprotect() should returns 0 when len is 0.

[一色聡一郎]

Thank you for your response.

Yes, we can also modify guest_range_valid_untagged() like this:

 static inline bool guest_range_valid_untagged(abi_ulong start, abi_ulong len)
 {
-    return len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;

+   return !len || len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;
 }

But actually, guest_range_valid_untagged() is called from several sites other than target_mprotect().

(1) target_mmap() in bsd-user

(2) target_madvise() in linux-user

(3) target_mmap() in linux-user

(4) target_munmap() in linux-user

(5) access_ok_untagged() in linux-user/qemu.h

(6) target_mremap() in linux-user

(7) do_shmat() in linux-user/syscall.c

(1)-(5) have explicit guards for the condition of len = 0 in front of calling  guest_range_valid_untagged().

(1) https://gitlab.com/qemu-project/qemu/-/blob/master/bsd-user/mmap.c#L477

(2) https://gitlab.com/qemu-project/qemu/-/blob/master/linux-user/mmap.c#L900

(3) https://gitlab.com/qemu-project/qemu/-/blob/master/linux-user/mmap.c#L456

(4) https://gitlab.com/qemu-project/qemu/-/blob/master/linux-user/mmap.c#L724

(5) https://gitlab.com/qemu-project/qemu/-/blob/master/linux-user/qemu.h#L176

But I'm not sure whether this change is correct for (6) and (7).

2022年10月7日(金) 3:31 Richard Henderson <richard.henderson@linaro.org>:

On 10/6/22 11:13, Peter Maydell wrote:
> On Thu, 6 Oct 2022 at 19:05, Soichiro Isshiki
> <sisshiki@isshiki-clinic.com> wrote:
>>
>> From: sisshiki1969 <sisshiki@mac.com>
>>
>> For now, qemu-x86_64 returns ENOMEM when mprotect() was called with an argument
>> len is 0 from a guest process.
>> This behavior is incompatible with the current Linux implementation,
>> which mprotect() with len = 0 does nothing and returns 0,
>> although it does not appear to be explicitly described in man.
>>
>> This is due to the following function which always returns false if len = 0.
>>
>> ```C
>> static inline bool guest_range_valid_untagged(abi_ulong start, abi_ulong len)
>> {
>>      return len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;
>> }
>>
...
> Cc'ing Richard -- is this the right fix, or would it be better instead
> to make guest_range_valid_untagged() correctly handle a zero-length
> range ?

I think fixing the range check might be best.


r~