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@xxxxxxxxxxxxxxxxxx> > Reviewed-by: Corey Bryant <coreyb@xxxxxxxxxxxxxxxxxx> > Tested-by: Corey Bryant <coreyb@xxxxxxxxxxxxxxxxxx> > > --- > 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@xxxxxxxxxxxxxxxxxx> > + */ > + > +#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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list