diff mbox

[v7,09/15] hvmloader: Check modules whereabouts in perform_tests

Message ID 20160728105013.22310-10-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anthony PERARD July 28, 2016, 10:50 a.m. UTC
As perform_tests() is going to clear memory past 4MB, we check that the
memory can be use or we skip the tests.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

---
Changes in V6:
- define and use TEST_START and PT_END.
- cast addresses to uintptr_t instead of uint32_t.
- use UINTPTR_MAX for upper limit checks, instead of UINT_MAX.
- fix typos
- include xen/arch-x86/hvm/start_info.h
- better check for the cmdlines, now check if a string would cross the
  4GB boundary.

Changes in V5:
- also account for the pages table
- fix coding style
- also check modules cmdline and main cmdline
  and modlist_paddr
- make use of check_overlap.

Changes in v4:
- move the check into the perform_test() function.
- skip tests instead of using BUG.

New in V3
---
 tools/firmware/hvmloader/tests.c | 76 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

Comments

Andrew Cooper July 28, 2016, 2:08 p.m. UTC | #1
On 28/07/16 11:50, Anthony PERARD wrote:
> As perform_tests() is going to clear memory past 4MB, we check that the
> memory can be use or we skip the tests.
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

This is a loosing battle of overlap checks, and they are far less useful
than they used to be if they are conditionally skipped.

I would just drop tests.c entirely.  This was one longterm goal of mine
with XTF, and it is a far more appropriate way to get tests done,
especially as it is almost integrated into OSSTest now.

~Andrew
Anthony PERARD Aug. 2, 2016, 6:34 p.m. UTC | #2
On Thu, Jul 28, 2016 at 03:08:29PM +0100, Andrew Cooper wrote:
> On 28/07/16 11:50, Anthony PERARD wrote:
> > As perform_tests() is going to clear memory past 4MB, we check that the
> > memory can be use or we skip the tests.
> >
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> This is a loosing battle of overlap checks, and they are far less useful
> than they used to be if they are conditionally skipped.

The tests may be skipped if OVMF is used as firmware, otherwise it does
not really happen.

> I would just drop tests.c entirely.  This was one longterm goal of mine
> with XTF, and it is a far more appropriate way to get tests done,
> especially as it is almost integrated into OSSTest now.

I guess I can drop tests.c and this patch if I have to send another
version of the patch series.
diff mbox

Patch

diff --git a/tools/firmware/hvmloader/tests.c b/tools/firmware/hvmloader/tests.c
index fea3ad3..4b5cb93 100644
--- a/tools/firmware/hvmloader/tests.c
+++ b/tools/firmware/hvmloader/tests.c
@@ -20,6 +20,8 @@ 
  */
 
 #include "util.h"
+#include "config.h"
+#include <xen/arch-x86/hvm/start_info.h>
 
 #define TEST_FAIL 0
 #define TEST_PASS 1
@@ -32,8 +34,10 @@ 
  *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
  *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
  */
+#define TEST_START (4ul << 20)
 #define PD_START (8ul << 20)
 #define PT_START (PD_START + 4096)
+#define PT_END (PT_START + 4 * PAGE_SIZE)
 
 static void setup_paging(void)
 {
@@ -189,6 +193,37 @@  static int shadow_gs_test(void)
     return (ebx == 2) ? TEST_PASS : TEST_FAIL;
 }
 
+static bool check_test_overlap(uint64_t start, uint64_t size)
+{
+    if ( start )
+        return check_overlap(start, size,
+                             TEST_START,
+                             PT_END - TEST_START);
+    return false;
+}
+
+/* Only return true if the string overlap with the TEST_START,PT_END */
+static bool check_string_overlap_with_test(uint64_t paddr)
+{
+    unsigned len = 0;
+    const char *s;
+
+    if ( !paddr || paddr > UINTPTR_MAX )
+        return false;
+
+    s = (char *)(uintptr_t)paddr;
+    while ( *s && (uintptr_t)s < UINTPTR_MAX )
+        s++;
+
+    /* Not valid string, ignore it. */
+    if ( (uintptr_t)s == UINTPTR_MAX && *s )
+        return false;
+
+    len = (uintptr_t)s - paddr;
+
+    return check_test_overlap(paddr, len);
+}
+
 void perform_tests(void)
 {
     int i, passed, skipped;
@@ -210,11 +245,50 @@  void perform_tests(void)
         return;
     }
 
+    /* Check that tests does not use memory where modules are stored */
+    if ( check_test_overlap((uintptr_t)hvm_start_info,
+                            sizeof(*hvm_start_info)) )
+    {
+        printf("Skipping tests due to memory used by hvm_start_info\n");
+        return;
+    }
+    if ( check_test_overlap(hvm_start_info->modlist_paddr,
+                            hvm_start_info->nr_modules *
+                              sizeof(struct hvm_modlist_entry)) )
+    {
+        printf("Skipping tests due to memory used by"
+               " hvm_start_info->modlist\n");
+        return;
+    }
+    for ( i = 0; i < hvm_start_info->nr_modules; i++ )
+    {
+        const struct hvm_modlist_entry *modlist =
+            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
+
+        if ( check_test_overlap(modlist[i].paddr, modlist[i].size) )
+        {
+            printf("Skipping tests due to memory used by module[%d]\n", i);
+            return;
+        }
+        if ( check_string_overlap_with_test(modlist[i].cmdline_paddr) )
+        {
+            printf("Skipping tests due to memory used by"
+                   " module[%d]'s cmdline\n", i);
+            return;
+        }
+    }
+    if ( check_string_overlap_with_test(hvm_start_info->cmdline_paddr) )
+    {
+        printf("Skipping tests due to memory used by the"
+               " hvm_start_info->cmdline\n");
+        return;
+    }
+
     passed = skipped = 0;
     for ( i = 0; tests[i].test; i++ )
     {
         printf(" - %s ... ", tests[i].description);
-        memset((char *)(4ul << 20), 0, 4ul << 20);
+        memset((char *)TEST_START, 0, 4ul << 20);
         setup_paging();
         switch ( (*tests[i].test)() )
         {