From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43871)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1ZgsXG-0008MB-7e
	for qemu-devel@nongnu.org; Tue, 29 Sep 2015 06:53:27 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1ZgsXB-0005VX-52
	for qemu-devel@nongnu.org; Tue, 29 Sep 2015 06:53:26 -0400
Received: from mx1.redhat.com ([209.132.183.28]:48510)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1ZgsXB-0005VP-0k
	for qemu-devel@nongnu.org; Tue, 29 Sep 2015 06:53:21 -0400
References: <1443523755-5873-1-git-send-email-mst@redhat.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <560A6D9D.3030809@redhat.com>
Date: Tue, 29 Sep 2015 12:53:17 +0200
MIME-Version: 1.0
In-Reply-To: <1443523755-5873-1-git-send-email-mst@redhat.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] util/mmap-alloc: add comments, assertions
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>



On 29/09/2015 12:51, Michael S. Tsirkin wrote:
> Document RAM guard page logic within mmap-alloc.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Paolo, can you pls confirm this is what you had in mind?
> 
>  util/mmap-alloc.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index 05c8b4b..d978399 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -12,9 +12,14 @@
>  #include <qemu/mmap-alloc.h>
>  #include <sys/types.h>
>  #include <sys/mman.h>
> +#include <assert.h>
>  
>  void *qemu_ram_mmap(int fd, size_t size, size_t align)
>  {
> +    /*
> +     * Note: this always allocates at least one extra page of virtual address
> +     * space, even if size is already aligned.
> +     */
>      size_t total = size + align;
>      void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>      size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
> @@ -24,6 +29,11 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align)
>          return NULL;
>      }
>  
> +    /* Make sure align is a power of 2 */
> +    assert(!(align & (align - 1)));
> +    /* Always align to host page size */
> +    assert(align >= getpagesize());
> +
>      ptr1 = mmap(ptr + offset, size, PROT_READ | PROT_WRITE,
>                  MAP_FIXED | MAP_ANONYMOUS | MAP_PRIVATE, fd, 0);
>      if (ptr1 == MAP_FAILED) {
> @@ -37,6 +47,11 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align)
>      if (offset > 0) {
>          munmap(ptr - offset, offset);
>      }
> +
> +    /*
> +     * Leave a single PROT_NONE page allocated after the RAM block, to serve as
> +     * a guard page guarding against potential buffer overflows.
> +     */
>      if (total > size + getpagesize()) {
>          munmap(ptr + size + getpagesize(), total - size - getpagesize());
>      }
> @@ -47,6 +62,7 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align)
>  void qemu_ram_munmap(void *ptr, size_t size)
>  {
>      if (ptr) {
> +        /* Unmap both the RAM block and the guard page */
>          munmap(ptr, size + getpagesize());
>      }
>  }
> 

Yes, thanks!

Paolo