[libvirt] [PATCH v5 09/20] tpm: Check whether previously found executables were updated

Stefan Berger posted 20 patches 6 years, 7 months ago
There is a newer version of this series
[libvirt] [PATCH v5 09/20] tpm: Check whether previously found executables were updated
Posted by Stefan Berger 6 years, 7 months ago
Check whether previously found executables were updated and if
so look for them again. This helps to use updated features of
swtpm and its tools upon updating them.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 src/qemu/qemu_tpm.c |  1 +
 src/util/virtpm.c   | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 61b4f72320..2afa8db448 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -20,6 +20,7 @@
 
 #include <config.h>
 
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <fcntl.h>
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 0179b1e8be..e4735f9c4d 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -77,8 +77,13 @@ virTPMCreateCancelPath(const char *devpath)
  * executables for the swtpm; to be found on the host
  */
 static char *swtpm_path;
+static struct stat swtpm_stat;
+
 static char *swtpm_setup;
+static struct stat swtpm_setup_stat;
+
 static char *swtpm_ioctl;
+static struct stat swtpm_ioctl_stat;
 
 const char *
 virTPMGetSwtpm(void)
@@ -116,18 +121,22 @@ virTPMEmulatorInit(void)
     static const struct {
         const char *name;
         char **path;
+        struct stat *stat;
     } prgs[] = {
         {
             .name = "swtpm",
             .path = &swtpm_path,
+            .stat = &swtpm_stat,
         },
         {
             .name = "swtpm_setup",
             .path = &swtpm_setup,
+            .stat = &swtpm_setup_stat,
         },
         {
             .name = "swtpm_ioctl",
             .path = &swtpm_ioctl,
+            .stat = &swtpm_ioctl_stat,
         }
     };
     size_t i;
@@ -135,6 +144,23 @@ virTPMEmulatorInit(void)
     for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) {
         char *path;
         bool findit = *prgs[i].path == NULL;
+        struct stat statbuf;
+        char *tmp;
+
+        if (!findit) {
+            /* has executables changed? */
+            if (stat(*prgs[i].path, &statbuf) < 0) {
+                virReportSystemError(errno,
+                                     _("Could not stat %s"), path);
+                findit = true;
+            }
+            if (!findit &&
+                memcmp(&statbuf.st_mtim,
+                       &prgs[i].stat->st_mtime,
+                       sizeof(statbuf.st_mtim))) {
+                findit = true;
+            }
+        }
 
         if (findit) {
             path = virFindFileInPath(prgs[i].name);
@@ -151,7 +177,15 @@ virTPMEmulatorInit(void)
                 VIR_FREE(path);
                 return -1;
             }
+            if (stat(path, prgs[i].stat) < 0) {
+                virReportSystemError(errno,
+                                     _("Could not stat %s"), path);
+                VIR_FREE(path);
+                return -1;
+            }
+            tmp = *prgs[i].path;
             *prgs[i].path = path;
+            VIR_FREE(tmp);
         }
     }
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 09/20] tpm: Check whether previously found executables were updated
Posted by Daniel P. Berrangé 6 years, 6 months ago
On Fri, Jul 12, 2019 at 12:23:43PM -0400, Stefan Berger wrote:
> Check whether previously found executables were updated and if
> so look for them again. This helps to use updated features of
> swtpm and its tools upon updating them.

Hmm, so this answers some of my earlier questions. If we actually
intentionally want the initializer to run many times, then we
definitely need some explicit locking in here, rather than jutt
assuming the QEMU driver has correctly serialized invokation.

> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  src/qemu/qemu_tpm.c |  1 +
>  src/util/virtpm.c   | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)



