[PATCH v2 2/2] lib: make safe_copy_string_from_guest() validate input

[Jan Beulich <jbeulich@xxxxxxxx>]

... rather than papering over guest flaws: Strings passed ought to be nul-
terminated (yet sadly libxc hasn't been doing so thus far). This way we
also avoid order-1 allocations, seeing that all present callers pass
PAGE_SIZE for max_size.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
I can't spot any caller side use of FLASK_DEVICETREE_LABEL, hence there's
no corresponding prereq patch.

--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@ The format is based on [Keep a Changelog
 ## [4.23.0 
UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
 - TBD
 
 ### Changed
+ - XEN_DOMCTL_DEV_DT's, FLASK_[GS]ETBOOL's, and FLASK_DEVICETREE_LABEL's input
+   string sizes need to include the nul terminator.
 
 ### Added
 
--- a/xen/lib/guest-strcpy.c
+++ b/xen/lib/guest-strcpy.c
@@ -3,8 +3,8 @@
 #include <xen/err.h>
 
 /*
- * The function copies a string from the guest and adds a NUL to
- * make sure the string is correctly terminated.
+ * The function copies a string from the guest and checks there's a NUL
+ * terminating the string.
  */
 char *safe_copy_string_from_guest(XEN_GUEST_HANDLE(char) u_buf,
                                   size_t size, size_t max_size)
@@ -14,8 +14,7 @@ char *safe_copy_string_from_guest(XEN_GU
     if ( size > max_size )
         return ERR_PTR(-ENOBUFS);
 
-    /* Add an extra +1 to append \0 */
-    tmp = xmalloc_array(char, size + 1);
+    tmp = xmalloc_array(char, size);
     if ( !tmp )
         return ERR_PTR(-ENOMEM);
 
@@ -24,7 +23,12 @@ char *safe_copy_string_from_guest(XEN_GU
         xfree(tmp);
         return ERR_PTR(-EFAULT);
     }
-    tmp[size] = '\0';
+
+    if ( !memchr(tmp, 0, size) )
+    {
+        xfree(tmp);
+        return ERR_PTR(-EMSGSIZE);
+    }
 
     return tmp;
 }
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -574,7 +574,7 @@ struct xen_domctl_assign_device {
             uint32_t machine_sbdf;   /* machine PCI ID of assigned device */
         } pci;
         struct {
-            uint32_t size; /* Length of the path */
+            uint32_t size; /* Length of the path, including nul terminator */
             XEN_GUEST_HANDLE_64(char) path; /* Path to the device tree node */
 #ifdef __XEN__
             struct dt_device_node *dev; /* Resolved device node of the above */
--- a/xen/include/public/xsm/flask_op.h
+++ b/xen/include/public/xsm/flask_op.h
@@ -26,7 +26,8 @@ typedef struct xen_flask_setenforce xen_
 struct xen_flask_sid_context {
     /* IN/OUT: sid to convert to/from string */
     uint32_t sid;
-    /* IN: size of the context buffer
+    /*
+     * IN: size of the context buffer, including nul terminator
      * OUT: actual size of the output context string
      */
     uint32_t size;
@@ -86,8 +87,11 @@ struct xen_flask_boolean {
     uint8_t new_value;
     /* IN: commit new value instead of only setting pending [SET] */
     uint8_t commit;
-    /* IN: size of boolean name buffer [GET/SET]
-     * OUT: actual size of name [GET only] */
+    /*
+     * IN: size of boolean name buffer [GET/SET]; must cover nul terminator
+     *     if "name" (below) is an input
+     * OUT: actual size of name [GET only]
+     */
     uint32_t size;
     /* IN: if bool_id is -1, used to find boolean [GET/SET]
      * OUT: textual name of boolean [GET only]
@@ -150,7 +154,7 @@ typedef struct xen_flask_relabel xen_fla
 struct xen_flask_devicetree_label {
     /* IN */
     uint32_t sid;
-    uint32_t length;
+    uint32_t length; /* length of the path, including nul terminator */
     XEN_GUEST_HANDLE(char) path;
 };
 typedef struct xen_flask_devicetree_label xen_flask_devicetree_label_t;