* [PATCH] libxc: Have xc_translate_foreign_address() set errno properly
@ 2016-03-02 6:51 Razvan Cojocaru
2016-03-03 12:47 ` Wei Liu
0 siblings, 1 reply; 4+ messages in thread
From: Razvan Cojocaru @ 2016-03-02 6:51 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, ian.jackson, Razvan Cojocaru, stefano.stabellini
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;
+ }
/* 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 */
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] libxc: Have xc_translate_foreign_address() set errno properly
2016-03-02 6:51 [PATCH] libxc: Have xc_translate_foreign_address() set errno properly Razvan Cojocaru
@ 2016-03-03 12:47 ` Wei Liu
2016-03-03 12:53 ` Razvan Cojocaru
0 siblings, 1 reply; 4+ messages in thread
From: Wei Liu @ 2016-03-03 12:47 UTC (permalink / raw)
To: Razvan Cojocaru; +Cc: wei.liu2, stefano.stabellini, ian.jackson, xen-devel
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.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libxc: Have xc_translate_foreign_address() set errno properly
2016-03-03 12:47 ` Wei Liu
@ 2016-03-03 12:53 ` Razvan Cojocaru
2016-03-03 12:54 ` Wei Liu
0 siblings, 1 reply; 4+ messages in thread
From: Razvan Cojocaru @ 2016-03-03 12:53 UTC (permalink / raw)
To: Wei Liu; +Cc: stefano.stabellini, ian.jackson, xen-devel
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libxc: Have xc_translate_foreign_address() set errno properly
2016-03-03 12:53 ` Razvan Cojocaru
@ 2016-03-03 12:54 ` Wei Liu
0 siblings, 0 replies; 4+ messages in thread
From: Wei Liu @ 2016-03-03 12:54 UTC (permalink / raw)
To: Razvan Cojocaru; +Cc: ian.jackson, stefano.stabellini, Wei Liu, xen-devel
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-03 12:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-02 6:51 [PATCH] libxc: Have xc_translate_foreign_address() set errno properly Razvan Cojocaru
2016-03-03 12:47 ` Wei Liu
2016-03-03 12:53 ` Razvan Cojocaru
2016-03-03 12:54 ` Wei Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).