From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38503) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RaFKT-0004aW-2w for qemu-devel@nongnu.org; Mon, 12 Dec 2011 18:30:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RaFKR-00070U-DZ for qemu-devel@nongnu.org; Mon, 12 Dec 2011 18:30:41 -0500 Received: from mail-qy0-f173.google.com ([209.85.216.173]:60265) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RaFKR-00070O-BF for qemu-devel@nongnu.org; Mon, 12 Dec 2011 18:30:39 -0500 Received: by qcsd15 with SMTP id d15so4830483qcs.4 for ; Mon, 12 Dec 2011 15:30:38 -0800 (PST) Message-ID: <4EE68E9C.80906@codemonkey.ws> Date: Mon, 12 Dec 2011 17:30:36 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1323717136-21661-1-git-send-email-stefanb@linux.vnet.ibm.com> <1323717136-21661-8-git-send-email-stefanb@linux.vnet.ibm.com> In-Reply-To: <1323717136-21661-8-git-send-email-stefanb@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V13 7/7] Add fd parameter for TPM passthrough driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Berger Cc: mst@redhat.com, qemu-devel@nongnu.org, andreas.niederl@iaik.tugraz.at 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 > > --- > > 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