Skip to contentSkip to conversation

Fix bugs in dmop code

PATCH Superseded Wei Liu 2 participants 5 messages Sep 28, 2017

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Wei Liu

author
#0 lore raw

cover Fix bugs in dmop code

I discovered two issues while trying to figure out what went wrong with Windows.

Wei Liu (2):
x86/hvm/dmop: fix EFAULT condition
libxendevicemodel: initialise extent to zero

 tools/libs/devicemodel/core.c | 2 ++
 xen/arch/x86/hvm/dm.c         | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)
Signature · 7 lines
2.11.0


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
author
#1 lore raw

PATCH 1/2 x86/hvm/dmop: fix EFAULT condition

The copy macro returns false when the copy fails.

Signed-off-by Wei Liu <[email protected]>

---
Cc: Jan Beulich <[email protected]>
Cc: Andrew Cooper <[email protected]>
Cc: Paul Durrant <[email protected]>

Backport to 4.9

---
 xen/arch/x86/hvm/dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
xen/arch/x86/hvm/dm.c +1 -1
index 87ef4b6ca9..b1cf0d54a2 100644
@@ -182,7 +182,7 @@ static int modified_memory(struct domain *d,
 
         rc = COPY_FROM_GUEST_BUF_OFFSET(extent,
             bufs, EXTENTS_BUFFER, (*rem_extents - 1) * sizeof(extent));
-        if ( rc )
+        if ( !rc )
             return -EFAULT;
 
         if ( extent.pad )
Signature · 7 lines
2.11.0


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
#2 lore raw

Re: [PATCH 1/2] x86/hvm/dmop: fix EFAULT condition

Quoted 27 lines

-----Original Message-----
From: Wei Liu [mailto:[email protected]]
Sent: 28 September 2017 11:24
To: Xen-devel <[email protected]>
Cc: Wei Liu <[email protected]>; Jan Beulich <[email protected]>; Andrew Cooper <[email protected]>; Paul Durrant <[email protected]>
Subject: [PATCH 1/2] x86/hvm/dmop: fix EFAULT condition

The copy macro returns false when the copy fails.

Signed-off-by Wei Liu <[email protected]>

---
Cc: Jan Beulich <[email protected]>
Cc: Andrew Cooper <[email protected]>
Cc: Paul Durrant <[email protected]>

Backport to 4.9

---
 xen/arch/x86/hvm/dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
xen/arch/x86/hvm/dm.c +0 -0
index 87ef4b6ca9..b1cf0d54a2 100644

xen/arch/x86/hvm/dm.c static int modified_memory(struct domain *d, · line 186

Quoted 6 lines
xen/arch/x86/hvm/dm.c +1 -1
@@ -182,7 +182,7 @@ static int modified_memory(struct domain *d,
 
         rc = COPY_FROM_GUEST_BUF_OFFSET(extent,
             bufs, EXTENTS_BUFFER, (*rem_extents - 1) * sizeof(extent));
-        if ( rc )
+        if ( !rc )
             return -EFAULT;

For clarity and consistency I think it would be better to avoid use of rc in this case and simply put the call to COPY_FROM_GUEST_BUF_OFFSET() inside the if clause.

Paul

Quoted 4 lines
xen/arch/x86/hvm/dm.c +0 -1
         if ( extent.pad )
--

2.11.0

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

author
#3 lore raw

PATCH 2/2 libxendevicemodel: initialise extent to zero

In reply to Wei Liu

The pad field needs to be zero as required by the hypervisor.

Instead of setting the pad separately, memset extent to zero.

Signed-off-by Wei Liu <[email protected]>

---
Cc: Ian Jackson <[email protected]>
Cc: Paul Durrant <[email protected]>

Backport to 4.9

---
 tools/libs/devicemodel/core.c | 2 ++
 1 file changed, 2 insertions(+)
tools/libs/devicemodel/core.c +2 -0
index fcb260d29b..af06936262 100644
@@ -485,6 +485,8 @@ int xendevicemodel_modified_memory(
 {
     struct xen_dm_op_modified_memory_extent extent;
 
+    memset(&extent, 0, sizeof(extent));
+
     extent.first_pfn = first_pfn;
     extent.nr = nr;
Signature · 7 lines
2.11.0


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
#4 lore raw

Re: [PATCH 2/2] libxendevicemodel: initialise extent to zero

Quoted 27 lines

-----Original Message-----
From: Wei Liu [mailto:[email protected]]
Sent: 28 September 2017 11:24
To: Xen-devel <[email protected]>
Cc: Wei Liu <[email protected]>; Ian Jackson <[email protected]>; Paul Durrant <[email protected]>
Subject: [PATCH 2/2] libxendevicemodel: initialise extent to zero

The pad field needs to be zero as required by the hypervisor.

Instead of setting the pad separately, memset extent to zero.

Signed-off-by Wei Liu <[email protected]>

---
Cc: Ian Jackson <[email protected]>
Cc: Paul Durrant <[email protected]>

Backport to 4.9

---
 tools/libs/devicemodel/core.c | 2 ++
 1 file changed, 2 insertions(+)
tools/libs/devicemodel/core.c +0 -0
index fcb260d29b..af06936262 100644

tools/libs/devicemodel/core.c int xendevicemodel_modified_memory( · line 489

Quoted 5 lines
tools/libs/devicemodel/core.c +2 -0
@@ -485,6 +485,8 @@ int xendevicemodel_modified_memory(
 {
     struct xen_dm_op_modified_memory_extent extent;
 
+    memset(&extent, 0, sizeof(extent));
+

Probably better to just use a C99 initializer instead so the compiler deals with the pad.

Paul

Quoted 5 lines
tools/libs/devicemodel/core.c +0 -1
     extent.first_pfn = first_pfn;
     extent.nr = nr;
 
--

2.11.0

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel