libx[lc]: Improve error reporting for invalid kernel images

libx[lc]: Improve error reporting for invalid kernel images

[Simon Gaiser]

Hi,

this series cleans up the error reporting in case a invalid kernel image
is supplied. For example if the user tries to boot a pre 4.11 Linux in
PVH mode.

Commit 96edb111dd ("libxc: panic when trying to create a PVH guest
without kernel support") already improved the situation:

  xc: error: panic: xc_dom_elfloader.c:66: xc_dom_guest_type: image not capable of booting inside a HVM container: Invalid kernel
  xc: error: panic: xc_dom_core.c:734: xc_dom_set_arch_hooks: not found (type xen-3.0-unknown): Invalid kernel
  xc: error: panic: xc_dom_core.c:936: xc_dom_mem_init: arch hooks not set: Internal error
  libxl: error: libxl_dom.c:729:libxl__build_dom: xc_dom_mem_init failed: No such file or directory
  libxl: error: libxl_create.c:1246:domcreate_rebuild_done: Domain 3:cannot (re-)build domain: -3

But you still see that the domain build don't fails immediately in
xc_dom_parse_elf_kernel() but only later in xc_dom_set_arch_hooks().
After fixing this there was still an unrelated errno string logged.

With this series the error reporting is cleaned up:

  xc: error: panic: xc_dom_elfloader.c:66: xc_dom_guest_type: image not capable of booting inside a HVM container: Invalid kernel
  libxl: error: libxl_dom.c:718:libxl__build_dom: xc_dom_parse_image failed
  libxl: error: libxl_create.c:1246:domcreate_rebuild_done: Domain 4:cannot (re-)build domain: -3

For some previous discussion see [1].

Simon

[1]: https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg01675.html


Simon Gaiser (3):
      libxc: Cleanup xc_dom_parse_elf_kernel()'s return value
      libxl: Improve logging in libxl__build_dom()
      libxc: xc_dom_parse_elf_kernel: Return error for invalid kernel images

 tools/libxc/xc_dom_elfloader.c | 24 +++++++++++++-----------
 tools/libxl/libxl_dom.c        |  2 +-
 2 files changed, 14 insertions(+), 12 deletions(-)

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

[PATCH 1/3] libxc: Cleanup xc_dom_parse_elf_kernel()'s return value

[Simon Gaiser]

xc_dom_loader.parser() should return elf_negerrnoval.

Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
---
 tools/libxc/xc_dom_elfloader.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 568d7f370c..c936f92a66 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -140,14 +140,10 @@ static elf_negerrnoval xc_dom_probe_elf_kernel(struct xc_dom_image *dom)
     return 0;
 }
 
