Fix bugs in dmop code

PATCH In review Wei Liu 3 participants 8 messages Sep 28, 2017

lore

Share & tools

lore

Wei Liu

author

2017-09-28 10:37 UTC #0 lore raw

v2 cover Fix bugs in dmop code

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         | 6 ++----
 2 files changed, 3 insertions(+), 5 deletions(-)

Signature · 7 lines

2.11.0


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

Wei Liu

author

2017-09-28 10:37 UTC #1 lore raw

PATCH v2 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 | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

xen/arch/x86/hvm/dm.c +2 -4

index 87ef4b6ca9..9cf53b551c 100644
@@ -178,11 +178,9 @@ static int modified_memory(struct domain *d,
         struct xen_dm_op_modified_memory_extent extent;
         unsigned int batch_nr;
         xen_pfn_t pfn, end_pfn;
-        int rc;
 
-        rc = COPY_FROM_GUEST_BUF_OFFSET(extent,
-            bufs, EXTENTS_BUFFER, (*rem_extents - 1) * sizeof(extent));
-        if ( rc )
+        if ( !COPY_FROM_GUEST_BUF_OFFSET(extent, bufs, EXTENTS_BUFFER,
+                                         (*rem_extents - 1) * sizeof(extent)) )
             return -EFAULT;
 
         if ( extent.pad )

Signature · 7 lines

2.11.0


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

Paul Durrant

Reviewed-by on v2 1/2

2017-09-28 10:39 UTC #2 lore raw

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

Quoted 9 lines

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

Quoted 3 lines

The copy macro returns false when the copy fails.
Signed-off-by Wei Liu <[email protected]>

Better diff stats too now :-)

Reviewed-by Paul Durrant <[email protected]>

Quoted 29 lines

---
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 | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

xen/arch/x86/hvm/dm.c +2 -5

