All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.