Re: [PATCH] libxc: check return values on mmap() and madvise() on xc_alloc_hypercall_buffer()

[Olaf Hering <olaf@aepfle.de>]

Please backport this patch to 4.4. 
Other branches may need the mmap() check as well. The callers expect
either NULL or a valid pointer.

It is upstream commit e86539a388314cd3dca88f5e69d7873343197cd8

Thanks,

Olaf

On Wed, May 21, Ian Campbell wrote:

> On Tue, 2014-05-20 at 05:37 -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > 
> > On a Thinkpad T4440p with OpenSUSE tumbleweed with v3.15-rc4
> > and today's latest xen tip from the git tree strace -f reveals
> > we end up on a never ending wait shortly after
> > 
> > write(20, "backend/console/5\0", 18 <unfinished ...>
> > 
> > This is right before we just wait on the qemu process which we
> > had mmap'd for. Without this you'll end up getting stuck on a
> > loop if mmap() worked but madvise() did not. While at it I noticed
> > even the mmap() error fail was not being checked, fix that too.
> > 
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com> and applied.
> 
> OOI why was madvise failing? (should be quite unusual I think?)
> 
> > ---
> >  tools/libxc/xc_linux_osdep.c | 20 +++++++++++++++++++-
> >  1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/libxc/xc_linux_osdep.c b/tools/libxc/xc_linux_osdep.c
> > index 73860a2..86bff3e 100644
> > --- a/tools/libxc/xc_linux_osdep.c
> > +++ b/tools/libxc/xc_linux_osdep.c
> > @@ -92,14 +92,32 @@ static void *linux_privcmd_alloc_hypercall_buffer(xc_interface *xch, xc_osdep_ha
> >  {
> >      size_t size = npages * XC_PAGE_SIZE;
> >      void *p;
> > +    int rc, saved_errno;
> >  
> >      /* Address returned by mmap is page aligned. */
> >      p = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_LOCKED, -1, 0);
> > +    if ( p == MAP_FAILED )
> > +    {
> > +        PERROR("xc_alloc_hypercall_buffer: mmap failed");
> > +        return NULL;
> > +    }
> >  
> >      /* Do not copy the VMA to child process on fork. Avoid the page being COW
> >          on hypercall. */
> > -    madvise(p, npages * XC_PAGE_SIZE, MADV_DONTFORK);
> > +    rc = madvise(p, npages * XC_PAGE_SIZE, MADV_DONTFORK);
> > +    if ( rc < 0 )
> > +    {
> > +        PERROR("xc_alloc_hypercall_buffer: madvise failed");
> > +        goto out;
> > +    }
> > +
> >      return p;
> > +
> > +out:
> > +    saved_errno = errno;
> > +    (void)munmap(p, size);
> > +    errno = saved_errno;
> > +    return NULL;
> >  }
> >  
> >  static void linux_privcmd_free_hypercall_buffer(xc_interface *xch, xc_osdep_handle h, void *ptr, int npages)