-static elf_errorstatus xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
-    /*
-     * This function sometimes returns -1 for error and sometimes
-     * an errno value.  ?!?!
-     */
+static elf_negerrnoval xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
 {
     struct elf_binary *elf;
-    elf_errorstatus rc;
+    elf_negerrnoval rc;
 
     rc = check_elf_kernel(dom, 1);
     if ( rc != 0 )
@@ -155,9 +151,9 @@ static elf_errorstatus xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
 
     elf = xc_dom_malloc(dom, sizeof(*elf));
     if ( elf == NULL )
-        return -1;
+        return -ENOMEM;
     dom->private_loader = elf;
-    rc = elf_init(elf, dom->kernel_blob, dom->kernel_size);
+    rc = elf_init(elf, dom->kernel_blob, dom->kernel_size) != 0 ? -EINVAL : 0;
     xc_elf_set_logfile(dom->xch, elf, 1);
     if ( rc != 0 )
     {
@@ -177,8 +173,9 @@ static elf_errorstatus xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
 
     /* parse binary and get xen meta info */
     elf_parse_binary(elf);
-    if ( (rc = elf_xen_parse(elf, &dom->parms)) != 0 )
+    if ( elf_xen_parse(elf, &dom->parms) != 0 )
     {
+        rc = -EINVAL;
         goto out;
     }
 
-- 
2.15.1


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

[PATCH 2/3] libxl: Improve logging in libxl__build_dom()

[Simon Gaiser]

xc_dom_parse_image() does not set errno (at least in many code paths).
So LOGE() is not useful.

Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
---
 tools/libxl/libxl_dom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index ef834e652d..52b6137943 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -715,7 +715,7 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid,
     }
 #endif
     if ( (ret = xc_dom_parse_image(dom)) != 0 ) {
-        LOGE(ERROR, "xc_dom_parse_image failed");
+        LOG(ERROR, "xc_dom_parse_image failed");
         goto out;
     }
     if ( (ret = libxl__arch_domain_init_hw_description(gc, info, state, dom)) != 0 ) {
-- 
2.15.1


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

[PATCH 3/3] libxc: xc_dom_parse_elf_kernel: Return error for invalid kernel images

[Simon Gaiser]

Commit 96edb111dd ("libxc: panic when trying to create a PVH guest
without kernel support") already improved the handling of non PVH
capable kernels. But xc_dom_parse_elf_kernel() still returned success on
invalid elf images and the domain build only failed later. Now the build
process will fail immediately on detecting the error.

Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
---
 tools/libxc/xc_dom_elfloader.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index c936f92a66..26b2846365 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -64,7 +64,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
         xc_dom_panic(dom->xch, XC_INVALID_KERNEL,
                      "%s: image not capable of booting inside a HVM container",
                      __FUNCTION__);
-        return "xen-3.0-unknown";
+        return NULL;
     }
 
     switch ( machine )
@@ -86,7 +86,10 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
     case EM_X86_64:
         return "xen-3.0-x86_64";
     default:
-        return "xen-3.0-unknown";
+        xc_dom_panic(dom->xch, XC_INVALID_KERNEL,
+                     "%s: unkown image type %"PRIu64,
+                     __FUNCTION__, machine);
+        return NULL;
     }
 }
 
@@ -192,6 +195,8 @@ static elf_negerrnoval xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
     dom->kernel_seg.vend   = dom->parms.virt_kend;
 
     dom->guest_type = xc_dom_guest_type(dom, elf);
+    if ( dom->guest_type == NULL )
+        return -EINVAL;
     DOMPRINTF("%s: %s: 0x%" PRIx64 " -> 0x%" PRIx64 "",
               __FUNCTION__, dom->guest_type,
               dom->kernel_seg.vstart, dom->kernel_seg.vend);
-- 
2.15.1


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

Re: [PATCH 1/3] libxc: Cleanup xc_dom_parse_elf_kernel()'s return value

[Roger Pau Monné]

On Thu, Feb 08, 2018 at 10:49:08PM +0100, Simon Gaiser wrote:
> xc_dom_loader.parser() should return elf_negerrnoval.
> 
> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>

LGTM:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

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

Re: [PATCH 2/3] libxl: Improve logging in libxl__build_dom()

[Roger Pau Monné]

On Thu, Feb 08, 2018 at 10:49:09PM +0100, Simon Gaiser wrote:
> xc_dom_parse_image() does not set errno (at least in many code paths).
> So LOGE() is not useful.

IIRC the expectation is that libxc functions will return -1 on failure
and set errno. This is however far from true, so:

> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

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

Re: [PATCH 3/3] libxc: xc_dom_parse_elf_kernel: Return error for invalid kernel images

[Roger Pau Monné]

On Thu, Feb 08, 2018 at 10:49:10PM +0100, Simon Gaiser wrote:
> Commit 96edb111dd ("libxc: panic when trying to create a PVH guest
> without kernel support") already improved the handling of non PVH
> capable kernels. But xc_dom_parse_elf_kernel() still returned success on
> invalid elf images and the domain build only failed later. Now the build
> process will fail immediately on detecting the error.
> 
> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>

LGTM:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I'm not sure about the point of returning "xen-3.0-unknown" in
xc_dom_guest_type in the first place, but maybe I'm missing something.
When parsing the kernel there's no falling back to other loaders.

Thanks, Roger.

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

Re: libx[lc]: Improve error reporting for invalid kernel images

[Wei Liu]

On Thu, Feb 08, 2018 at 10:49:07PM +0100, Simon Gaiser wrote:
> Simon Gaiser (3):
>       libxc: Cleanup xc_dom_parse_elf_kernel()'s return value
>       libxl: Improve logging in libxl__build_dom()
>       libxc: xc_dom_parse_elf_kernel: Return error for invalid kernel images

Acked-by: Wei Liu <wei.liu2@citrix.com>

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