On 05/13/2016 12:04 PM, Peter Krempa wrote:
> Add a new helper that sanitizes error semantics of base64_encode_alloc.
> ---
> src/conf/virsecretobj.c | 7 ++-----
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_agent.c | 6 ++----
> src/storage/storage_backend_rbd.c | 17 ++++-------------
> src/util/virstring.c | 24 ++++++++++++++++++++++++
> src/util/virstring.h | 2 ++
> tools/virsh-secret.c | 6 +++---
> 7 files changed, 38 insertions(+), 25 deletions(-)
>
probably means #include "base64.h" is no longer necessary in places
where the call has been removed.
> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
> index 721e0b4..4babd31 100644
> --- a/src/conf/virsecretobj.c
> +++ b/src/conf/virsecretobj.c
> @@ -30,6 +30,7 @@
> #include "virfile.h"
> #include "virhash.h"
> #include "virlog.h"
> +#include "virstring.h"
> #include "base64.h"
>
> #define VIR_FROM_THIS VIR_FROM_SECRET
> @@ -730,12 +731,8 @@ virSecretObjSaveData(virSecretObjPtr secret)
> if (!secret->value)
> return 0;
>
> - base64_encode_alloc((const char *)secret->value, secret->value_size,
> - &base64);
> - if (base64 == NULL) {
> - virReportOOMError();
> + if (!(base64 = virStringEncodeBase64(secret->value, secret->value_size)))
> goto cleanup;
> - }
>
> if (virFileRewrite(secret->base64File, S_IRUSR | S_IWUSR,
> virSecretRewriteFile, base64) < 0)
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9c1abbb..ca2b6b8 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2302,6 +2302,7 @@ virSkipSpacesBackwards;
> virStrcpy;
> virStrdup;
> virStringArrayHasString;
> +virStringEncodeBase64;
> virStringFreeList;
> virStringFreeListCount;
> virStringGetFirstWithPrefix;
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index c55f304..cbc0995 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -2142,11 +2142,9 @@ qemuAgentSetUserPassword(qemuAgentPtr mon,
> virJSONValuePtr reply = NULL;
> char *password64 = NULL;
>
> - base64_encode_alloc(password, strlen(password), &password64);
> - if (!password64) {
> - virReportOOMError();
> + if (!(password64 = virStringEncodeBase64((unsigned char *) password,
> + strlen(password))))
> goto cleanup;
> - }
>
> if (!(cmd = qemuAgentMakeCommand("guest-set-user-password",
> "b:crypted", crypted,
> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> index 6e92ff7..ac46b9d 100644
> --- a/src/storage/storage_backend_rbd.c
> +++ b/src/storage/storage_backend_rbd.c
> @@ -59,7 +59,7 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> int r = 0;
> virStorageAuthDefPtr authdef = source->auth;
> unsigned char *secret_value = NULL;
> - size_t secret_value_size;
> + size_t secret_value_size = 0;
> char *rados_key = NULL;
> virBuffer mon_host = VIR_BUFFER_INITIALIZER;
> virSecretPtr secret = NULL;
> @@ -129,15 +129,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> goto cleanup;
> }
>
> - base64_encode_alloc((char *)secret_value,
> - secret_value_size, &rados_key);
> - memset(secret_value, 0, secret_value_size);
> -
> - if (rados_key == NULL) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("failed to decode the RADOS key"));
> + if (!(rados_key = virStringEncodeBase64(secret_value, secret_value_size)))
> goto cleanup;
> - }
>
> VIR_DEBUG("Found cephx key: %s", rados_key);
> if (rados_conf_set(ptr->cluster, "key", rados_key) < 0) {
> @@ -147,8 +140,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> goto cleanup;
> }
>
> - memset(rados_key, 0, strlen(rados_key));
> -
> if (rados_conf_set(ptr->cluster, "auth_supported", "cephx") < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("failed to set RADOS option: %s"),
> @@ -233,8 +224,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> ret = 0;
>
> cleanup:
> - VIR_FREE(secret_value);
> - VIR_FREE(rados_key);
> + VIR_DISPOSE_N(secret_value, secret_value_size);
> + VIR_DISPOSE_STRING(rados_key);
>
> virObjectUnref(secret);
>
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index 735e65b..2702cec 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -25,6 +25,7 @@
> #include <stdio.h>
> #include <regex.h>
>
> +#include "base64.h"
> #include "c-ctype.h"
> #include "virstring.h"
> #include "viralloc.h"
> @@ -1066,3 +1067,26 @@ virStringIsPrintable(const char *str)
>
> return true;
> }
> +
> +
> +/**
> + * virStringEncodeBase64:
> + * @buf: buffer of bytes to encode
> + * @buflen: number of bytes to encode
> + *
> + * Encodes @buf to base 64 and returns the resulting string. The caller is
> + * responsible for freeing the result.
> + */
> +char *
> +virStringEncodeBase64(const uint8_t *buf, size_t buflen)
> +{
> + char *ret;
> +
> + base64_encode_alloc((const char *) buf, buflen, &ret);
> + if (!ret) {
> + virReportOOMError();
> + return NULL;
> + }
Don't think the return NULL would be necessary... just return ret
> +
> + return ret;
> +}
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index fd2868a..6bc2726 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -277,4 +277,6 @@ void virStringStripControlChars(char *str);
>
> bool virStringIsPrintable(const char *str);
>
> +char *virStringEncodeBase64(const uint8_t *buf, size_t buflen);
> +
> #endif /* __VIR_STRING_H__ */
> diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
> index 087c2ed..532c4d5 100644
> --- a/tools/virsh-secret.c
> +++ b/tools/virsh-secret.c
> @@ -32,6 +32,7 @@
> #include "viralloc.h"
> #include "virfile.h"
> #include "virutil.h"
> +#include "virstring.h"
> #include "conf/secret_conf.h"
>
> static virSecretPtr
> @@ -265,9 +266,8 @@ cmdSecretGetValue(vshControl *ctl, const vshCmd *cmd)
> if (value == NULL)
> goto cleanup;
>
> - base64_encode_alloc((char *)value, value_size, &base64);
> - memset(value, 0, value_size);
> - VIR_FREE(value);
Lost the VIR_FREE(value).... Could be VIR_DISPOSE(value) too.
John
> + if (!(base64 = virStringEncodeBase64(value, value_size)))
> + goto cleanup;
>
> if (base64 == NULL) {
> vshError(ctl, "%s", _("Failed to allocate memory"));
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list