xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity
@ 2016-08-22 12:47 Wei Liu
  2016-08-22 12:47 ` [PATCH v2 1/2] hvmloader: correctly copy signature to info structures Wei Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Wei Liu @ 2016-08-22 12:47 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Wei Liu (2):
  hvmloader: correctly copy signature to info structures
  hvmloader: use bound checking in get_module_entry

 tools/firmware/hvmloader/hvmloader.c | 4 ++--
 tools/firmware/hvmloader/ovmf.c      | 8 ++++----
 tools/firmware/hvmloader/seabios.c   | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] hvmloader: correctly copy signature to info structures
  2016-08-22 12:47 [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity Wei Liu
@ 2016-08-22 12:47 ` Wei Liu
  2016-08-22 12:53   ` Jan Beulich
  2016-08-22 12:47 ` [PATCH v2 2/2] hvmloader: use bound checking in get_module_entry Wei Liu
  2016-08-22 13:21 ` [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity Wei Liu
  2 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2016-08-22 12:47 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

The original code used sizeof(info->signature) as the size parameter for
memcpy, which was wrong.

Fix that by using structure assignment.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/hvmloader/ovmf.c    | 8 ++++----
 tools/firmware/hvmloader/seabios.c | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index b4bcc93..0ac3416 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -68,10 +68,10 @@ static void ovmf_setup_bios_info(void)
 {
     struct ovmf_info *info = (void *)OVMF_INFO_PHYSICAL_ADDRESS;
 
-    memset(info, 0, sizeof(*info));
-
-    memcpy(info->signature, "XenHVMOVMF", sizeof(info->signature));
-    info->length = sizeof(*info);
+    *info = (struct ovmf_info) {
+        .signature = "XenHVMOVMF",
+        .length = sizeof(*info)
+    };
 }
 
 static void ovmf_finish_bios_info(void)
diff --git a/tools/firmware/hvmloader/seabios.c b/tools/firmware/hvmloader/seabios.c
index 5c9a351..44ff0d7 100644
--- a/tools/firmware/hvmloader/seabios.c
+++ b/tools/firmware/hvmloader/seabios.c
@@ -56,10 +56,10 @@ static void seabios_setup_bios_info(void)
 {
     struct seabios_info *info = (void *)BIOS_INFO_PHYSICAL_ADDRESS;
 
-    memset(info, 0, sizeof(*info));
-
-    memcpy(info->signature, "XenHVMSeaBIOS", sizeof(info->signature));
-    info->length = sizeof(*info);
+    *info = (struct seabios_info) {
+        .signature = "XenHVMSeaBIOS",
+        .length = sizeof(*info)
+    };
 
     info->tables = (uint32_t)scratch_alloc(MAX_TABLES*sizeof(uint32_t), 0);
 }
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] hvmloader: use bound checking in get_module_entry
  2016-08-22 12:47 [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity Wei Liu
  2016-08-22 12:47 ` [PATCH v2 1/2] hvmloader: correctly copy signature to info structures Wei Liu
@ 2016-08-22 12:47 ` Wei Liu
  2016-08-22 12:54   ` Jan Beulich
  2016-08-22 13:21 ` [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity Wei Liu
  2 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2016-08-22 12:47 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Coverity complains:

overflow_before_widen: Potentially overflowing expression
info->nr_modules * 32U with type unsigned int (32 bits, unsigned) is
evaluated using 32-bit arithmetic, and then used in a context that
expects an expression of type uint64_t (64 bits, unsigned).

The overflow is unlikely to happen in reality because we only expect a
few modules.

Fix that by converting the check to use bound checking to placate
Coverity.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/hvmloader/hvmloader.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index 7b32d86..bbd4e34 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -272,8 +272,8 @@ const struct hvm_modlist_entry *get_module_entry(
 
     if ( !modlist ||
          info->modlist_paddr > UINTPTR_MAX ||
-         (info->modlist_paddr + info->nr_modules * sizeof(*modlist) - 1)
-            > UINTPTR_MAX )
+         (UINTPTR_MAX - (uintptr_t)info->modlist_paddr) / sizeof(*modlist)
+         < info->nr_modules )
         return NULL;
 
     for ( i = 0; i < info->nr_modules; i++ )
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] hvmloader: correctly copy signature to info structures
  2016-08-22 12:47 ` [PATCH v2 1/2] hvmloader: correctly copy signature to info structures Wei Liu
@ 2016-08-22 12:53   ` Jan Beulich
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2016-08-22 12:53 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Xen-devel

>>> On 22.08.16 at 14:47, <wei.liu2@citrix.com> wrote:
> The original code used sizeof(info->signature) as the size parameter for
> memcpy, which was wrong.
> 
> Fix that by using structure assignment.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] hvmloader: use bound checking in get_module_entry
  2016-08-22 12:47 ` [PATCH v2 2/2] hvmloader: use bound checking in get_module_entry Wei Liu
@ 2016-08-22 12:54   ` Jan Beulich
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2016-08-22 12:54 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Xen-devel

>>> On 22.08.16 at 14:47, <wei.liu2@citrix.com> wrote:
> Coverity complains:
> 
> overflow_before_widen: Potentially overflowing expression
> info->nr_modules * 32U with type unsigned int (32 bits, unsigned) is
> evaluated using 32-bit arithmetic, and then used in a context that
> expects an expression of type uint64_t (64 bits, unsigned).
> 
> The overflow is unlikely to happen in reality because we only expect a
> few modules.
> 
> Fix that by converting the check to use bound checking to placate
> Coverity.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity
  2016-08-22 12:47 [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity Wei Liu
  2016-08-22 12:47 ` [PATCH v2 1/2] hvmloader: correctly copy signature to info structures Wei Liu
  2016-08-22 12:47 ` [PATCH v2 2/2] hvmloader: use bound checking in get_module_entry Wei Liu
@ 2016-08-22 13:21 ` Wei Liu
  2 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2016-08-22 13:21 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

On Mon, Aug 22, 2016 at 01:47:51PM +0100, Wei Liu wrote:
> Wei Liu (2):
>   hvmloader: correctly copy signature to info structures
>   hvmloader: use bound checking in get_module_entry
> 

Pushed.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-08-22 13:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-22 12:47 [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity Wei Liu
2016-08-22 12:47 ` [PATCH v2 1/2] hvmloader: correctly copy signature to info structures Wei Liu
2016-08-22 12:53   ` Jan Beulich
2016-08-22 12:47 ` [PATCH v2 2/2] hvmloader: use bound checking in get_module_entry Wei Liu
2016-08-22 12:54   ` Jan Beulich
2016-08-22 13:21 ` [PATCH v2 0/2] hvmloader: fix two issues spotted by Coverity 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).