- Devel - libvirt List Archives

[libvirt] [PATCH] support lzop save compression for qemu
by Charles Duffy 01 Sep '09

01 Sep '09

Per prior discussion -- this was, indeed, trivial. I'm a little disappointed to be breaking the ordering characteristics of the enum (as it had been ordered by increasing time requirements and decreasing output size), but breaking any save files with the old constants in the headers would of course be worse.

3 5

[libvirt] [PATCH] OpenVZ: accept NULL as type for GetMaxVCPUs.
by Chris Lalancette 01 Sep '09

01 Sep '09

All of the other drivers that support the getMaxVcpus callback also accept a NULL value for type. Make openvz also accept a NULL value. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/openvz_driver.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/openvz_driver.c b/src/openvz_driver.c index 6b7c49d..43d54f9 100644 --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -1000,9 +1000,10 @@ cleanup: return ret; } -static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type) { - if (STRCASEEQ(type, "openvz")) - return 1028; //OpenVZ has no limitation +static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type) +{ + if (type == NULL || STRCASEEQ(type, "openvz")) + return 1028; /* OpenVZ has no limitation */ openvzError(conn, VIR_ERR_INVALID_ARG, _("unknown type '%s'"), type); -- 1.6.0.6

2 2

[libvirt] [PATCH] Remove use of strncpy in qemudExtractMonitorPath.
by Chris Lalancette 01 Sep '09

01 Sep '09

qemudExtractMonitorPath() was doing a VIR_ALLOC_N followed by a strncpy. However, this isn't necessary; we can do the same thing using strndup, which is much safer. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 772f2f9..a40d386 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1029,12 +1029,12 @@ qemudExtractMonitorPath(virConnectPtr conn, */ while (*tmp) { if (c_isspace(*tmp)) { - if (VIR_ALLOC_N(*path, (tmp-dev)+1) < 0) { + *path = strndup(dev, tmp-dev); + if (*path == NULL) { virReportOOMError(conn); return -1; } - strncpy(*path, dev, (tmp-dev)); - (*path)[(tmp-dev)] = '\0'; + /* ... now further update offset till we get EOL */ *offset = tmp - haystack; return 0; -- 1.6.0.6

2 2

[libvirt] [PATCH]: Fix up virNodeGetCellsFreeMemory
by Chris Lalancette 01 Sep '09

01 Sep '09

The documentation for virNodeGetCellsFreeMemory claims the values returned are in kilobytes, but that's actually wrong; the value returned is actually in bytes. Fix up the documentation to be correct. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- docs/devhelp/libvirt-libvirt.html | 2 +- docs/html/libvirt-libvirt.html | 2 +- docs/libvirt-api.xml | 2 +- docs/libvirt-refs.xml | 1 - src/libvirt.c | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-)

2 2

[libvirt] [PATCH] Introduce virStrncpy.
by Chris Lalancette 31 Aug '09

31 Aug '09

Add the virStrncpy function, which takes a dst string, source string, the number of bytes to copy and the number of bytes available in the dest string. If the source string is too large to fit into the destination string, including the \0 byte, then no data is copied and the function returns NULL. Otherwise, this function copies n bytes from source into dst, including the \0, and returns a pointer to the dst string. This function is intended to replace all unsafe uses of strncpy in the code base, since strncpy does *not* guarantee that the buffer terminates with a \0. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- HACKING | 29 +++++++++ cfg.mk | 6 ++ proxy/libvirt_proxy.c | 6 ++- qemud/Makefile.am | 3 +- qemud/qemud.c | 6 +- qemud/remote.c | 8 ++- src/bridge.c | 89 ++++++++------------------- src/datatypes.c | 9 ++- src/esx/esx_driver.c | 9 ++- src/esx/esx_vmx.c | 7 ++- src/lxc_controller.c | 6 ++- src/lxc_driver.c | 12 +++- src/nodeinfo.c | 4 +- src/opennebula/one_client.c | 11 ++-- src/openvz_conf.c | 33 +++++++--- src/phyp/phyp_driver.c | 2 +- src/proxy_internal.c | 5 +- src/qemu_conf.c | 16 +++--- src/qemu_driver.c | 12 +++- src/remote_internal.c | 19 ++++-- src/test.c | 24 +++++-- src/uml_driver.c | 17 ++++-- src/util.c | 36 +++++++++-- src/util.h | 4 + src/xen_internal.c | 22 +++++-- src/xend_internal.c | 65 ++++++++++++------- src/xm_internal.c | 145 ++++++++++++++++++++++++++++--------------- 27 files changed, 392 insertions(+), 213 deletions(-) diff --git a/HACKING b/HACKING index da28e98..7a38f05 100644 --- a/HACKING +++ b/HACKING @@ -231,6 +231,35 @@ one of the following semantically named macros +String copying +============== + +Do not use the strncpy function. According to the man page, it does +*not* guarantee a NULL-terminated buffer, which makes it extremely dangerous +to use. Instead, use one of the functionally equivalent functions: + + - virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) + The first three arguments have the same meaning as for strncpy; namely the + destination, source, and number of bytes to copy, respectively. The last + argument is the number of bytes available in the destination string; if a + copy of the source string (including a \0) will not fit into the + destination, no bytes are copied and the routine returns NULL. + Otherwise, n bytes from the source are copied into the destination and a + trailing \0 is appended. + + - virStrcpy(char *dest, const char *src, size_t destbytes) + Use this variant if you know you want to copy the entire src string + into dest. This is equivalent to + virStrncpy(dest, src, strlen(src), destbytes) + + - virStrcpyStatic(char *dest, const char *src) + Use this variant if you know you want to copy the entire src string + into dest *and* you know that your destination string is a static string + (i.e. that sizeof(dest) returns something meaningful). This is + equivalent to virStrncpy(dest, src, strlen(src), sizeof(dest)). + + + Variable length string buffer ============================= diff --git a/cfg.mk b/cfg.mk index 64bd26e..2d2dd7e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -91,6 +91,12 @@ sc_prohibit_asprintf: msg='use virAsprintf, not a'sprintf \ $(_prohibit_regexp) +sc_prohibit_strncpy: + @grep -nE 'strncpy *\(' \ + $$($(VC_LIST) | grep -v 'src/util.c') && \ + { echo '$(ME): use virStrncpy, not strncpy' \ + 1>&2; exit 1; } || : + sc_prohibit_VIR_ERR_NO_MEMORY: @re='\<V''IR_ERR_NO_MEMORY\>' \ msg='use virReportOOMError, not V'IR_ERR_NO_MEMORY \ diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index e008a7f..f058b38 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -170,7 +170,11 @@ proxyListenUnixSocket(const char *path) { memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + fprintf(stderr, "Path %s too long to fit into destination\n", path); + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 3d143da..67f1b4a 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -108,9 +108,10 @@ libvirtd_LDADD = \ $(SASL_LIBS) \ $(POLKIT_LIBS) +libvirtd_LDADD += ../src/libvirt_util.la + if WITH_DRIVER_MODULES libvirtd_LDADD += ../src/libvirt_driver.la - libvirtd_LDADD += ../src/libvirt_util.la else if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la diff --git a/qemud/qemud.c b/qemud/qemud.c index df275e6..fd6e0af 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -506,11 +506,13 @@ static int qemudListenUnix(struct qemud_server *server, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, path, sizeof(addr.sun_path)-1); + if (virStrcpyStatic(addr.sun_path, path) == NULL) { + VIR_ERROR(_("Path %s too long for unix socket"), path); + goto cleanup; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; - oldgrp = getgid(); oldmask = umask(readonly ? ~unix_sock_ro_mask : ~unix_sock_rw_mask); if (server->privileged) diff --git a/qemud/remote.c b/qemud/remote.c index b29d5d8..990c15f 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -554,9 +554,11 @@ remoteDispatchDomainSetSchedulerParameters (struct qemud_server *server ATTRIBUT /* Deserialise parameters. */ for (i = 0; i < nparams; ++i) { - strncpy (params[i].field, args->params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[i].field, args->params.params_val[i].field) == NULL) { + remoteDispatchFormatError(rerr, _("Field %s too big for destination"), + args->params.params_val[i].field); + return -1; + } params[i].type = args->params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/bridge.c b/src/bridge.c index 414d87b..f9a7668 100644 --- a/src/bridge.c +++ b/src/bridge.c @@ -149,23 +149,19 @@ brHasBridge(brControl *ctl, const char *name) { struct ifreq ifr; - int len; if (!ctl || !name) { errno = EINVAL; return -1; } - if ((len = strlen(name)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpyStatic(ifr.ifr_name, name) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, name, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr)) return -1; @@ -216,18 +212,14 @@ brAddDelInterface(brControl *ctl, const char *iface) { struct ifreq ifr; - int len; if (!ctl || !ctl->fd || !bridge || !iface) return EINVAL; - if ((len = strlen(bridge)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, bridge, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, bridge) == NULL) + return EINVAL; if (!(ifr.ifr_ifindex = if_nametoindex(iface))) return ENODEV; @@ -305,23 +297,19 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED, static int ifGetMtu(brControl *ctl, const char *ifname) { struct ifreq ifr; - int len; if (!ctl || !ifname) { errno = EINVAL; return -1; } - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFMTU, &ifr)) return -1; @@ -343,18 +331,14 @@ static int ifGetMtu(brControl *ctl, const char *ifname) static int ifSetMtu(brControl *ctl, const char *ifname, int mtu) { struct ifreq ifr; - int len; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; ifr.ifr_mtu = mtu; return ioctl(ctl->fd, SIOCSIFMTU, &ifr) == 0 ? 0 : errno; @@ -466,7 +450,7 @@ brAddTap(brControl *ctl, int vnet_hdr, int *tapfd) { - int fd, len; + int fd; struct ifreq ifr; if (!ctl || !ctl->fd || !bridge || !ifname) @@ -486,17 +470,14 @@ brAddTap(brControl *ctl, (void) vnet_hdr; #endif - strncpy(ifr.ifr_name, *ifname, IFNAMSIZ-1); - - if (ioctl(fd, TUNSETIFF, &ifr) < 0) - goto error; - - len = strlen(ifr.ifr_name); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { errno = EINVAL; goto error; } + if (ioctl(fd, TUNSETIFF, &ifr) < 0) + goto error; + /* We need to set the interface MTU before adding it * to the bridge, because the bridge will have its * MTU adjusted automatically when we add the new interface. @@ -527,7 +508,6 @@ int brDeleteTap(brControl *ctl, const char *ifname) { struct ifreq try; - int len; int fd; if (!ctl || !ctl->fd || !ifname) @@ -539,15 +519,11 @@ int brDeleteTap(brControl *ctl, memset(&try, 0, sizeof(struct ifreq)); try.ifr_flags = IFF_TAP|IFF_NO_PI; - len = strlen(ifname); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpyStatic(try.ifr_name, ifname) == NULL) { errno = EINVAL; goto error; } - strncpy(try.ifr_name, ifname, len); - try.ifr_name[len] = '\0'; - if (ioctl(fd, TUNSETIFF, &try) == 0) { if ((errno = ioctl(fd, TUNSETPERSIST, 0))) goto error; @@ -576,19 +552,15 @@ brSetInterfaceUp(brControl *ctl, int up) { struct ifreq ifr; - int len; int flags; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -621,18 +593,14 @@ brGetInterfaceUp(brControl *ctl, int *up) { struct ifreq ifr; - int len; if (!ctl || !ifname || !up) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -654,18 +622,15 @@ brSetInetAddr(brControl *ctl, } s; struct ifreq ifr; struct in_addr inaddr; - int len, ret; + int ret; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if ((ret = inet_pton(AF_INET, addr, &inaddr)) < 0) return errno; @@ -692,18 +657,14 @@ brGetInetAddr(brControl *ctl, { struct ifreq ifr; struct in_addr *inaddr; - int len; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, cmd, &ifr) < 0) return errno; diff --git a/src/datatypes.c b/src/datatypes.c index d03a679..4a8bffa 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -25,6 +25,7 @@ #include "virterror_internal.h" #include "logging.h" #include "memory.h" +#include "util.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -899,8 +900,12 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c virReportOOMError(conn); goto error; } - strncpy(ret->key, key, sizeof(ret->key)-1); - ret->key[sizeof(ret->key)-1] = '\0'; + if (virStrcpyStatic(ret->key, key) == NULL) { + virMutexUnlock(&conn->lock); + virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, + _("Volume key %s too large for destination"), key); + goto error; + } ret->magic = VIR_STORAGE_VOL_MAGIC; ret->conn = conn; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0225e9a..24c4422 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -694,9 +694,12 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) ++ptr; } - strncpy (nodeinfo->model, dynamicProperty->val->string, - sizeof (nodeinfo->model) - 1); - nodeinfo->model[sizeof (nodeinfo->model) - 1] = '\0'; + if (virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Model %s too long for destination", + dynamicProperty->val->string); + goto failure; + } } else { VIR_WARN("Unexpected '%s' property", dynamicProperty->name); } diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index 54c2594..2a58fd8 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -883,8 +883,11 @@ esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix) return NULL; } - strncpy(buffer, prefix, sizeof (buffer) - 1); - buffer[sizeof (buffer) - 1] = '\0'; + if (virStrcpyStatic(buffer, prefix) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Prefix %s too big for destination", prefix); + return NULL; + } if (idx < 26) { buffer[length] = 'a' + idx; diff --git a/src/lxc_controller.c b/src/lxc_controller.c index 8d11238..f7729ba 100644 --- a/src/lxc_controller.c +++ b/src/lxc_controller.c @@ -178,7 +178,11 @@ static int lxcMonitorServer(const char *sockpath) unlink(sockpath); memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, sockpath) == NULL) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too long for destination"), sockpath); + goto error; + } if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(NULL, errno, diff --git a/src/lxc_driver.c b/src/lxc_driver.c index bd0cf0e..fd49789 100644 --- a/src/lxc_driver.c +++ b/src/lxc_driver.c @@ -719,7 +719,11 @@ static int lxcMonitorClient(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, sockpath) == NULL) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too big for destination"), sockpath); + goto error; + } if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(conn, errno, "%s", @@ -1739,7 +1743,11 @@ static int lxcGetSchedulerParameters(virDomainPtr domain, if (virCgroupGetCpuShares(group, &val) != 0) goto cleanup; params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); + if (virStrcpyStatic(params[0].field, "cpu_shares") == NULL) { + lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too big for destination")); + goto cleanup; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; ret = 0; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0a211a7..7d26b2b 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -141,8 +141,8 @@ int nodeGetInfo(virConnectPtr conn, uname(&info); - strncpy(nodeinfo->model, info.machine, sizeof(nodeinfo->model)-1); - nodeinfo->model[sizeof(nodeinfo->model)-1] = '\0'; + if (virStrcpyStatic(nodeinfo->model, info.machine) == NULL) + return -1; #else /* !HAVE_UNAME */ diff --git a/src/opennebula/one_client.c b/src/opennebula/one_client.c index 21f303a..39bb1ea 100644 --- a/src/opennebula/one_client.c +++ b/src/opennebula/one_client.c @@ -24,6 +24,8 @@ #include <fcntl.h> #include <unistd.h> #include "one_client.h" +#include "datatypes.h" +#include "util.h" oneClient one_client; @@ -171,7 +173,7 @@ int c_oneFinalize(int vmid) return c_oneAction(vmid, (char *)"finalize"); } -int c_oneVmInfo(int vmid, char* ret_info,int length) +int c_oneVmInfo(int vmid, char* ret_info, int length) { xmlrpc_value *resultP; int return_code; @@ -186,10 +188,9 @@ int c_oneVmInfo(int vmid, char* ret_info,int length) if( return_code ) { - strncpy(ret_info, return_string, length-1); - ret_info[length-1] = '\0'; - - retval = 0; + if (virStrncpy(ret_info, return_string, length-1, length) != NULL) + /* Only set the return value to 0 if we succeeded */ + retval = 0; } xmlrpc_DECREF(resultP); diff --git a/src/openvz_conf.c b/src/openvz_conf.c index a172fe3..08951df 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -261,8 +261,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->ifname, len+1) < 0) goto no_memory; - strncpy(net->ifname, p, len); - net->ifname[len] = '\0'; + if (virStrncpy(net->ifname, p, len, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Network ifname %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "bridge=")) { p += 7; len = next - p; @@ -275,8 +278,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->data.bridge.brname, len+1) < 0) goto no_memory; - strncpy(net->data.bridge.brname, p, len); - net->data.bridge.brname[len] = '\0'; + if (virStrncpy(net->data.bridge.brname, p, len, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge name %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "mac=")) { p += 4; len = next - p; @@ -285,8 +291,11 @@ openvzReadNetworkConf(virConnectPtr conn, "%s", _("Wrong length MAC address")); goto error; } - strncpy(cpy_temp, p, len); - cpy_temp[len] = '\0'; + if (virStrncpy(cpy_temp, p, len, sizeof(cpy_temp)) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too long for destination"), p); + goto error; + } if (virParseMacAddr(cpy_temp, net->mac) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Wrong MAC address")); @@ -629,8 +638,10 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char *value, i if (sf[0] == '=' && sf[1] != '\0' ) { sf ++; if ((token = strtok_r(sf,"\"\t\n", &saveptr)) != NULL) { - strncpy(value, token, maxlen) ; - value[maxlen-1] = '\0'; + if (virStrcpy(value, token, maxlen) == NULL) { + ret = -1; + break; + } found = 1; } } @@ -812,6 +823,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) char uuidbuf[1024]; char iden[1024]; int fd, ret; + int retval = 0; if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) return -1; @@ -832,13 +844,14 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) sscanf(line, "%s %s\n", iden, uuidbuf); if(STREQ(iden, "#UUID:")) { - strncpy(uuidstr, uuidbuf, len); + if (virStrcpy(uuidstr, uuidbuf, len) == NULL) + retval = -1; break; } } close(fd); - return 0; + return retval; } /* Do actual checking for UUID presence in conf file, diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 3100144..22c4fee 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1387,7 +1387,7 @@ escape_specialcharacters(char *src, char *dst, size_t dstlen) } temp_buffer[j] = '\0'; - if (strncpy(dst, temp_buffer, dstlen) == NULL) + if (virStrcpy(dst, temp_buffer, dstlen) == NULL) return -1; return 0; diff --git a/src/proxy_internal.c b/src/proxy_internal.c index 5b92ad8..8609201 100644 --- a/src/proxy_internal.c +++ b/src/proxy_internal.c @@ -197,7 +197,10 @@ retry: memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..f47ada2 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1302,18 +1302,18 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, { switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - strncpy(buf, "null", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "null", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_VC: - strncpy(buf, "vc", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "vc", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_PTY: - strncpy(buf, "pty", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "pty", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_DEV: @@ -1335,8 +1335,8 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, break; case VIR_DOMAIN_CHR_TYPE_STDIO: - strncpy(buf, "stdio", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "stdio", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_UDP: diff --git a/src/qemu_driver.c b/src/qemu_driver.c index a40d386..f99d4c0 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -921,7 +921,11 @@ qemudOpenMonitorUnix(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, monitor, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, monitor) == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Monitor path %s too big for destination"), monitor); + goto error; + } do { ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); @@ -6241,8 +6245,12 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, goto cleanup; } params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; + if (virStrcpyStatic(params[0].field, "cpu_shares") == NULL) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too long for destination")); + goto cleanup; + } ret = 0; diff --git a/src/remote_internal.c b/src/remote_internal.c index 26425c2..e288188 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -652,7 +652,11 @@ doRemoteOpen (virConnectPtr conn, memset (&addr, 0, sizeof addr); addr.sun_family = AF_UNIX; - strncpy (addr.sun_path, sockname, UNIX_PATH_MAX (addr)); + if (virStrcpyStatic(addr.sun_path, sockname) == NULL) { + errorf(conn, VIR_ERR_INTERNAL_ERROR, + _("Socket %s too big for destination"), sockname); + goto failed; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; @@ -1567,8 +1571,8 @@ remoteNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info) (xdrproc_t) xdr_remote_node_get_info_ret, (char *) &ret) == -1) goto done; - strncpy (info->model, ret.model, 32); - info->model[31] = '\0'; + if (virStrcpyStatic(info->model, ret.model) == NULL) + goto done; info->memory = ret.memory; info->cpus = ret.cpus; info->mhz = ret.mhz; @@ -2999,9 +3003,12 @@ remoteDomainGetSchedulerParameters (virDomainPtr domain, /* Deserialise the result. */ for (i = 0; i < *nparams; ++i) { - strncpy (params[i].field, ret.params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) == NULL) { + errorf(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Parameter %s too big for destination"), + ret.params.params_val[i].field); + goto cleanup; + } params[i].type = ret.params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/test.c b/src/test.c index 305f2c9..9bb8e85 100644 --- a/src/test.c +++ b/src/test.c @@ -415,10 +415,13 @@ static char *testBuildFilename(const char *relativeTo, offset = strrchr(relativeTo, '/'); if ((baseLen = (offset-relativeTo+1))) { char *absFile; - if (VIR_ALLOC_N(absFile, baseLen + strlen(filename) + 1) < 0) + int totalLen = baseLen + strlen(filename) + 1; + if (VIR_ALLOC_N(absFile, totalLen) < 0) return NULL; - strncpy(absFile, relativeTo, baseLen); - absFile[baseLen] = '\0'; + if (virStrncpy(absFile, relativeTo, baseLen, totalLen) == NULL) { + VIR_FREE(absFile); + return NULL; + } strcat(absFile, filename); return absFile; } else { @@ -571,8 +574,11 @@ static int testOpenFromFile(virConnectPtr conn, privconn->nextDomID = 1; privconn->numCells = 0; - strncpy(privconn->path, file, PATH_MAX-1); - privconn->path[PATH_MAX-1] = '\0'; + if (virStrcpyStatic(privconn->path, file) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Path %s too big for destination"), file); + goto error; + } memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); nodeInfo = &privconn->nodeInfo; @@ -628,8 +634,12 @@ static int testOpenFromFile(virConnectPtr conn, str = virXPathString(conn, "string(/node/cpu/model[1])", ctxt); if (str != NULL) { - strncpy(nodeInfo->model, str, sizeof(nodeInfo->model)-1); - nodeInfo->model[sizeof(nodeInfo->model)-1] = '\0'; + if (virStrcpyStatic(nodeInfo->model, str) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), str); + VIR_FREE(str); + goto error; + } VIR_FREE(str); } diff --git a/src/uml_driver.c b/src/uml_driver.c index a2b9495..ecc0c75 100644 --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -562,6 +562,7 @@ static int umlMonitorAddress(virConnectPtr conn, virDomainObjPtr vm, struct sockaddr_un *addr) { char *sockname; + int retval = 0; if (virAsprintf(&sockname, "%s/%s/mconsole", driver->monitorDir, vm->def->name) < 0) { @@ -571,10 +572,13 @@ static int umlMonitorAddress(virConnectPtr conn, memset(addr, 0, sizeof *addr); addr->sun_family = AF_UNIX; - strncpy(addr->sun_path, sockname, sizeof(addr->sun_path)-1); - NUL_TERMINATE(addr->sun_path); + if (virStrcpyStatic(addr->sun_path, sockname) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Unix path %s too long for destination"), sockname); + retval = -1; + } VIR_FREE(sockname); - return 0; + return retval; } static int umlOpenMonitor(virConnectPtr conn, @@ -668,8 +672,11 @@ static int umlMonitorCommand(virConnectPtr conn, cmd, req.length); return -1; } - strncpy(req.data, cmd, req.length); - req.data[req.length] = '\0'; + if (virStrcpyStatic(req.data, cmd) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Command %s too long for destination"), cmd); + return -1; + } if (sendto(vm->monitor, &req, sizeof req, 0, (struct sockaddr *)&addr, sizeof addr) != (sizeof req)) { diff --git a/src/util.c b/src/util.c index 0d4f3fa..c021953 100644 --- a/src/util.c +++ b/src/util.c @@ -1121,9 +1121,8 @@ char *virFindFileInPath(const char *file) char fullpath[PATH_MAX]; /* copy PATH env so we can tweak it */ - strncpy(pathenv, getenv("PATH"), PATH_MAX); - pathenv[PATH_MAX - 1] = '\0'; - + if (virStrcpyStatic(pathenv, getenv("PATH")) == NULL) + return NULL; /* for each path segment, append the file to search for and test for * it. return it if found. @@ -1155,8 +1154,8 @@ int virFileMakePath(const char *path) if (stat(path, &st) >= 0) return 0; - strncpy(parent, path, PATH_MAX); - parent[PATH_MAX - 1] = '\0'; + if (virStrcpyStatic(parent, path) == NULL) + return EINVAL; if (!(p = strrchr(parent, '/'))) return EINVAL; @@ -1573,6 +1572,33 @@ virAsprintf(char **strp, const char *fmt, ...) return ret; } +/** + * virStrncpy + * + * A safe version of strncpy. The last parameter is the number of bytes + * available in the destination string, *not* the number of bytes you want + * to copy. If the destination is not large enough to hold all n of the + * src string bytes plus a \0, NULL is returned and no data is copied. + * If the destination is large enough to hold the n bytes plus \0, then the + * string is copied and a pointer to the destination string is returned. + */ +char * +virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) +{ + char *ret; + + if (n > (destbytes - 1)) + return NULL; + + ret = strncpy(dest, src, n); + /* strncpy NULL terminates iff the last character is \0. Therefore + * force the last byte to be \0 + */ + dest[n] = '\0'; + + return ret; +} + /* Compare two MAC addresses, ignoring differences in case, * as well as leading zeros. */ diff --git a/src/util.h b/src/util.h index b3e628a..895e07d 100644 --- a/src/util.h +++ b/src/util.h @@ -164,6 +164,10 @@ void virSkipSpaces(const char **str); int virParseNumber(const char **str); int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(2, 3); +char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) + ATTRIBUTE_RETURN_CHECK; +#define virStrcpy(dest, src, destbytes) virStrncpy(dest, src, strlen(src), destbytes) +#define virStrcpyStatic(dest, src) virStrncpy(dest, src, strlen(src), sizeof(dest)) #define VIR_MAC_BUFLEN 6 #define VIR_MAC_PREFIX_BUFLEN 3 diff --git a/src/xen_internal.c b/src/xen_internal.c index ae78f84..1f4cf55 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -1208,13 +1208,19 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain, if (ret < 0) return(-1); - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[0].field, str_weight) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Weight %s too big for destination", str_weight); + return -1; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = op_dom.u.getschedinfo.u.credit.weight; - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[1].field, str_cap) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Cap %s too big for destination", str_cap); + return -1; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = op_dom.u.getschedinfo.u.credit.cap; @@ -2431,9 +2437,11 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, while (fgets (line, sizeof line, cpuinfo)) { if (regexec (&flags_hvm_rec, line, sizeof(subs)/sizeof(regmatch_t), subs, 0) == 0 && subs[0].rm_so != -1) { - strncpy (hvm_type, - &line[subs[1].rm_so], subs[1].rm_eo-subs[1].rm_so+1); - hvm_type[subs[1].rm_eo-subs[1].rm_so] = '\0'; + if (virStrncpy(hvm_type, + &line[subs[1].rm_so], + subs[1].rm_eo-subs[1].rm_so, + sizeof(hvm_type)) == NULL) + return NULL; } else if (regexec (&flags_pae_rec, line, 0, NULL, 0) == 0) host_pae = 1; } diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..d39fd4d 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -763,7 +763,8 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) addr = (struct sockaddr_un *)&priv->addr; addr->sun_family = AF_UNIX; memset(addr->sun_path, 0, sizeof(addr->sun_path)); - strncpy(addr->sun_path, path, sizeof(addr->sun_path)); + if (virStrcpyStatic(addr->sun_path, path) == NULL) + return -1; return (0); } @@ -1433,7 +1434,7 @@ xenDaemonParseSxprChar(virConnectPtr conn, const char *value, const char *tty) { - char prefix[10]; + const char *prefix; char *tmp; virDomainChrDefPtr def; @@ -1442,18 +1443,17 @@ xenDaemonParseSxprChar(virConnectPtr conn, return NULL; } - strncpy(prefix, value, sizeof(prefix)-1); - NUL_TERMINATE(prefix); + prefix = value; if (value[0] == '/') { def->type = VIR_DOMAIN_CHR_TYPE_DEV; } else { - if ((tmp = strchr(prefix, ':')) != NULL) { + if ((tmp = strchr(value, ':')) != NULL) { *tmp = '\0'; - value += (tmp - prefix) + 1; + value = tmp + 1; } - if (STREQ(prefix, "telnet")) { + if (STRPREFIX(prefix, "telnet")) { def->type = VIR_DOMAIN_CHR_TYPE_TCP; def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; } else { @@ -1647,8 +1647,13 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverName, (offset-src)+1) < 0) goto no_memory; - strncpy(disk->driverName, src, (offset-src)); - disk->driverName[offset-src] = '\0'; + if (virStrncpy(disk->driverName, src, offset-src, + (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + src); + goto error; + } src = offset + 1; @@ -1662,8 +1667,13 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverType, (offset-src)+1)< 0) goto no_memory; - strncpy(disk->driverType, src, (offset-src)); - disk->driverType[offset-src] = '\0'; + if (virStrncpy(disk->driverType, src, offset-src, + (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + src); + goto error; + } src = offset + 1; /* Its possible to use blktap driver for block devs @@ -1891,13 +1901,12 @@ xenDaemonParseSxprSound(virConnectPtr conn, len = (offset2 - offset); else len = strlen(offset); - if (len > (sizeof(model)-1)) { + if (virStrncpy(model, offset, len, sizeof(model)) == NULL) { virXendError(conn, VIR_ERR_INTERNAL_ERROR, - _("unexpected sound model %s"), offset); + _("Sound model %s too big for destination"), + offset); goto error; } - strncpy(model, offset, len); - model[len] = '\0'; if (VIR_ALLOC(sound) < 0) goto no_memory; @@ -4786,13 +4795,20 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain, goto error; } - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[0].field, str_weight) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Weight %s too big for destination"), + str_weight); + goto error; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = sexpr_int(root, "domain/cpu_weight"); - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[1].field, str_cap) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Cap %s too big for destination"), str_cap); + goto error; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = sexpr_int(root, "domain/cpu_cap"); *nparams = XEN_SCHED_CRED_NPARAM; @@ -5842,6 +5858,7 @@ virDomainXMLDevID(virDomainPtr domain, { xenUnifiedPrivatePtr priv = domain->conn->privateData; char *xref; + char *tmp; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { if (dev->data.disk->driverName && @@ -5859,9 +5876,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { char mac[30]; virDomainNetDefPtr def = dev->data.net; @@ -5878,9 +5896,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..83eebee 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -872,8 +872,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { } else { if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->src, head, (offset - head)); - disk->src[(offset-head)] = '\0'; + if (virStrncpy(disk->src, head, offset - head, + (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Source file %s too big for destination"), + head); + goto cleanup; + } } head = offset + 1; @@ -886,8 +891,12 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->dst, head, (offset - head)); - disk->dst[(offset-head)] = '\0'; + if (virStrncpy(disk->dst, head, offset - head, + (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Dest file %s too big for destination"), head); + goto cleanup; + } head = offset + 1; @@ -897,8 +906,14 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if ((tmp = strchr(disk->src, ':')) != NULL) { if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverName, disk->src, (tmp - disk->src)); - disk->driverName[tmp - disk->src] = '\0'; + if (virStrncpy(disk->driverName, disk->src, + (tmp - disk->src), + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -912,8 +927,14 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->driverType, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverType, disk->src, (tmp - disk->src)); - disk->driverType[tmp - disk->src] = '\0'; + if (virStrncpy(disk->driverType, disk->src, + (tmp - disk->src), + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -1026,42 +1047,52 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { data++; if (STRPREFIX(key, "mac=")) { - int len = nextkey ? (nextkey - data) : 17; - if (len > 17) - len = 17; - strncpy(mac, data, len); - mac[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(mac) - 1; + if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "bridge=")) { - int len = nextkey ? (nextkey - data) : sizeof(bridge)-1; + int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1; type = 1; - if (len > (sizeof(bridge)-1)) - len = sizeof(bridge)-1; - strncpy(bridge, data, len); - bridge[len] = '\0'; + if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "script=")) { - int len = nextkey ? (nextkey - data) : PATH_MAX-1; - if (len > (PATH_MAX-1)) - len = PATH_MAX-1; - strncpy(script, data, len); - script[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(script) - 1; + if (virStrncpy(script, data, len, sizeof(script)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Script %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "model=")) { - int len = nextkey ? (nextkey - data) : sizeof(model)-1; - if (len > (sizeof(model)-1)) - len = sizeof(model)-1; - strncpy(model, data, len); - model[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(model) - 1; + if (virStrncpy(model, data, len, sizeof(model)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), data); + goto skipnic; + } } else if (STRPREFIX(key, "vifname=")) { - int len = nextkey ? (nextkey - data) : sizeof(vifname)-1; - if (len > (sizeof(vifname)-1)) - len = sizeof(vifname)-1; - strncpy(vifname, data, len); - vifname[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1; + if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Vifname %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "ip=")) { - int len = nextkey ? (nextkey - data) : 15; - if (len > 15) - len = 15; - strncpy(ip, data, len); - ip[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(ip) - 1; + if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("IP %s too big for destination"), data); + goto skipnic; + } } while (nextkey && (nextkey[0] == ',' || @@ -1165,32 +1196,41 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if (!(nextkey = strchr(key, ':'))) goto skippci; - if ((nextkey - key) > (sizeof(domain)-1)) + if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Domain %s too big for destination"), key); goto skippci; - - strncpy(domain, key, sizeof(domain)); - domain[sizeof(domain)-1] = '\0'; + } key = nextkey + 1; if (!(nextkey = strchr(key, ':'))) goto skippci; - strncpy(bus, key, sizeof(bus)); - bus[sizeof(bus)-1] = '\0'; + if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bus %s too big for destination"), key); + goto skippci; + } key = nextkey + 1; if (!(nextkey = strchr(key, '.'))) goto skippci; - strncpy(slot, key, sizeof(slot)); - slot[sizeof(slot)-1] = '\0'; + if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Slot %s too big for destination"), key); + goto skippci; + } key = nextkey + 1; if (strlen(key) != 1) goto skippci; - strncpy(func, key, sizeof(func)); - func[sizeof(func)-1] = '\0'; + if (virStrncpy(func, key, 1, sizeof(func)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Function %s too big for destination"), key); + goto skippci; + } if (virStrToLong_i(domain, NULL, 16, &domainID) < 0) goto skippci; @@ -1301,8 +1341,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { list->list->str) { char vfb[MAX_VFB]; char *key = vfb; - strncpy(vfb, list->list->str, MAX_VFB-1); - vfb[MAX_VFB-1] = '\0'; + + if (virStrcpyStatic(vfb, list->list->str) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("VFB %s too big for destination"), + list->list->str); + goto cleanup; + } if (VIR_ALLOC(graphics) < 0) goto no_memory; -- 1.6.0.6

3 3

[libvirt] [PATCH] Compressed save image format for Qemu.
by Chris Lalancette 28 Aug '09

28 Aug '09

Implement a compressed save image format for qemu. While ideally we would have the choice between compressed/non-compressed available to the libvirt API, unfortunately there is no "flags" parameter to the virDomainSave() API. Therefore, implement this as a qemu.conf option. Both gzip and bzip2 are implemented, and it should be very easy to implement additional compression methods. One open question is if/how we should detect the gzip and bzip2 binaries. One way to do it is to do compile-time setting of the paths (via configure.in), but that doesn't seem like a great thing to do. Another solution (my preferred solution) is not to detect at all; when we go to run the commands that need them, if they aren't available, or aren't available in one of the standard paths, then we'll fail. Maybe somebody else has another option or opinion, though. In the future, we'll have a more robust (managed) save/restore API, at which time we can expose this functionality properly in the API. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu.conf | 10 +++++++ src/qemu_conf.c | 11 ++++++++ src/qemu_conf.h | 2 + src/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/qemu.conf b/src/qemu.conf index 653f487..86dcc9d 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -129,3 +129,13 @@ # "/dev/ptmx", "/dev/kvm", "/dev/kqemu", # "/dev/rtc", "/dev/hpet", "/dev/net/tun", #] + +# The default format for Qemu/KVM guest save images is raw; that is, the +# memory from the domain is dumped out directly to a file. If you have +# guests with a large amount of memory, however, this can take up quite +# a bit of space. If you would like to compress the images while they +# are being saved to disk, you can also set "gzip" or "bzip2" for the +# save_image_format. Note that this means you slow down the process +# of saving a domain in order to save disk space. +# +# save_image_format = "raw" diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 7ca5a15..ed87e13 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -280,6 +280,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, driver->cgroupDeviceACL[i] = NULL; } + p = virConfGetValue (conf, "save_image_format"); + CHECK_TYPE ("save_image_format", VIR_CONF_STRING); + if (p && p->str) { + VIR_FREE(driver->saveImageFormat); + if (!(driver->saveImageFormat = strdup(p->str))) { + virReportOOMError(NULL); + virConfFree(conf); + return -1; + } + } + virConfFree (conf); return 0; } diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 8f4ef6a..e34baab 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -111,6 +111,8 @@ struct qemud_driver { char *securityDriverName; virSecurityDriverPtr securityDriver; + + char *saveImageFormat; }; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 3c92635..f642fea 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3437,11 +3437,26 @@ static int qemudDomainSave(virDomainPtr dom, struct qemud_save_header header; int ret = -1; virDomainEventPtr event = NULL; + int internalret; memset(&header, 0, sizeof(header)); memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)); header.version = QEMUD_SAVE_VERSION; + if (driver->saveImageFormat == NULL) + header.compressed = QEMUD_SAVE_FORMAT_RAW; + else if (STREQ(driver->saveImageFormat, "raw")) + header.compressed = QEMUD_SAVE_FORMAT_RAW; + else if (STREQ(driver->saveImageFormat, "gzip")) + header.compressed = QEMUD_SAVE_FORMAT_GZIP; + else if (STREQ(driver->saveImageFormat, "bzip2")) + header.compressed = QEMUD_SAVE_FORMAT_BZIP2; + else { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("Invalid save image format specified in configuration file")); + return -1; + } + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -3514,11 +3529,27 @@ static int qemudDomainSave(virDomainPtr dom, virReportOOMError(dom->conn); goto cleanup; } - if (virAsprintf(&command, "migrate \"exec:" - "dd of='%s' oflag=append conv=notrunc 2>/dev/null" - "\"", safe_path) == -1) { + + if (header.compressed == QEMUD_SAVE_FORMAT_RAW) + internalret = virAsprintf(&command, "migrate \"exec:" + "dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else if (header.compressed == QEMUD_SAVE_FORMAT_GZIP) + internalret = virAsprintf(&command, "migrate \"exec:" + "gzip -c | dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2) + internalret = virAsprintf(&command, "migrate \"exec:" + "bzip2 -c | dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + _("Invalid compress format %d"), + header.compressed); + goto cleanup; + } + if (internalret < 0) { virReportOOMError(dom->conn); - command = NULL; goto cleanup; } @@ -4039,6 +4070,9 @@ static int qemudDomainRestore(virConnectPtr conn, char *xml = NULL; struct qemud_save_header header; virDomainEventPtr event = NULL; + int intermediatefd = -1; + pid_t intermediate_pid = -1; + int childstat; qemuDriverLock(driver); /* Verify the header and read the XML */ @@ -4128,8 +4162,39 @@ static int qemudDomainRestore(virConnectPtr conn, } def = NULL; + if (header.version == 2) { + const char *intermediate_argv[3] = { NULL, "-dc", NULL }; + if (header.compressed == QEMUD_SAVE_FORMAT_GZIP) + intermediate_argv[0] = "gzip"; + else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2) + intermediate_argv[0] = "bzip2"; + else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("Unknown compressed save format %d"), + header.compressed); + goto cleanup; + } + if (intermediate_argv[0] != NULL) { + intermediatefd = fd; + fd = -1; + if (virExec(conn, intermediate_argv, NULL, NULL, + &intermediate_pid, intermediatefd, &fd, NULL, 0) < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to start decompression binary %s"), + intermediate_argv[0]); + goto cleanup; + } + } + } /* Set the migration source and start it up. */ ret = qemudStartVMDaemon(conn, driver, vm, "stdio", fd); + if (intermediate_pid != -1) { + /* Wait for intermediate process to exit */ + while (waitpid(intermediate_pid, &childstat, 0) == -1 && + errno == EINTR); + } + if (intermediatefd != -1) + close(intermediatefd); close(fd); fd = -1; if (ret < 0) { -- 1.6.0.6

3 3

[libvirt] perl bindings Sys Virt compilation problems
by Daniel de Araujo 28 Aug '09

28 Aug '09

Hi, I'm trying to compile the libvirt perl bindings (Sys-Virt-0.2.1) on x86_64 RHEL 5.3 and I'm running into the following issue: /usr/bin/perl "-Iblib/arch" "-Iblib/lib" perl-Sys-Virt.spec.PL<http://perl-sys-virt.spec.pl/>perl-Sys-Virt.spec Can't load 'blib/arch/auto/Sys/Virt/Virt.so' for module Sys::Virt: libvirt.so.0: cannot open shared object file: No such file or directory at /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/DynaLoader.pm line 230. at perl-Sys-Virt.spec.PL <http://perl-sys-virt.spec.pl/> line 9 make: *** [perl-Sys-Virt.spec] Error 9 I've specified the PKG_CONFIG_PATH as /usr/local/lib/pkgconfig; libvirt shared objects are in /usr/local/lib I'm using libvirt 0.6.5. Any help would be appreciated. -- -Dan

2 1

[libvirt] how to change the boot order of a qemu-kvm guest os
by Gopalakrishnan Subramanian 28 Aug '09

28 Aug '09

2 2

[libvirt] Schedule for next release 0.7.1
by Daniel Veillard 28 Aug '09

28 Aug '09

Hello, following a bit too much backlog on libxml2 for the last years and an happy familly event, I have been a bit sidetracked in the last weeks and will be in the near future. I know there is a tons of pending patches, updates for drives and cleanups waiting for a release. I will try to make sure everything is reviewed and pushed by end of next week, which would allow for a 0.7.1 release by Sept 11, I hope this won't generate too much disruption while the initial schedule was at the end of the month ! thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0

[libvirt] [PATCH] virsh vol-key segfault fix
by Pritesh Kothari 28 Aug '09

28 Aug '09

Hi, There is a bug in virsh command vol-key, it segfaults cause the fourth option to vshCommandOptVolBy() is null instead of "pool" on virsh.c:5209. The patch to fix this is attached. Regards, Pritesh

2 1