[libvirt] [PATCH v5 04/11] Helper functions for host TPM support
Daniel P. Berrange
berrange at redhat.com
Fri Apr 12 10:22:18 UTC 2013
On Thu, Apr 11, 2013 at 04:04:46PM -0400, Stefan Berger wrote:
> Implement helper functions to find the TPM's sysfs cancel file.
>
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> Reviewed-by: Corey Bryant <coreyb at linux.vnet.ibm.com>
> Tested-by: Corey Bryant <coreyb at linux.vnet.ibm.com>
>
> ---
> po/POTFILES.in | 1
> src/Makefile.am | 1
> src/libvirt_private.syms | 4 +
> src/util/virtpm.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virtpm.h | 27 +++++++++
> 5 files changed, 165 insertions(+)
>
> Index: libvirt/src/Makefile.am
> ===================================================================
> --- libvirt.orig/src/Makefile.am
> +++ libvirt/src/Makefile.am
> @@ -122,6 +122,7 @@ UTIL_SOURCES = \
> util/virthreadwin32.h \
> util/virthreadpool.c util/virthreadpool.h \
> util/virtime.h util/virtime.c \
> + util/virtpm.h util/virtpm.c \
> util/virtypedparam.c util/virtypedparam.h \
> util/virusb.c util/virusb.h \
> util/viruri.h util/viruri.c \
> Index: libvirt/src/util/virtpm.c
> ===================================================================
> --- /dev/null
> +++ libvirt/src/util/virtpm.c
> @@ -0,0 +1,132 @@
> +/*
> + * virtpm.c: TPM support
> + *
> + * Copyright (C) 2013 IBM Corporation
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Stefan Berger <stefanb at linux.vnet.ibm.com>
> + */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <dirent.h>
> +#include <unistd.h>
> +#include <sys/stat.h>
> +
> +#include "c-ctype.h"
> +#include "virobject.h"
> +#include "viralloc.h"
> +#include "virutil.h"
> +#include "virerror.h"
> +#include "virbuffer.h"
> +#include "virtpm.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +/*
> + * Check whether the given base path, e.g., /sys/class/misc/tpm0/device,
> + * is the sysfs entry of a TPM. A TPM sysfs entry should be uniquely
> + * recognizable by the file entries 'pcrs' and 'cancel'.
> + * Upon success the basepath with '/cancel' appended is returned, NULL
> + * otherwise.
> + */
> +static char *
> +virTPMCheckSysfsCancel(char *basepath)
> +{
> + char *path = NULL;
> + struct stat statbuf;
> +
> + if (virAsprintf(&path, "%s/pcrs", basepath) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not determine TPM cancel path since pcrs "
> + "file was not found in directory %s"),
> + basepath);
> + goto cleanup;
> + }
> +
> + VIR_FREE(path);
> +
> + if (virAsprintf(&path, "%s/cancel", basepath) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not determine TPM cancel path since cancel "
> + "file was not found in directory %s"),
> + basepath);
> + goto cleanup;
> + }
> +
> + return path;
> +
> +cleanup:
> + VIR_FREE(path);
> + return NULL;
> +}
> +
> +
> +char *
> +virTPMFindCancelPath(void)
> +{
> + unsigned int idx;
> + DIR *pnp_dir;
> + char *path = NULL, *basepath = NULL;
> + struct dirent *entry;
> +
> + pnp_dir = opendir("/sys/class/misc");
> + if (pnp_dir != NULL) {
> + while ((entry = readdir(pnp_dir)) != NULL) {
> + /* expecting 'tpm%u' -- skip other file patterns */
> + if (!STRPREFIX(entry->d_name, "tpm") ||
> + virStrToLong_ui(entry->d_name + strlen("tpm"),
> + NULL, 10, &idx) < 0)
> + continue;
> + if (virAsprintf(&basepath, "/sys/class/misc/%s/device",
> + entry->d_name) < 0) {
> + virReportOOMError();
> + break;
> + }
> + if ((path = virTPMCheckSysfsCancel(basepath)))
> + break;
> +
You're still silently ignoring errors from virTPMCheckSysfsCancel
here. As I said before, either you need to break out of the loop
and return an error, or you need to call virResetLastError().
With this code, if the loop executes multiple times, you're going
to be overwriting errors.
> + VIR_FREE(basepath);
> + }
> + closedir(pnp_dir);
> + } else {
> + virReportSystemError(errno, "%s",
> + _("Could not open directory /sys/class/misc "
> + "to find TPM cancel path"));
> + }
> +
> + VIR_FREE(basepath);
> +
> + /* report error if no path was found & no error was reported earlier */
> + if (!path && !virGetLastError())
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> + _("Could not determine TPM cancel path. You may need "
> + "to insert the TPM driver module into the kernel"));
All this error reporting code is really badly structured, and you're not
handling errors from readdir() as I described previously. Can you just
use the normal "goto error" / "goto cleanup" approach we have everywhere
ie like this
char *
virTPMFindCancelPath(void)
{
unsigned int idx;
DIR *pnp_dir;
char *path = NULL, *basepath = NULL;
struct dirent *entry;
if (!(pnp_dir = opendir("/sys/class/misc"))) {
virReportSystemError(errno, "%s",
_("Could not open directory /sys/class/misc"));
goto cleanup;
}
errno = 0;
while ((entry = readdir(pnp_dir)) != NULL) {
/* expecting 'tpm%u' -- skip other file patterns */
if (!STRPREFIX(entry->d_name, "tpm") ||
virStrToLong_ui(entry->d_name + strlen("tpm"),
NULL, 10, &idx) < 0)
continue;
if (virAsprintf(&basepath, "/sys/class/misc/%s/device",
entry->d_name) < 0) {
virReportOOMError();
break;
}
path = virTPMCheckSysfsCancel(basepath);
VIR_FREE(basepath);
if (!path)
virResetLastError();
else
break;
errno = 0;
}
if (errno != 0) {
virReportSystemError(errno, "%s",
_("Could not read directory /sys/class/misc"));
goto cleanup;
}
/* report error if no path was found & no error was reported earlier */
if (!path) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s",
_("Could not determine TPM cancel path. You may need "
"to insert the TPM driver module into the kernel"));
goto cleanup;
}
cleanup:
closedir(pnp_dir);
return path;
}
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list