On 24.02.20 18:36, Peter Xu wrote: > On Mon, Feb 24, 2020 at 11:57:03AM +0100, David Hildenbrand wrote: >> On 24.02.20 11:50, David Hildenbrand wrote: >>> On 19.02.20 23:46, Peter Xu wrote: >>>> On Wed, Feb 12, 2020 at 02:42:46PM +0100, David Hildenbrand wrote: >>>>> Factor it out and add a comment. >>>>> >>>>> Reviewed-by: Igor Kotrasinski <i.kotrasi...@partner.samsung.com> >>>>> Acked-by: Murilo Opsfelder Araujo <muri...@linux.ibm.com> >>>>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org> >>>>> Cc: "Michael S. Tsirkin" <m...@redhat.com> >>>>> Cc: Murilo Opsfelder Araujo <muri...@linux.ibm.com> >>>>> Cc: Greg Kurz <gr...@kaod.org> >>>>> Cc: Eduardo Habkost <ehabk...@redhat.com> >>>>> Cc: "Dr. David Alan Gilbert" <dgilb...@redhat.com> >>>>> Cc: Igor Mammedov <imamm...@redhat.com> >>>>> Signed-off-by: David Hildenbrand <da...@redhat.com> >>>>> --- >>>>> util/mmap-alloc.c | 21 ++++++++++++--------- >>>>> 1 file changed, 12 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c >>>>> index 27dcccd8ec..82f02a2cec 100644 >>>>> --- a/util/mmap-alloc.c >>>>> +++ b/util/mmap-alloc.c >>>>> @@ -82,17 +82,27 @@ size_t qemu_mempath_getpagesize(const char *mem_path) >>>>> return qemu_real_host_page_size; >>>>> } >>>>> >>>>> +static inline size_t mmap_pagesize(int fd) >>>>> +{ >>>>> +#if defined(__powerpc64__) && defined(__linux__) >>>>> + /* Mappings in the same segment must share the same page size */ >>>>> + return qemu_fd_getpagesize(fd); >>>>> +#else >>>>> + return qemu_real_host_page_size; >>>>> +#endif >>>>> +} >>>> >>>> Pure question: This will return 4K even for huge pages on x86, is this >>>> what we want? >>> >>> (was asking myself the same question) I *think* it's intended. It's >>> mainly only used to allocate one additional guard page. The callers of >>> qemu_ram_mmap() make sure that the size is properly aligned (e.g., to >>> huge pages). >>> >>> Of course, a 4k guard page is sufficient - unless we can't use that >>> (special case for ppc64 here). >>> >>> Thanks! >>> >> >> We could rename the function to mmap_guard_pagesize(), thoughts? > > Yeh this looks better. > > Out of topic: Actually I'm still confused on why we'd need to do so > much to just leave an PROT_NONE page after the buffer. If the > hypervisor is buggy, it can logically writes to anywhere after all. > It's not a stack structure but it can be any guest RAM so I'm not sure > overflow detection really matters that much...
The comment in the file says "Leave a single PROT_NONE page allocated after the RAM block, to serve as a guard page guarding against potential buffer overflows." So it's really just a safety net. > > Thanks, > -- Thanks, David / dhildenb