src: Don't rely on strncpy()-like behavior

The strncpy() function has this quirk where it will copy
*up* to the requested number of bytes, that is, it will
stop early if it encounters a NULL byte in the source
string.

This makes it legal to pass the size of the destination
buffer (minus one byte needed for the string terminator)
as the number of bytes to copy and still get something
somewhat reasonable out of the operation; unfortunately,
it also makes the function difficult to reason about
and way too easy to misuse.

We want to move away from the way strncpy() behaves and
towards better defined semantics, where virStrncpy()
will always copy *exactly* the number of bytes it's
been asked to copy; before we can do that, though, we
have to change a few of the callers.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

src/locking/lock_driver_sanlock.c

@@ -1004,7 +1004,8 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock,
     /* sanlock doesn't use owner_name for anything, so it's safe to take just
      * the first SANLK_NAME_LEN - 1 characters from vm_name */
     ignore_value(virStrncpy(opt->owner_name, priv->vm_name,
-                            SANLK_NAME_LEN - 1, SANLK_NAME_LEN));
+                            MIN(strlen(priv->vm_name), SANLK_NAME_LEN - 1),
+                            SANLK_NAME_LEN));
 
     if (state && STRNEQ(state, "")) {
         if ((rv = sanlock_state_to_args((char *)state,

src/xenapi/xenapi_driver.c

@@ -430,7 +430,9 @@ xenapiNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
     if (xen_host_cpu_get_all(session, &host_cpu_set)) {
         host_cpu = host_cpu_set->contents[0];
         xen_host_cpu_get_modelname(session, &modelname, host_cpu);
-        if (!virStrncpy(info->model, modelname, LIBVIRT_MODELNAME_LEN - 1, LIBVIRT_MODELNAME_LEN)) {
+        if (!virStrncpy(info->model, modelname,
+                        MIN(strlen(modelname), LIBVIRT_MODELNAME_LEN - 1),
+                        LIBVIRT_MODELNAME_LEN)) {
             virReportOOMError();
             xen_host_cpu_set_free(host_cpu_set);
             VIR_FREE(modelname);

src/xenconfig/xen_common.c

@@ -879,7 +879,7 @@ xenParseVif(char *entry, const char *vif_typename)
         data++;
 
         if (STRPREFIX(key, "mac=")) {
-            int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
+            int len = nextkey ? (nextkey - data) : strlen(data);
             if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("MAC address %s too big for destination"),
@@ -887,7 +887,7 @@ xenParseVif(char *entry, const char *vif_typename)
                 return NULL;
             }
         } else if (STRPREFIX(key, "bridge=")) {
-            int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1;
+            int len = nextkey ? (nextkey - data) : strlen(data);
             if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Bridge %s too big for destination"),
@@ -900,7 +900,7 @@ xenParseVif(char *entry, const char *vif_typename)
             if (VIR_STRNDUP(script, data, len) < 0)
                 return NULL;
         } else if (STRPREFIX(key, "model=")) {
-            int len = nextkey ? (nextkey - data) : sizeof(model) - 1;
+            int len = nextkey ? (nextkey - data) : strlen(data);
             if (virStrncpy(model, data, len, sizeof(model)) == NULL) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Model %s too big for destination"),
@@ -908,7 +908,7 @@ xenParseVif(char *entry, const char *vif_typename)
                 return NULL;
             }
         } else if (STRPREFIX(key, "type=")) {
-            int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
+            int len = nextkey ? (nextkey - data) : strlen(data);
             if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Type %s too big for destination"),
@@ -916,7 +916,7 @@ xenParseVif(char *entry, const char *vif_typename)
                 return NULL;
             }
         } else if (STRPREFIX(key, "vifname=")) {
-            int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1;
+            int len = nextkey ? (nextkey - data) : strlen(data);
             if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Vifname %s too big for destination"),
@@ -924,14 +924,14 @@ xenParseVif(char *entry, const char *vif_typename)
                 return NULL;
             }
         } else if (STRPREFIX(key, "ip=")) {
-            int len = nextkey ? (nextkey - data) : sizeof(ip) - 1;
+            int len = nextkey ? (nextkey - data) : strlen(data);
             if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("IP %s too big for destination"), data);
                 return NULL;
             }
         } else if (STRPREFIX(key, "rate=")) {
-            int len = nextkey ? (nextkey - data) : sizeof(rate) - 1;
+            int len = nextkey ? (nextkey - data) : strlen(data);
             if (virStrncpy(rate, data, len, sizeof(rate)) == NULL) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("rate %s too big for destination"), data);

src/xenconfig/xen_xl.c

@@ -899,7 +899,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def)
                 data++;
 
                 if (STRPREFIX(key, "type=")) {
-                    int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
+                    int len = nextkey ? (nextkey - data) : strlen(data);
                     if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
                         virReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("type %s invalid"),
@@ -907,7 +907,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def)
                         goto skipusbctrl;
                     }
                 } else if (STRPREFIX(key, "version=")) {
-                    int len = nextkey ? (nextkey - data) : sizeof(version) - 1;
+                    int len = nextkey ? (nextkey - data) : strlen(data);
                     if (virStrncpy(version, data, len, sizeof(version)) == NULL) {
                         virReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("version %s invalid"),
@@ -917,7 +917,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def)
                     if (virStrToLong_i(version, NULL, 16, &usbctrl_version) < 0)
                         goto skipusbctrl;
                 } else if (STRPREFIX(key, "ports=")) {
-                    int len = nextkey ? (nextkey - data) : sizeof(ports) - 1;
+                    int len = nextkey ? (nextkey - data) : strlen(data);
                     if (virStrncpy(ports, data, len, sizeof(ports)) == NULL) {
                         virReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("version %s invalid"),
@@ -1001,7 +1001,7 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def)
                 data++;
 
                 if (STRPREFIX(key, "hostbus=")) {
-                    int len = nextkey ? (nextkey - data) : sizeof(bus) - 1;
+                    int len = nextkey ? (nextkey - data) : strlen(data);
                     if (virStrncpy(bus, data, len, sizeof(bus)) == NULL) {
                         virReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("bus %s too big for destination"),
@@ -1009,7 +1009,7 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def)
                         goto skipusb;
                     }
                 } else if (STRPREFIX(key, "hostaddr=")) {
-                    int len = nextkey ? (nextkey - data) : sizeof(device) - 1;
+                    int len = nextkey ? (nextkey - data) : strlen(data);
                     if (virStrncpy(device, data, len, sizeof(device)) == NULL) {
                         virReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("device %s too big for destination"),
@@ -1077,7 +1077,7 @@ xenParseXLChannel(virConfPtr conf, virDomainDefPtr def)
                 data++;
 
                 if (STRPREFIX(key, "connection=")) {
-                    int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
+                    int len = nextkey ? (nextkey - data) : strlen(data);
                     if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
                         virReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("connection %s too big"), data);