> 
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 61b4f72320..2afa8db448 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -20,6 +20,7 @@
>  
>  #include <config.h>
>  
> +#include <sys/stat.h>
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> diff --git a/src/util/virtpm.c b/src/util/virtpm.c
> index 0179b1e8be..e4735f9c4d 100644
> --- a/src/util/virtpm.c
> +++ b/src/util/virtpm.c
> @@ -77,8 +77,13 @@ virTPMCreateCancelPath(const char *devpath)
>   * executables for the swtpm; to be found on the host
>   */
>  static char *swtpm_path;
> +static struct stat swtpm_stat;
> +
>  static char *swtpm_setup;
> +static struct stat swtpm_setup_stat;
> +
>  static char *swtpm_ioctl;
> +static struct stat swtpm_ioctl_stat;
>  
>  const char *
>  virTPMGetSwtpm(void)
> @@ -116,18 +121,22 @@ virTPMEmulatorInit(void)
>      static const struct {
>          const char *name;
>          char **path;
> +        struct stat *stat;
>      } prgs[] = {
>          {
>              .name = "swtpm",
>              .path = &swtpm_path,
> +            .stat = &swtpm_stat,
>          },
>          {
>              .name = "swtpm_setup",
>              .path = &swtpm_setup,
> +            .stat = &swtpm_setup_stat,
>          },
>          {
>              .name = "swtpm_ioctl",
>              .path = &swtpm_ioctl,
> +            .stat = &swtpm_ioctl_stat,
>          }
>      };
>      size_t i;
> @@ -135,6 +144,23 @@ virTPMEmulatorInit(void)
>      for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) {
>          char *path;
>          bool findit = *prgs[i].path == NULL;
> +        struct stat statbuf;
> +        char *tmp;
> +
> +        if (!findit) {
> +            /* has executables changed? */
> +            if (stat(*prgs[i].path, &statbuf) < 0) {
> +                virReportSystemError(errno,
> +                                     _("Could not stat %s"), path);
> +                findit = true;

If we can't stat the binary, then I we should reset ourselves
back to fully uninitialized state. Certainly if we intend to
call virReportSystemError, then we must ensure the caller
treats it as a fatal error. Currently I believe its still
treated as success by the caller since progs[i].path is
already non-NULL at this point.

> +            }
> +            if (!findit &&
> +                memcmp(&statbuf.st_mtim,
> +                       &prgs[i].stat->st_mtime,
> +                       sizeof(statbuf.st_mtim))) {
> +                findit = true;
> +            }
> +        }
>  
>          if (findit) {
>              path = virFindFileInPath(prgs[i].name);
> @@ -151,7 +177,15 @@ virTPMEmulatorInit(void)
>                  VIR_FREE(path);
>                  return -1;
>              }
> +            if (stat(path, prgs[i].stat) < 0) {
> +                virReportSystemError(errno,
> +                                     _("Could not stat %s"), path);
> +                VIR_FREE(path);
> +                return -1;
> +            }
> +            tmp = *prgs[i].path;
>              *prgs[i].path = path;
> +            VIR_FREE(tmp);
>          }
>      }
>  
> -- 
> 2.20.1
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 09/20] tpm: Check whether previously found executables were updated
Posted by Stefan Berger 6 years, 6 months ago
On 7/24/19 1:02 PM, Daniel P. Berrangé wrote:
> On Fri, Jul 12, 2019 at 12:23:43PM -0400, Stefan Berger wrote:
>
>       for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) {
>           char *path;
>           bool findit = *prgs[i].path == NULL;
> +        struct stat statbuf;
> +        char *tmp;
> +
> +        if (!findit) {
> +            /* has executables changed? */
> +            if (stat(*prgs[i].path, &statbuf) < 0) {
> +                virReportSystemError(errno,
> +                                     _("Could not stat %s"), path);
> +                findit = true;
> If we can't stat the binary, then I we should reset ourselves
> back to fully uninitialized state. Certainly if we intend to
> call virReportSystemError, then we must ensure the caller
> treats it as a fatal error. Currently I believe its still
> treated as success by the caller since progs[i].path is
> already non-NULL at this point.


Removed the virReportError() here since we will try to find it again 
below and report errors there then.


>
>> +            }
>> +            if (!findit &&
>> +                memcmp(&statbuf.st_mtim,
>> +                       &prgs[i].stat->st_mtime,
>> +                       sizeof(statbuf.st_mtim))) {
>> +                findit = true;
>> +            }
>> +        }
>>   
>>           if (findit) {
>>               path = virFindFileInPath(prgs[i].name);
>> @@ -151,7 +177,15 @@ virTPMEmulatorInit(void)
>>                   VIR_FREE(path);
>>                   return -1;
>>               }
>> +            if (stat(path, prgs[i].stat) < 0) {
>> +                virReportSystemError(errno,
>> +                                     _("Could not stat %s"), path);
>> +                VIR_FREE(path);
>> +                return -1;
>> +            }
>> +            tmp = *prgs[i].path;
>>               *prgs[i].path = path;
>> +            VIR_FREE(tmp);
>>           }
>>       }
>>   
>> -- 
>> 2.20.1
>>
> Regards,
> Daniel


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list