| Message ID | 1478622135-22192-1-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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>
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.
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);
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(-)