Re: [PULL 08/16] linux-user: don't use MAP_FIXED in pgd_find_hole_fallback

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 27 Jul 2020 at 13:24, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Plain MAP_FIXED has the undesirable behaviour of splatting exiting
>> maps so we don't actually achieve what we want when looking for gaps.
>> We should be using MAP_FIXED_NOREPLACE. As this isn't always available
>> we need to potentially check the returned address to see if the kernel
>> gave us what we asked for.
>>
>> Fixes: ad592e37dfc ("linux-user: provide fallback pgd_find_hole for bare 
>> chroots")
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Message-Id: <20200724064509.331-9-alex.bennee@linaro.org>
>
> Hi; Coverity thinks this conditional expression is suspicious
> (CID 1431059):
>
>>              if (mmap_start != MAP_FAILED) {
>>                  munmap((void *) align_start, guest_size);
>> -                return (uintptr_t) mmap_start + offset;
>> +                if (MAP_FIXED_NOREPLACE || mmap_start == (void *) 
>> align_start) {
>
> because it's performing a logical OR operation where the left
> operand is an integer constant that's neither 0 nor 1
> (it's 1048576). What was this intended to be?

It's 0 if the header doesn't provide it. If it's !0 we don't need to
check the address because it should have been in the correct place.

>
>> +                    return (uintptr_t) mmap_start + offset;
>> +                }
>>              }
>
> thanks
> -- PMM


-- 
Alex Bennée