[PATCH for-4.8] libxc: fix unmap of ACPI guest memory region

[PATCH for-4.8] libxc: fix unmap of ACPI guest memory region

[Roger Pau Monne]

Commit fac7f7 changed the value of ptr so that it points to the right memory
area, taking the page offset into account, but failed to remove this when
doing the unmap, which caused the region to not be unmapped. Fix this by not
modifying ptr and instead adding the page offset directly in the memcpy
call.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 tools/libxc/xc_dom_core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index ad819dd..36cd3c8 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -1119,10 +1119,9 @@ static int xc_dom_load_acpi(struct xc_dom_image *dom)
             goto err;
         }
 
-        ptr = (uint8_t *)ptr +
-              (dom->acpi_modules[i].guest_addr_out & ~XC_PAGE_MASK);
-
-        memcpy(ptr, dom->acpi_modules[i].data, dom->acpi_modules[i].length);
+        memcpy((uint8_t *)ptr +
+               (dom->acpi_modules[i].guest_addr_out & ~XC_PAGE_MASK),
+               dom->acpi_modules[i].data, dom->acpi_modules[i].length);
         munmap(ptr, XC_PAGE_SIZE * num_pages);
 
         free(extents);
-- 
2.7.4 (Apple Git-66)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH for-4.8] libxc: fix unmap of ACPI guest memory region

[Boris Ostrovsky]

On 11/08/2016 11:22 AM, Roger Pau Monne wrote:
> Commit fac7f7 changed the value of ptr so that it points to the right memory
> area, taking the page offset into account, but failed to remove this when
> doing the unmap, which caused the region to not be unmapped. Fix this by not
> modifying ptr and instead adding the page offset directly in the memcpy
> call.
>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  tools/libxc/xc_dom_core.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
> index ad819dd..36cd3c8 100644
> --- a/tools/libxc/xc_dom_core.c
> +++ b/tools/libxc/xc_dom_core.c
> @@ -1119,10 +1119,9 @@ static int xc_dom_load_acpi(struct xc_dom_image *dom)
>              goto err;
>          }
>
> -        ptr = (uint8_t *)ptr +
> -              (dom->acpi_modules[i].guest_addr_out & ~XC_PAGE_MASK);
> -
> -        memcpy(ptr, dom->acpi_modules[i].data, dom->acpi_modules[i].length);
> +        memcpy((uint8_t *)ptr +
> +               (dom->acpi_modules[i].guest_addr_out & ~XC_PAGE_MASK),
> +               dom->acpi_modules[i].data, dom->acpi_modules[i].length);
>          munmap(ptr, XC_PAGE_SIZE * num_pages);
>
>          free(extents);
>


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

(Although I don't think this would cause memory not to be unmapped: per 
Linux man page "All pages containing a part of the indicated range are 
unmapped ..." and ptr is offset from its original value by a fraction of 
a page.)

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH for-4.8] libxc: fix unmap of ACPI guest memory region

[Roger Pau Monne]

On Tue, Nov 08, 2016 at 12:19:06PM -0500, Boris Ostrovsky wrote:
> 
> 
> On 11/08/2016 11:22 AM, Roger Pau Monne wrote:
> > Commit fac7f7 changed the value of ptr so that it points to the right memory
> > area, taking the page offset into account, but failed to remove this when
> > doing the unmap, which caused the region to not be unmapped. Fix this by not
> > modifying ptr and instead adding the page offset directly in the memcpy
> > call.
> > 
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > ---
> >  tools/libxc/xc_dom_core.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
> > index ad819dd..36cd3c8 100644
> > --- a/tools/libxc/xc_dom_core.c
> > +++ b/tools/libxc/xc_dom_core.c
> > @@ -1119,10 +1119,9 @@ static int xc_dom_load_acpi(struct xc_dom_image *dom)
> >              goto err;
> >          }
> > 
> > -        ptr = (uint8_t *)ptr +
> > -              (dom->acpi_modules[i].guest_addr_out & ~XC_PAGE_MASK);
> > -
> > -        memcpy(ptr, dom->acpi_modules[i].data, dom->acpi_modules[i].length);
> > +        memcpy((uint8_t *)ptr +
> > +               (dom->acpi_modules[i].guest_addr_out & ~XC_PAGE_MASK),
> > +               dom->acpi_modules[i].data, dom->acpi_modules[i].length);
> >          munmap(ptr, XC_PAGE_SIZE * num_pages);
> > 
> >          free(extents);
> > 
> 
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> (Although I don't think this would cause memory not to be unmapped: per
> Linux man page "All pages containing a part of the indicated range are
> unmapped ..." and ptr is offset from its original value by a fraction of a
> page.)

Linux man page states:

"The implementation shall require that addr be a multiple of the page size 
{PAGESIZE}."

And on FreeBSD:

"The munmap() system call will fail if: The addr argument was not page 
aligned, [...]"

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH for-4.8] libxc: fix unmap of ACPI guest memory region

[Andrew Cooper]

On 08/11/16 16:22, Roger Pau Monne wrote:
> Commit fac7f7 changed the value of ptr so that it points to the right memory
> area, taking the page offset into account, but failed to remove this when
> doing the unmap, which caused the region to not be unmapped. Fix this by not
> modifying ptr and instead adding the page offset directly in the memcpy
> call.

Coverity-ID: 1394285

(Coverity scan has now run and found this issue, so we have a public ID
to use).

> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH for-4.8] libxc: fix unmap of ACPI guest memory region

[Wei Liu]

On Tue, Nov 08, 2016 at 05:22:15PM +0100, Roger Pau Monne wrote:
> Commit fac7f7 changed the value of ptr so that it points to the right memory
> area, taking the page offset into account, but failed to remove this when
> doing the unmap, which caused the region to not be unmapped. Fix this by not
> modifying ptr and instead adding the page offset directly in the memcpy
> call.
> 
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Applied.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel