| Message ID | 1456901511-22065-1-git-send-email-rcojocaru@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 02, 2016 at 08:51:51AM +0200, Razvan Cojocaru wrote: > Currently it's possible for xc_translate_foreign_address() to fail > and errno still be set to success. This patch fixes the issue. > Based on the first half of Don Slutz' patch: > http://lists.xen.org/archives/html/xen-devel/2014-03/msg03720.html > > Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> > --- > tools/libxc/xc_pagetab.c | 36 +++++++++++++++++++++++++++++++++--- > 1 file changed, 33 insertions(+), 3 deletions(-) > > diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c > index ec97890..f7dc30b 100644 > --- a/tools/libxc/xc_pagetab.c > +++ b/tools/libxc/xc_pagetab.c > @@ -34,9 +34,14 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, > int size, level, pt_levels = 2; > void *map; > > - if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 > + if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 > || dominfo.domid != dom) > + { > + if (errno == 0) > + errno = EINVAL; > + > return 0; > + } > Shouldn't we look into fixing the libxc functions that fail to set errno? With an approach like this you end up getting whatever errno set prior to calling xc_ function, which isn't very helpful in any case. Wei.
On 03/03/2016 02:47 PM, Wei Liu wrote: > On Wed, Mar 02, 2016 at 08:51:51AM +0200, Razvan Cojocaru wrote: >> Currently it's possible for xc_translate_foreign_address() to fail >> and errno still be set to success. This patch fixes the issue. >> Based on the first half of Don Slutz' patch: >> http://lists.xen.org/archives/html/xen-devel/2014-03/msg03720.html >> >> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> >> --- >> tools/libxc/xc_pagetab.c | 36 +++++++++++++++++++++++++++++++++--- >> 1 file changed, 33 insertions(+), 3 deletions(-) >> >> diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c >> index ec97890..f7dc30b 100644 >> --- a/tools/libxc/xc_pagetab.c >> +++ b/tools/libxc/xc_pagetab.c >> @@ -34,9 +34,14 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, >> int size, level, pt_levels = 2; >> void *map; >> >> - if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 >> + if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 >> || dominfo.domid != dom) >> + { >> + if (errno == 0) >> + errno = EINVAL; >> + >> return 0; >> + } >> > > Shouldn't we look into fixing the libxc functions that fail to set errno? > > With an approach like this you end up getting whatever errno set prior > to calling xc_ function, which isn't very helpful in any case. Yes, you're right. I took that part of Don's patch a while ago and ran with it, but a better approach would be indeed to make sure that all those functions do set errno properly - so I'll check to see if they do. The only exception is the "if (!(pte & 1))" case, where errno should just be set directly I think. I'll see if I can spin this into a V2. Thanks, Razvan
On Thu, Mar 03, 2016 at 02:53:25PM +0200, Razvan Cojocaru wrote: > On 03/03/2016 02:47 PM, Wei Liu wrote: > > On Wed, Mar 02, 2016 at 08:51:51AM +0200, Razvan Cojocaru wrote: > >> Currently it's possible for xc_translate_foreign_address() to fail > >> and errno still be set to success. This patch fixes the issue. > >> Based on the first half of Don Slutz' patch: > >> http://lists.xen.org/archives/html/xen-devel/2014-03/msg03720.html > >> > >> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> > >> --- > >> tools/libxc/xc_pagetab.c | 36 +++++++++++++++++++++++++++++++++--- > >> 1 file changed, 33 insertions(+), 3 deletions(-) > >> > >> diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c > >> index ec97890..f7dc30b 100644 > >> --- a/tools/libxc/xc_pagetab.c > >> +++ b/tools/libxc/xc_pagetab.c > >> @@ -34,9 +34,14 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, > >> int size, level, pt_levels = 2; > >> void *map; > >> > >> - if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 > >> + if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 > >> || dominfo.domid != dom) > >> + { > >> + if (errno == 0) > >> + errno = EINVAL; > >> + > >> return 0; > >> + } > >> > > > > Shouldn't we look into fixing the libxc functions that fail to set errno? > > > > With an approach like this you end up getting whatever errno set prior > > to calling xc_ function, which isn't very helpful in any case. > > Yes, you're right. I took that part of Don's patch a while ago and ran > with it, but a better approach would be indeed to make sure that all > those functions do set errno properly - so I'll check to see if they do. > > The only exception is the "if (!(pte & 1))" case, where errno should > just be set directly I think. > > I'll see if I can spin this into a V2. > Thanks! > > Thanks, > Razvan
diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c index ec97890..f7dc30b 100644 --- a/tools/libxc/xc_pagetab.c +++ b/tools/libxc/xc_pagetab.c @@ -34,9 +34,14 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, int size, level, pt_levels = 2; void *map; - if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 + if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 || dominfo.domid != dom) + { + if (errno == 0) + errno = EINVAL; + return 0; + } /* What kind of paging are we dealing with? */ if (dominfo.hvm) { @@ -44,7 +49,12 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, if (xc_domain_hvm_getcontext_partial(xch, dom, HVM_SAVE_CODE(CPU), vcpu, &ctx, sizeof ctx) != 0) + { + if (errno == 0) + errno = EINVAL; + return 0; + } if (!(ctx.cr0 & CR0_PG)) return virt >> PAGE_SHIFT; pt_levels = (ctx.msr_efer&EFER_LMA) ? 4 : (ctx.cr4&CR4_PAE) ? 3 : 2; @@ -53,9 +63,19 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, unsigned int gwidth; vcpu_guest_context_any_t ctx; if (xc_vcpu_getcontext(xch, dom, vcpu, &ctx) != 0) + { + if (errno == 0) + errno = EINVAL; + return 0; + } if (xc_domain_get_guest_width(xch, dom, &gwidth) != 0) + { + if (errno == 0) + errno = EINVAL; + return 0; + } if (gwidth == 8) { pt_levels = 4; paddr = (uint64_t)xen_cr3_to_pfn_x86_64(ctx.x64.ctrlreg[3]) @@ -84,12 +104,22 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, paddr += ((virt & mask) >> (xc_ffs64(mask) - 1)) * size; map = xc_map_foreign_range(xch, dom, PAGE_SIZE, PROT_READ, paddr >>PAGE_SHIFT); - if (!map) + if (!map) + { + if (errno == 0) + errno = EINVAL; + return 0; + } memcpy(&pte, map + (paddr & (PAGE_SIZE - 1)), size); munmap(map, PAGE_SIZE); - if (!(pte & 1)) + if (!(pte & 1)) + { + if (errno == 0) + errno = EADDRNOTAVAIL; + return 0; + } paddr = pte & 0x000ffffffffff000ull; if (level == 2 && (pte & PTE_PSE)) { mask = ((mask ^ ~-mask) >> 1); /* All bits below first set bit */
Currently it's possible for xc_translate_foreign_address() to fail and errno still be set to success. This patch fixes the issue. Based on the first half of Don Slutz' patch: http://lists.xen.org/archives/html/xen-devel/2014-03/msg03720.html Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> --- tools/libxc/xc_pagetab.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)