index 87ef4b6ca9..9cf53b551c 100644
@@ -178,11 +178,9 @@ static int modified_memory(struct domain *d,
         struct xen_dm_op_modified_memory_extent extent;
         unsigned int batch_nr;
         xen_pfn_t pfn, end_pfn;
-        int rc;
 
-        rc = COPY_FROM_GUEST_BUF_OFFSET(extent,
-            bufs, EXTENTS_BUFFER, (*rem_extents - 1) * sizeof(extent));
-        if ( rc )
+        if ( !COPY_FROM_GUEST_BUF_OFFSET(extent, bufs, EXTENTS_BUFFER,
+                                         (*rem_extents - 1) * sizeof(extent)) )
             return -EFAULT;
 
         if ( extent.pad )
--

Quoted 1 line

2.11.0

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

Wei Liu

author

2017-09-28 10:36 UTC #3 lore raw

PATCH v2 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, use C99 initialiser to clear extent.

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, 1 insertion(+), 1 deletion(-)

tools/libs/devicemodel/core.c +1 -1

index fcb260d29b..986f147658 100644
@@ -483,7 +483,7 @@ int xendevicemodel_modified_memory(
     xendevicemodel_handle *dmod, domid_t domid, uint64_t first_pfn,
     uint32_t nr)
 {
-    struct xen_dm_op_modified_memory_extent extent;
+    struct xen_dm_op_modified_memory_extent 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

Paul Durrant

2017-09-28 10:40 UTC #4 lore raw

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

Quoted 29 lines

-----Original Message-----
From: Wei Liu [mailto:[email protected]]
Sent: 28 September 2017 11:36
To: Xen-devel <[email protected]>
Cc: Anthony Perard <[email protected]>; Wei Liu <[email protected]>; Ian Jackson <[email protected]>; Paul Durrant <[email protected]>
Subject: [PATCH v2 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, use C99 initialiser to clear extent.
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, 1 insertion(+), 1 deletion(-)
tools/libs/devicemodel/core.c +0 -0
index fcb260d29b..986f147658 100644

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

Quoted 5 lines

tools/libs/devicemodel/core.c +1 -1

@@ -483,7 +483,7 @@ int xendevicemodel_modified_memory(
     xendevicemodel_handle *dmod, domid_t domid, uint64_t first_pfn,
     uint32_t nr)
 {
-    struct xen_dm_op_modified_memory_extent extent;
+    struct xen_dm_op_modified_memory_extent extent = { };

Not quite. I meant:

struct xen_dm_op_modified_memory_extent extent = {
.first_pfn = first_pfn,
.nr = nr,
};

then you can drop the lines below.

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

Wei Liu

author

2017-09-28 10:46 UTC #5 lore raw

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

Quoted 37 lines · Paul Durrant

Quoted 29 lines
-----Original Message-----
From: Wei Liu [mailto:[email protected]]
Sent: 28 September 2017 11:36
To: Xen-devel <[email protected]>
Cc: Anthony Perard <[email protected]>; Wei Liu <[email protected]>; Ian Jackson <[email protected]>; Paul Durrant <[email protected]>
Subject: [PATCH v2 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, use C99 initialiser to clear extent.
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, 1 insertion(+), 1 deletion(-)
tools/libs/devicemodel/core.c +0 -0
index fcb260d29b..986f147658 100644
tools/libs/devicemodel/core.c int xendevicemodel_modified_memory( · line 486
Quoted 5 lines
tools/libs/devicemodel/core.c +1 -1
@@ -483,7 +483,7 @@ int xendevicemodel_modified_memory(
     xendevicemodel_handle *dmod, domid_t domid, uint64_t first_pfn,
     uint32_t nr)
 {
-    struct xen_dm_op_modified_memory_extent extent;
+    struct xen_dm_op_modified_memory_extent extent = { };
Not quite. I meant:

Quoted 6 lines

struct xen_dm_op_modified_memory_extent extent = {
.first_pfn = first_pfn,
.nr = nr,
};
then you can drop the lines below.

Here you go.

---8<---
From 75c955c2d2c7077fe59b5e3b14e3deff613025e8 Mon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Thu, 28 Sep 2017 11:00:00 +0100
Subject: [PATCH] libxendevicemodel: initialise extent.pad to zero

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

Instead of setting the pad separately, use C99 initialiser.

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 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

tools/libs/devicemodel/core.c +4 -4

index fcb260d29b..0f2c1a791f 100644
@@ -483,10 +483,10 @@ int xendevicemodel_modified_memory(
     xendevicemodel_handle *dmod, domid_t domid, uint64_t first_pfn,
     uint32_t nr)
 {
-    struct xen_dm_op_modified_memory_extent extent;
-
-    extent.first_pfn = first_pfn;
-    extent.nr = nr;
+    struct xen_dm_op_modified_memory_extent extent = {
+        .first_pfn = first_pfn,
+        .nr = nr,
+    };
 
     return xendevicemodel_modified_memory_bulk(dmod, domid, &extent, 1);
 }

Signature · 7 lines

2.11.0


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

Paul Durrant

Reviewed-by

2017-09-28 10:48 UTC #6 lore raw

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

Quoted 85 lines

-----Original Message-----
From: Wei Liu [mailto:[email protected]]
Sent: 28 September 2017 11:46
To: Paul Durrant <[email protected]>
Cc: Wei Liu <[email protected]>; Xen-devel <xen- [email protected]>; Anthony Perard <[email protected]>; Ian Jackson <[email protected]>
Subject: Re: [PATCH v2 2/2] libxendevicemodel: initialise extent to zero
Quoted 6 lines · Paul Durrant
Quoted 6 lines
-----Original Message-----
From: Wei Liu [mailto:[email protected]]
Sent: 28 September 2017 11:36
To: Xen-devel <[email protected]>
Cc: Anthony Perard <[email protected]>; Wei Liu <[email protected]>; Ian Jackson <[email protected]>; Paul
Durrant
Quoted 13 lines
Quoted 13 lines
<[email protected]>
Subject: [PATCH v2 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, use C99 initialiser to clear extent.
Signed-off-by Wei Liu <[email protected]>
---
Cc: Ian Jackson <[email protected]>
Cc: Paul Durrant <[email protected]>
Quoted 6 lines
Review
Quoted 6 lines
Backport to 4.9
---
 tools/libs/devicemodel/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0
b/tools/libs/devicemodel/core.c
Quoted 12 lines
tools/libs/devicemodel/core.c int xendevicemodel_modified_memory( · line 486
Quoted 9 lines
index fcb260d29b..986f147658 100644
tools/libs/devicemodel/core.c +1 -1
@@ -483,7 +483,7 @@ int xendevicemodel_modified_memory(
     xendevicemodel_handle *dmod, domid_t domid, uint64_t first_pfn,
     uint32_t nr)
 {
-    struct xen_dm_op_modified_memory_extent extent;
+    struct xen_dm_op_modified_memory_extent extent = { };
Not quite. I meant:
Quoted 6 lines
struct xen_dm_op_modified_memory_extent extent = {
.first_pfn = first_pfn,
.nr = nr,
};
then you can drop the lines below.
Here you go.
---8<---
From 75c955c2d2c7077fe59b5e3b14e3deff613025e8 Mon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Thu, 28 Sep 2017 11:00:00 +0100
Subject: [PATCH] libxendevicemodel: initialise extent.pad to zero
The pad field needs to be zero as required by the hypervisor.
Instead of setting the pad separately, use C99 initialiser.
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 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
tools/libs/devicemodel/core.c +0 -0
index fcb260d29b..0f2c1a791f 100644
@@ -483,10 +483,10 @@ int xendevicemodel_modified_memory(
     xendevicemodel_handle *dmod, domid_t domid, uint64_t first_pfn,

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

Quoted 10 lines

tools/libs/devicemodel/core.c +4 -4

     uint32_t nr)
 {
-    struct xen_dm_op_modified_memory_extent extent;
-
-    extent.first_pfn = first_pfn;
-    extent.nr = nr;
+    struct xen_dm_op_modified_memory_extent extent = {
+        .first_pfn = first_pfn,
+        .nr = nr,
+    };

Yep, LGTM.

Reviewed-by Paul Durrant <[email protected]>

Quoted 6 lines

tools/libs/devicemodel/core.c +0 -1

     return xendevicemodel_modified_memory_bulk(dmod, domid, &extent,
 1);
 }
--

2.11.0

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

Andrew Cooper

2017-09-28 10:40 UTC #7 lore raw

In reply to Wei Liu

Quoted 3 lines · Wei Liu

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

Both Reviewed-by: Andrew Cooper <[email protected]>

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