[libvirt] [PATCH v2 5/8] src: Don't rely on strncpy()-like behavior
Andrea Bolognani
abologna at redhat.com
Fri Jul 20 14:44:10 UTC 2018
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: Andrea Bolognani <abologna at redhat.com>
---
src/locking/lock_driver_sanlock.c | 3 ++-
src/xenapi/xenapi_driver.c | 4 +++-
src/xenconfig/xen_common.c | 14 +++++++-------
src/xenconfig/xen_xl.c | 12 ++++++------
4 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index 345cf0a772..3f3a587541 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/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,
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 42b305d316..f4375c5874 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/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);
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 4a94127da1..815ccd030e 100644
--- a/src/xenconfig/xen_common.c
+++ b/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);
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 807fe621d6..bc3191ad5e 100644
--- a/src/xenconfig/xen_xl.c
+++ b/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);
--
2.17.1
More information about the libvir-list
mailing list