All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] libxl: don't segfault when creating domain with invalid pvusb device
@ 2017-02-08  7:04 Juergen Gross
  2017-02-08 10:34 ` Wei Liu
  0 siblings, 1 reply; 2+ messages in thread
From: Juergen Gross @ 2017-02-08  7:04 UTC (permalink / raw)
  To: xen-devel; +Cc: Juergen Gross, wei.liu2, ian.jackson

Creating a domain with an invalid controller specification for a pvusb
device will currently segfault.

Avoid this by bailing out early in case of a mandatory xenstore path
not existing.

Signed-of-by: Juergen Gross <jgross@suse.com>
---
This patch is a backport candidate for 4.8

V2: introduce libxl__xs_read_mandatory() as suggested by Ian Jackson
---
 tools/libxl/libxl_internal.h |  6 ++++++
 tools/libxl/libxl_usb.c      |  6 +++---
 tools/libxl/libxl_xshelp.c   | 12 ++++++++++++
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5f46578..d591b79 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -738,6 +738,12 @@ int libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t,
 
 /* On success, *result_out came from the gc.
  * On error, *result_out is undefined.
+ */
+int libxl__xs_read_mandatory(libxl__gc *gc, xs_transaction_t t,
+                             const char *path, const char **result_out);
+
+/* On success, *result_out came from the gc.
+ * On error, *result_out is undefined.
  * ENOENT counts as success but sets *result_out=0
  */
 int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index ea7a2ab..d8948d5 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -652,9 +652,9 @@ int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
     usbctrlinfo->devid = usbctrl->devid;
 
 #define READ_SUBPATH(path, subpath) ({                                  \
-        rc = libxl__xs_read_checked(gc, XBT_NULL,                       \
-                                    GCSPRINTF("%s/" subpath, path),     \
-                                    &tmp);                              \
+        rc = libxl__xs_read_mandatory(gc, XBT_NULL,                     \
+                                      GCSPRINTF("%s/" subpath, path),   \
+                                      &tmp);                            \
         if (rc) goto out;                                               \
         (char *)tmp;                                                    \
     })
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index a50805f..c4a18df 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -193,6 +193,18 @@ char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid)
     return s;
 }
 
+int libxl__xs_read_mandatory(libxl__gc *gc, xs_transaction_t t,
+                             const char *path, const char **result_out)
+{
+    char *result = libxl__xs_read(gc, t, path);
+    if (!result) {
+        LOGE(ERROR, "xenstore read failed: `%s'", path);
+        return ERROR_FAIL;
+    }
+    *result_out = result;
+    return 0;
+}
+
 int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
                            const char *path, const char **result_out)
 {
-- 
2.10.2


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

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

* Re: [PATCH v2] libxl: don't segfault when creating domain with invalid pvusb device
  2017-02-08  7:04 [PATCH v2] libxl: don't segfault when creating domain with invalid pvusb device Juergen Gross
@ 2017-02-08 10:34 ` Wei Liu
  0 siblings, 0 replies; 2+ messages in thread
From: Wei Liu @ 2017-02-08 10:34 UTC (permalink / raw)
  To: Juergen Gross; +Cc: xen-devel, ian.jackson, wei.liu2

On Wed, Feb 08, 2017 at 08:04:04AM +0100, Juergen Gross wrote:
> Creating a domain with an invalid controller specification for a pvusb
> device will currently segfault.
> 
> Avoid this by bailing out early in case of a mandatory xenstore path
> not existing.
> 
> Signed-of-by: Juergen Gross <jgross@suse.com>
> ---
> This patch is a backport candidate for 4.8
> 
> V2: introduce libxl__xs_read_mandatory() as suggested by Ian Jackson
> ---
>  tools/libxl/libxl_internal.h |  6 ++++++
>  tools/libxl/libxl_usb.c      |  6 +++---
>  tools/libxl/libxl_xshelp.c   | 12 ++++++++++++
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 5f46578..d591b79 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -738,6 +738,12 @@ int libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t,
>  
>  /* On success, *result_out came from the gc.
>   * On error, *result_out is undefined.
> + */

Please spell out the distinction between this function and the _checked
variant: This doesn't tolerate ENOENT.

Other than this, the code looks good.

> +int libxl__xs_read_mandatory(libxl__gc *gc, xs_transaction_t t,
> +                             const char *path, const char **result_out);
> +
> +/* On success, *result_out came from the gc.
> + * On error, *result_out is undefined.
>   * ENOENT counts as success but sets *result_out=0
>   */
>  int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
> diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
> index ea7a2ab..d8948d5 100644
> --- a/tools/libxl/libxl_usb.c
> +++ b/tools/libxl/libxl_usb.c
> @@ -652,9 +652,9 @@ int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
>      usbctrlinfo->devid = usbctrl->devid;
>  
>  #define READ_SUBPATH(path, subpath) ({                                  \
> -        rc = libxl__xs_read_checked(gc, XBT_NULL,                       \
> -                                    GCSPRINTF("%s/" subpath, path),     \
> -                                    &tmp);                              \
> +        rc = libxl__xs_read_mandatory(gc, XBT_NULL,                     \
> +                                      GCSPRINTF("%s/" subpath, path),   \
> +                                      &tmp);                            \
>          if (rc) goto out;                                               \
>          (char *)tmp;                                                    \
>      })
> diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
> index a50805f..c4a18df 100644
> --- a/tools/libxl/libxl_xshelp.c
> +++ b/tools/libxl/libxl_xshelp.c
> @@ -193,6 +193,18 @@ char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid)
>      return s;
>  }
>  
> +int libxl__xs_read_mandatory(libxl__gc *gc, xs_transaction_t t,
> +                             const char *path, const char **result_out)
> +{
> +    char *result = libxl__xs_read(gc, t, path);
> +    if (!result) {
> +        LOGE(ERROR, "xenstore read failed: `%s'", path);
> +        return ERROR_FAIL;
> +    }
> +    *result_out = result;
> +    return 0;
> +}
> +
>  int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
>                             const char *path, const char **result_out)
>  {
> -- 
> 2.10.2
> 

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

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

end of thread, other threads:[~2017-02-08 10:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-08  7:04 [PATCH v2] libxl: don't segfault when creating domain with invalid pvusb device Juergen Gross
2017-02-08 10:34 ` 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.