Patchwork [V13,7/7] Add fd parameter for TPM passthrough driver

login
register
mail settings
Submitter Stefan Berger
Date Dec. 12, 2011, 7:12 p.m.
Message ID <1323717136-21661-8-git-send-email-stefanb@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/130943/
State New
Headers show

Comments

Stefan Berger - Dec. 12, 2011, 7:12 p.m.
Enable the passing of a file descriptor via fd=<..> to access the host's
TPM device using the TPM passthrough driver.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

---

v13:
 - Only accepting a character device's file descriptor

v12:
 - added documentation part
---
 hw/tpm_passthrough.c |   73 +++++++++++++++++++++++++++++++++++++-------------
 qemu-config.c        |    5 +++
 qemu-options.hx      |    6 +++-
 3 files changed, 64 insertions(+), 20 deletions(-)
Anthony Liguori - Dec. 12, 2011, 11:30 p.m.
On 12/12/2011 01:12 PM, Stefan Berger wrote:
> Enable the passing of a file descriptor via fd=<..>  to access the host's
> TPM device using the TPM passthrough driver.
>
> Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
>
> ---
>
> v13:
>   - Only accepting a character device's file descriptor
>
> v12:
>   - added documentation part
> ---
>   hw/tpm_passthrough.c |   73 +++++++++++++++++++++++++++++++++++++-------------
>   qemu-config.c        |    5 +++
>   qemu-options.hx      |    6 +++-
>   3 files changed, 64 insertions(+), 20 deletions(-)
>
> diff --git a/hw/tpm_passthrough.c b/hw/tpm_passthrough.c
> index f9cfe3d..57bb77a 100644
> --- a/hw/tpm_passthrough.c
> +++ b/hw/tpm_passthrough.c
> @@ -361,33 +361,68 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
>       const char *value;
>       char buf[64];
>       int n;
> +    struct stat statbuf;
>
> -    value = qemu_opt_get(opts, "path");
> -    if (!value) {
> -        value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
> -    }
> +    value = qemu_opt_get(opts, "fd");
> +    if (value) {
> +        if (qemu_opt_get(opts, "path")) {
> +            error_report("fd= is invalid with path=");
> +            return -1;
> +        }
> +
> +        tb->s.tpm_pt->tpm_fd = qemu_parse_fd(value);
> +        if (tb->s.tpm_pt->tpm_fd<  0) {
> +            error_report("Illegal file descriptor for TPM device.\n");
> +            return -1;
> +        }
>
> -    n = snprintf(tb->s.tpm_pt->tpm_dev, sizeof(tb->s.tpm_pt->tpm_dev),
> -                 "%s", value);
> +        snprintf(buf, sizeof(buf), "fd=%d", tb->s.tpm_pt->tpm_fd);
>
> -    if (n>= sizeof(tb->s.tpm_pt->tpm_dev)) {
> -        error_report("TPM device path is too long.\n");
> -        goto err_exit;
> -    }
> +        tb->parameters = g_strdup(buf);
>
> -    snprintf(buf, sizeof(buf), "path=%s", tb->s.tpm_pt->tpm_dev);
> +        if (tb->parameters == NULL) {
> +            goto err_close_tpmdev;
> +        }
> +    } else {
> +        value = qemu_opt_get(opts, "path");
> +        if (!value) {
> +            value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
> +        }
> +
> +        n = snprintf(tb->s.tpm_pt->tpm_dev, sizeof(tb->s.tpm_pt->tpm_dev),
> +                     "%s", value);
> +
> +        if (n>= sizeof(tb->s.tpm_pt->tpm_dev)) {
> +            error_report("TPM device path is too long.\n");
> +            goto err_exit;
> +        }
>
> -    tb->parameters = g_strdup(buf);
> +        snprintf(buf, sizeof(buf), "path=%s", tb->s.tpm_pt->tpm_dev);
>
> -    if (tb->parameters == NULL) {
> -        return 1;
> +        tb->parameters = g_strdup(buf);
> +
> +        if (tb->parameters == NULL) {
> +            return 1;
> +        }
> +
> +        tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR);
> +        if (tb->s.tpm_pt->tpm_fd<  0) {
> +            error_report("Cannot access TPM device using '%s'.\n",
> +                         tb->s.tpm_pt->tpm_dev);
> +            goto err_exit;
> +        }
>       }
>
> -    tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR);
> -    if (tb->s.tpm_pt->tpm_fd<  0) {
> -        error_report("Cannot access TPM device using '%s'.\n",
> -                     tb->s.tpm_pt->tpm_dev);
> -        goto err_exit;
> +    if (fstat(tb->s.tpm_pt->tpm_fd,&statbuf) != 0) {
> +        error_report("Cannot determine file descriptor type for TPM "
> +                     "device: %s", strerror(errno));
> +        goto err_close_tpmdev;
> +    }
> +
> +    /* only allow character devices for now */
> +    if (!S_ISCHR(statbuf.st_mode)) {
> +        error_report("TPM file descriptor is not a character device");
> +        goto err_close_tpmdev;
>       }

I think you're being overzealous here.  The backend only uses read/write to 
interact with the passthrough device.  You could use this as a mechanism to tie 
in an emulated VTPM by using a socket.  I'm not suggesting we do that for 
libvtpm, but I think we don't gain anything from being overly restrictive here.

I don't think a user passing the wrong type of fd is the common case to optimize 
for wrt usability.

Regards,

Anthony Liguori
Stefan Berger - Dec. 13, 2011, 12:17 a.m.
On 12/12/2011 06:30 PM, Anthony Liguori wrote:
> On 12/12/2011 01:12 PM, Stefan Berger wrote:
>> Enable the passing of a file descriptor via fd=<..>  to access the 
>> host's
>> TPM device using the TPM passthrough driver.
>>
>> Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
>>
[...]
>> -    tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR);
>> -    if (tb->s.tpm_pt->tpm_fd<  0) {
>> -        error_report("Cannot access TPM device using '%s'.\n",
>> -                     tb->s.tpm_pt->tpm_dev);
>> -        goto err_exit;
>> +    if (fstat(tb->s.tpm_pt->tpm_fd,&statbuf) != 0) {
>> +        error_report("Cannot determine file descriptor type for TPM "
>> +                     "device: %s", strerror(errno));
>> +        goto err_close_tpmdev;
>> +    }
>> +
>> +    /* only allow character devices for now */
>> +    if (!S_ISCHR(statbuf.st_mode)) {
>> +        error_report("TPM file descriptor is not a character device");
>> +        goto err_close_tpmdev;
>>       }
>
> I think you're being overzealous here.  The backend only uses 
> read/write to interact with the passthrough device.  You could use 
> this as a mechanism to tie in an emulated VTPM by using a socket.  I'm 
> not suggesting we do that for libvtpm, but I think we don't gain 
> anything from being overly restrictive here.

We prevent files, pipes, sockets and block devices using this check. 
Sockets may make sense in the future, but would like to enable that 
separately.

>
> I don't think a user passing the wrong type of fd is the common case 
> to optimize for wrt usability.

I don't think it makes sense to have the TPM passthrough driver write() 
into a block device or file, so therefore I prevented that. The above 
check is just a single line...

    Stefan

Patch

diff --git a/hw/tpm_passthrough.c b/hw/tpm_passthrough.c
index f9cfe3d..57bb77a 100644
--- a/hw/tpm_passthrough.c
+++ b/hw/tpm_passthrough.c
@@ -361,33 +361,68 @@  static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
     const char *value;
     char buf[64];
     int n;
+    struct stat statbuf;
 
-    value = qemu_opt_get(opts, "path");
-    if (!value) {
-        value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
-    }
+    value = qemu_opt_get(opts, "fd");
+    if (value) {
+        if (qemu_opt_get(opts, "path")) {
+            error_report("fd= is invalid with path=");
+            return -1;
+        }
+
+        tb->s.tpm_pt->tpm_fd = qemu_parse_fd(value);
+        if (tb->s.tpm_pt->tpm_fd < 0) {
+            error_report("Illegal file descriptor for TPM device.\n");
+            return -1;
+        }
 
-    n = snprintf(tb->s.tpm_pt->tpm_dev, sizeof(tb->s.tpm_pt->tpm_dev),
-                 "%s", value);
+        snprintf(buf, sizeof(buf), "fd=%d", tb->s.tpm_pt->tpm_fd);
 
-    if (n >= sizeof(tb->s.tpm_pt->tpm_dev)) {
-        error_report("TPM device path is too long.\n");
-        goto err_exit;
-    }
+        tb->parameters = g_strdup(buf);
 
-    snprintf(buf, sizeof(buf), "path=%s", tb->s.tpm_pt->tpm_dev);
+        if (tb->parameters == NULL) {
+            goto err_close_tpmdev;
+        }
+    } else {
+        value = qemu_opt_get(opts, "path");
+        if (!value) {
+            value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
+        }
+
+        n = snprintf(tb->s.tpm_pt->tpm_dev, sizeof(tb->s.tpm_pt->tpm_dev),
+                     "%s", value);
+
+        if (n >= sizeof(tb->s.tpm_pt->tpm_dev)) {
+            error_report("TPM device path is too long.\n");
+            goto err_exit;
+        }
 
-    tb->parameters = g_strdup(buf);
+        snprintf(buf, sizeof(buf), "path=%s", tb->s.tpm_pt->tpm_dev);
 
-    if (tb->parameters == NULL) {
-        return 1;
+        tb->parameters = g_strdup(buf);
+
+        if (tb->parameters == NULL) {
+            return 1;
+        }
+
+        tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR);
+        if (tb->s.tpm_pt->tpm_fd < 0) {
+            error_report("Cannot access TPM device using '%s'.\n",
+                         tb->s.tpm_pt->tpm_dev);
+            goto err_exit;
+        }
     }
 
-    tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR);
-    if (tb->s.tpm_pt->tpm_fd < 0) {
-        error_report("Cannot access TPM device using '%s'.\n",
-                     tb->s.tpm_pt->tpm_dev);
-        goto err_exit;
+    if (fstat(tb->s.tpm_pt->tpm_fd, &statbuf) != 0) {
+        error_report("Cannot determine file descriptor type for TPM "
+                     "device: %s", strerror(errno));
+        goto err_close_tpmdev;
+    }
+
+    /* only allow character devices for now */
+    if (!S_ISCHR(statbuf.st_mode)) {
+        error_report("TPM file descriptor is not a character device");
+        goto err_close_tpmdev;
     }
 
     if (tpm_passthrough_test_tpmdev(tb->s.tpm_pt->tpm_fd)) {
diff --git a/qemu-config.c b/qemu-config.c
index cc4c31d..e49f4d8 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -564,6 +564,11 @@  static QemuOptsList qemu_tpmdev_opts = {
             .type = QEMU_OPT_STRING,
             .help = "Persistent storage for TPM state",
         },
+        {
+            .name = "fd",
+            .type = QEMU_OPT_STRING,
+            .help = "Filedescriptor for accessing the TPM",
+        },
         { /* end of list */ }
     },
 };
diff --git a/qemu-options.hx b/qemu-options.hx
index 4916baf..7eca701 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1932,7 +1932,7 @@  Use ? to print all available TPM backend types.
 qemu -tpmdev ?
 @end example
 
-@item -tpmdev passthrough, id=@var{id}, path=@var{path}
+@item -tpmdev passthrough, id=@var{id}, path=@var{path}, fd=@var{h}
 
 (Linux-host only) Enable access to the host's TPM using the passthrough
 driver.
@@ -1941,6 +1941,10 @@  driver.
 a Linux host this would be @code{/dev/tpm0}.
 @option{path} is optional and by default @code{/dev/tpm0} is used.
 
+@option{fd} specifies the file descriptor of the host's TPM device.
+@option{fd} and @option{path} are mutually exclusive.
+@option{fd} is optional.
+
 Some notes about using the host's TPM with the passthrough driver:
 
 The TPM device accessed by the passthrough driver must not be