Re: [Qemu-devel] [PATCH V13 7/7] Add fd parameter for TPM passthrough driver

[Anthony Liguori]

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<address@hidden>
>
> ---
>
> 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