diff mbox

[V20,6/8] Add support for cancelling of a TPM command

Message ID 1358524968-22297-7-git-send-email-stefanb@linux.vnet.ibm.com
State New
Headers show

Commit Message

Stefan Berger Jan. 18, 2013, 4:02 p.m. UTC
This patch adds support for cancelling an executing TPM command.
In Linux for example a user can cancel a command through the TPM's
sysfs 'cancel' entry using

echo "1" > /sysfs/.../cancel

This patch propagates the cancellation to the host TPM's sysfs entry.
It also uses the possibility to cancel the command before QEMU VM
shutdown or reboot, which helps in preventing QEMU from hanging while
waiting for the completion of the command.
To relieve higher layers or users from having to determine the TPM's
cancel sysfs entry, the driver searches for the entry in well known
locations.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm_passthrough.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 155 insertions(+), 13 deletions(-)

Comments

Corey Bryant Feb. 4, 2013, 4:02 p.m. UTC | #1
On 01/18/2013 11:02 AM, Stefan Berger wrote:
> This patch adds support for cancelling an executing TPM command.
> In Linux for example a user can cancel a command through the TPM's
> sysfs 'cancel' entry using
>
> echo "1" > /sysfs/.../cancel
>
> This patch propagates the cancellation to the host TPM's sysfs entry.
> It also uses the possibility to cancel the command before QEMU VM
> shutdown or reboot, which helps in preventing QEMU from hanging while
> waiting for the completion of the command.
> To relieve higher layers or users from having to determine the TPM's
> cancel sysfs entry, the driver searches for the entry in well known
> locations.

We discussed offline, but as an fyi for other reviewers this is missing 
command line support for the cancel path.

>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>   hw/tpm_passthrough.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 155 insertions(+), 13 deletions(-)
>
> diff --git a/hw/tpm_passthrough.c b/hw/tpm_passthrough.c
> index f9e6923..1b17c30 100644
> --- a/hw/tpm_passthrough.c
> +++ b/hw/tpm_passthrough.c
> @@ -22,6 +22,8 @@
>    * License along with this library; if not, see <http://www.gnu.org/licenses/>
>    */
>
> +#include <dirent.h>
> +
>   #include "qemu-common.h"
>   #include "qemu-error.h"
>   #include "qemu_socket.h"
> @@ -57,11 +59,18 @@ struct TPMPassthruState {
>
>       char *tpm_dev;
>       int tpm_fd;
> +    bool tpm_executing;
> +    bool tpm_op_canceled;
> +    int cancel_fd;
>       bool had_startup_error;
>   };
>
>   #define TPM_PASSTHROUGH_DEFAULT_DEVICE "/dev/tpm0"
>
> +/* functions */
> +
> +static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
> +
>   static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len)
>   {
>       return send_all(fd, buf, len);
> @@ -79,25 +88,34 @@ static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf)
>       return be32_to_cpu(resp->len);
>   }
>
> -static int tpm_passthrough_unix_tx_bufs(int tpm_fd,
> +static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>                                           const uint8_t *in, uint32_t in_len,
>                                           uint8_t *out, uint32_t out_len)
>   {
>       int ret;
>
> -    ret = tpm_passthrough_unix_write(tpm_fd, in, in_len);
> +    tpm_pt->tpm_op_canceled = false;
> +    tpm_pt->tpm_executing = true;
> +
> +    ret = tpm_passthrough_unix_write(tpm_pt->tpm_fd, in, in_len);
>       if (ret != in_len) {
> -        error_report("tpm_passthrough: error while transmitting data "
> -                     "to TPM: %s (%i)\n",
> -                     strerror(errno), errno);
> +        if (!tpm_pt->tpm_op_canceled ||
> +            (tpm_pt->tpm_op_canceled && errno != ECANCELED)) {
> +            error_report("tpm_passthrough: error while transmitting data "
> +                         "to TPM: %s (%i)\n",
> +                         strerror(errno), errno);
> +        }
>           goto err_exit;
>       }
>
> -    ret = tpm_passthrough_unix_read(tpm_fd, out, out_len);
> +    ret = tpm_passthrough_unix_read(tpm_pt->tpm_fd, out, out_len);
>       if (ret < 0) {
> -        error_report("tpm_passthrough: error while reading data from "
> -                     "TPM: %s (%i)\n",
> -                     strerror(errno), errno);
> +        if (!tpm_pt->tpm_op_canceled ||
> +            (tpm_pt->tpm_op_canceled && errno != ECANCELED)) {
> +            error_report("tpm_passthrough: error while reading data from "
> +                         "TPM: %s (%i)\n",
> +                         strerror(errno), errno);
> +        }
>       } else if (ret < sizeof(struct tpm_resp_hdr) ||
>                  tpm_passthrough_get_size_from_buffer(out) != ret) {
>           ret = -1;
> @@ -110,13 +128,15 @@ err_exit:
>           tpm_write_fatal_error_response(out, out_len);
>       }
>
> +    tpm_pt->tpm_executing = false;
> +
>       return ret;
>   }
>
> -static int tpm_passthrough_unix_transfer(int tpm_fd,
> +static int tpm_passthrough_unix_transfer(TPMPassthruState *tpm_pt,
>                                            const TPMLocality *locty_data)
>   {
> -    return tpm_passthrough_unix_tx_bufs(tpm_fd,
> +    return tpm_passthrough_unix_tx_bufs(tpm_pt,
>                                           locty_data->w_buffer.buffer,
>                                           locty_data->w_offset,
>                                           locty_data->r_buffer.buffer,
> @@ -134,7 +154,7 @@ static void tpm_passthrough_worker_thread(gpointer data,
>
>       switch (cmd) {
>       case TPM_BACKEND_CMD_PROCESS_CMD:
> -        tpm_passthrough_unix_transfer(tpm_pt->tpm_fd,
> +        tpm_passthrough_unix_transfer(tpm_pt,
>                                         thr_parms->tpm_state->locty_data);
>
>           thr_parms->recv_data_callback(thr_parms->tpm_state,
> @@ -174,6 +194,8 @@ static void tpm_passthrough_reset(TPMBackend *tb)
>
>       tpm_backend_thread_end(&tpm_pt->tbt);
>
> +    tpm_passthrough_cancel_cmd(tb);
> +
>       tpm_pt->had_startup_error = false;
>   }
>
> @@ -221,7 +243,24 @@ static void tpm_passthrough_deliver_request(TPMBackend *tb)
>
>   static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
>   {
> -    /* cancelling an ongoing command is known not to work with some TPMs */
> +    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
> +    int n;
> +
> +    /*
> +     * As of Linux 3.7 the tpm_tis driver does not properly cancel
> +     * commands for all TPMs.

Any idea what the plan is for this issue?  Is it an ongoing matter of 
adding kernel support as unsupported TPMs are identified?

> +     * Only cancel if we're busy so we don't cancel someone else's
> +     * command, e.g., a command executed on the host.
> +     */
> +    if (tpm_pt->cancel_fd >= 0 && tpm_pt->tpm_executing) {
> +        n = write(tpm_pt->cancel_fd, "-", 1);
> +        if (n != 1) {
> +            error_report("Canceling TPM command failed: %s\n",
> +                         strerror(errno));
> +        } else {
> +            tpm_pt->tpm_op_canceled = true;
> +        }
> +    }

Would an informational message make sense here for unsupported TPMs 
(when tpm_pt->cancel_fd < 0)?

>   }
>
>   static const char *tpm_passthrough_create_desc(void)
> @@ -281,6 +320,103 @@ static int tpm_passthrough_test_tpmdev(int fd)
>       return 0;
>   }
>
> +/*
> + * Check whether the given base path, e.g.,  /sys/devices/platfrom/tpm_tis,
> + * is the sysfs entry of a TPM. A TPM sysfs entry should be uniquely
> + * recognizable by the file entries 'pcrs' and 'cancel'.
> + * Upon success 'true' is returned and the basebath buffer has '/cancel'
> + * appended.
> + */
> +static bool tpm_passthrough_check_sysfs_cancel(char *basepath, size_t bufsz)
> +{
> +    char path[PATH_MAX];
> +    struct stat statbuf;
> +
> +    snprintf(path, sizeof(path), "%s/pcrs", basepath);
> +    if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) {
> +        return false;
> +    }
> +
> +    snprintf(path, sizeof(path), "%s/cancel", basepath);
> +    if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) {
> +        return false;
> +    }
> +
> +    strncpy(basepath, path, bufsz);
> +
> +    return true;
> +}
> +
> +static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb)
> +{
> +    int fd = -1;
> +    unsigned int i = 0;
> +    DIR *pnp_dir;
> +    char path[PATH_MAX];
> +    struct dirent entry, *result;
> +    bool found = false;

You might be able to get rid of the found variable and..

> +
> +    while (!found) {

loop while (fd == -1)

> +        snprintf(path, sizeof(path), "/sys/devices/pnp%u", i);
> +        pnp_dir = opendir(path);
> +        if (pnp_dir != NULL) {
> +            while (readdir_r(pnp_dir, &entry, &result) == 0 &&
> +                   result != NULL) {
> +                if (!strcmp(entry.d_name, ".") ||
> +                    !strcmp(entry.d_name, "..")) {
> +                    continue;
> +                }
> +                snprintf(path, sizeof(path), "/sys/devices/pnp%u/%s",
> +                         i, entry.d_name);
> +                if (!tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
> +                    continue;
> +                }
> +
> +                fd = open(path, O_WRONLY);
> +                found = true;
> +                break;
> +            }
> +            closedir(pnp_dir);
> +        } else {
> +            break;
> +        }
> +        i++;
> +    }
> +
> +    if (fd >= 0) {
> +        goto exit;
> +    }
> +
> +    /*
> +     * alternative path if ACPI for TPM was not available:
> +     *   modprobe tpm_tis force=1
> +     */
> +    snprintf(path, sizeof(path), "/sys/devices/platform/tpm_tis");
> +    if (tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
> +        fd = open(path, O_WRONLY);
> +        goto exit;
> +    }
> +
> +    /*
> +     * one more thing to try: /sys/class/misc/tpm%u/device/cancel
> +     */
> +    for (i = 0; i < 9; i++) {

Is tpm9 a defined limit, or should this loop be similar to the first one 
where it loops until 'opendir(path) == false'?

> +        snprintf(path, sizeof(path), "/sys/class/misc/tpm%u/device", i);
> +        if (!tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
> +            continue;
> +        }
> +        fd = open(path, O_WRONLY);
> +        break;
> +    }
> +
> +exit:
> +    if (fd >= 0) {
> +        tb->cancel_path = g_strdup(path);
> +    }
> +
> +    return fd;
> +}
> +

A general question about the function above.  I see that 
"/sys/class/misc/tpm%u/device" will be explained in kernel documentation 
here:

https://github.com/shpedoikal/linux/commit/4e21d66c9efbe740b5655bcf66e39873e354f8e9

And the following paths apparently have the same behavior.  But are 
these paths defined somewhere else?
"/sys/devices/pnp%u/%s"
"/sys/devices/platform/tpm_tis"

Also I think a comment pointing to documentation would be a worth-while 
addition to the code.

>   static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
>   {
>       const char *value;
> @@ -337,6 +473,8 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
>           goto err_exit;
>       }
>
> +    tb->s.tpm_pt->cancel_fd = tpm_passthrough_open_sysfs_cancel(tb);
> +
>       return tb;
>
>   err_exit:
> @@ -351,12 +489,16 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
>   {
>       TPMPassthruState *tpm_pt = tb->s.tpm_pt;
>
> +    tpm_passthrough_cancel_cmd(tb);
> +
>       tpm_backend_thread_end(&tpm_pt->tbt);
>
>       close(tpm_pt->tpm_fd);
> +    close(tb->s.tpm_pt->cancel_fd);
>
>       g_free(tb->id);
>       g_free(tb->path);
> +    g_free(tb->cancel_path);
>       g_free(tb->s.tpm_pt->tpm_dev);
>       g_free(tb->s.tpm_pt);
>       g_free(tb);
>
Stefan Berger Feb. 4, 2013, 6:48 p.m. UTC | #2
On 02/04/2013 11:02 AM, Corey Bryant wrote:
> @@ -221,7 +243,24 @@ static void 
> tpm_passthrough_deliver_request(TPMBackend *tb)
>>
>>   static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
>>   {
>> -    /* cancelling an ongoing command is known not to work with some 
>> TPMs */
>> +    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
>> +    int n;
>> +
>> +    /*
>> +     * As of Linux 3.7 the tpm_tis driver does not properly cancel
>> +     * commands for all TPMs.
>
> Any idea what the plan is for this issue?  Is it an ongoing matter of 
> adding kernel support as unsupported TPMs are identified?


I submitted a patch to the tpmdd-devel mailing list fixing the 
cancellation issue with the particular TPM I have on one of my machines. 
Kent Yoder modified it to add a fix for yet another TPM (from a 
different vendor).

https://github.com/shpedoikal/linux/commit/9f11986de7280f999cad342389b48c29002c0f37

The spec seems not be clear enough as to what bits must be set in the 
status register when a TPM command is canceled so that the so far 
implemented cancellation detection is insufficient and needs to be 
adapted to TPM vendors' implementations. All TIS interfaces are supposed 
to support cancellation, though.

>
>> +     * Only cancel if we're busy so we don't cancel someone else's
>> +     * command, e.g., a command executed on the host.
>> +     */
>> +    if (tpm_pt->cancel_fd >= 0 && tpm_pt->tpm_executing) {
>> +        n = write(tpm_pt->cancel_fd, "-", 1);
>> +        if (n != 1) {
>> +            error_report("Canceling TPM command failed: %s\n",
>> +                         strerror(errno));
>> +        } else {
>> +            tpm_pt->tpm_op_canceled = true;
>> +        }
>> +    }
>
> Would an informational message make sense here for unsupported TPMs 
> (when tpm_pt->cancel_fd < 0)?
>

Sure, I can add that.


>> +        snprintf(path, sizeof(path), "/sys/class/misc/tpm%u/device", 
>> i);
>> +        if (!tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
>> +            continue;
>> +        }
>> +        fd = open(path, O_WRONLY);
>> +        break;
>> +    }
>> +
>> +exit:
>> +    if (fd >= 0) {
>> +        tb->cancel_path = g_strdup(path);
>> +    }
>> +
>> +    return fd;
>> +}
>> +
>
> A general question about the function above.  I see that 
> "/sys/class/misc/tpm%u/device" will be explained in kernel 
> documentation here:
>
> https://github.com/shpedoikal/linux/commit/4e21d66c9efbe740b5655bcf66e39873e354f8e9 
>
>
> And the following paths apparently have the same behavior.  But are 
> these paths defined somewhere else?
> "/sys/devices/pnp%u/%s"
> "/sys/devices/platform/tpm_tis"
>
> Also I think a comment pointing to documentation would be a 
> worth-while addition to the code.
>

The /sys/class/misc/tpm%u/device path seems to be always there 
independent of what type of device the TPM is registered as (pnp or 
platform). So I'll modify above code to always use that path instead.

   Stefan
Corey Bryant Feb. 4, 2013, 10:06 p.m. UTC | #3
On 02/04/2013 01:48 PM, Stefan Berger wrote:
> On 02/04/2013 11:02 AM, Corey Bryant wrote:
>> @@ -221,7 +243,24 @@ static void
>> tpm_passthrough_deliver_request(TPMBackend *tb)
>>>
>>>   static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
>>>   {
>>> -    /* cancelling an ongoing command is known not to work with some
>>> TPMs */
>>> +    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
>>> +    int n;
>>> +
>>> +    /*
>>> +     * As of Linux 3.7 the tpm_tis driver does not properly cancel
>>> +     * commands for all TPMs.
>>
>> Any idea what the plan is for this issue?  Is it an ongoing matter of
>> adding kernel support as unsupported TPMs are identified?
>
>
> I submitted a patch to the tpmdd-devel mailing list fixing the
> cancellation issue with the particular TPM I have on one of my machines.
> Kent Yoder modified it to add a fix for yet another TPM (from a
> different vendor).
>
> https://github.com/shpedoikal/linux/commit/9f11986de7280f999cad342389b48c29002c0f37
>
>
> The spec seems not be clear enough as to what bits must be set in the
> status register when a TPM command is canceled so that the so far
> implemented cancellation detection is insufficient and needs to be
> adapted to TPM vendors' implementations. All TIS interfaces are supposed
> to support cancellation, though.
>

It sounds like this is a work in progress at the spec and Linux kernel 
level.  It's certainly not ideal, but I think as long as a message is 
issued for unsupported TPMs, it shouldn't hold up integration of this 
support into QEMU.

>>
>>> +     * Only cancel if we're busy so we don't cancel someone else's
>>> +     * command, e.g., a command executed on the host.
>>> +     */
>>> +    if (tpm_pt->cancel_fd >= 0 && tpm_pt->tpm_executing) {
>>> +        n = write(tpm_pt->cancel_fd, "-", 1);
>>> +        if (n != 1) {
>>> +            error_report("Canceling TPM command failed: %s\n",
>>> +                         strerror(errno));
>>> +        } else {
>>> +            tpm_pt->tpm_op_canceled = true;
>>> +        }
>>> +    }
>>
>> Would an informational message make sense here for unsupported TPMs
>> (when tpm_pt->cancel_fd < 0)?
>>
>
> Sure, I can add that.
>
>

Great, thanks.

>>> +        snprintf(path, sizeof(path), "/sys/class/misc/tpm%u/device",
>>> i);
>>> +        if (!tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
>>> +            continue;
>>> +        }
>>> +        fd = open(path, O_WRONLY);
>>> +        break;
>>> +    }
>>> +
>>> +exit:
>>> +    if (fd >= 0) {
>>> +        tb->cancel_path = g_strdup(path);
>>> +    }
>>> +
>>> +    return fd;
>>> +}
>>> +
>>
>> A general question about the function above.  I see that
>> "/sys/class/misc/tpm%u/device" will be explained in kernel
>> documentation here:
>>
>> https://github.com/shpedoikal/linux/commit/4e21d66c9efbe740b5655bcf66e39873e354f8e9
>>
>>
>> And the following paths apparently have the same behavior.  But are
>> these paths defined somewhere else?
>> "/sys/devices/pnp%u/%s"
>> "/sys/devices/platform/tpm_tis"
>>
>> Also I think a comment pointing to documentation would be a
>> worth-while addition to the code.
>>
>
> The /sys/class/misc/tpm%u/device path seems to be always there
> independent of what type of device the TPM is registered as (pnp or
> platform). So I'll modify above code to always use that path instead.
>

Great, that would simplify things a bit.

>    Stefan
>
>
Stefan Berger Feb. 4, 2013, 10:43 p.m. UTC | #4
On 02/04/2013 05:06 PM, Corey Bryant wrote:
>
>
> On 02/04/2013 01:48 PM, Stefan Berger wrote:
>> On 02/04/2013 11:02 AM, Corey Bryant wrote:
>>>> +    /*
>>>> +     * As of Linux 3.7 the tpm_tis driver does not properly cancel
>>>> +     * commands for all TPMs.
>>>
>>>
>>> Any idea what the plan is for this issue?  Is it an ongoing matter of
>>> adding kernel support as unsupported TPMs are identified?
>>
>>
>> I submitted a patch to the tpmdd-devel mailing list fixing the
>> cancellation issue with the particular TPM I have on one of my machines.
>> Kent Yoder modified it to add a fix for yet another TPM (from a
>> different vendor).
>>
>> https://github.com/shpedoikal/linux/commit/9f11986de7280f999cad342389b48c29002c0f37 
>>
>>
>>
>> The spec seems not be clear enough as to what bits must be set in the
>> status register when a TPM command is canceled so that the so far
>> implemented cancellation detection is insufficient and needs to be
>> adapted to TPM vendors' implementations. All TIS interfaces are supposed
>> to support cancellation, though.
>>
>
> It sounds like this is a work in progress at the spec and Linux kernel 
> level.  It's certainly not ideal, but I think as long as a message is 
> issued for unsupported TPMs, it shouldn't hold up integration of this 
> support into QEMU.


There may be a misunderstanding here: whether canceling an ongoing TPM 
command works is not something that QEMU can tell you. We will always 
find the cancel sysfs entry and we can write a byte into it to trigger 
the cancellation but whether the driver then detects the cancellation 
correctly is something different - the problem here is that the thread 
is waiting inside the driver for the command to complete and 
periodically check whether it has finished and/or was canceled. I 
previously used a different machine where cancellation was working but 
now with the new machine and a different vendor's TPM the cancel sysfs 
entry cancels the command fine but the driver does not detect the fact 
that the command was actually canceled and just continues waiting for 
the command's completion. Also since we don't have access to all TPM 
vendors' TPMs we cannot tell for sure which ones are working and which 
ones are not.

    Stefan
diff mbox

Patch

diff --git a/hw/tpm_passthrough.c b/hw/tpm_passthrough.c
index f9e6923..1b17c30 100644
--- a/hw/tpm_passthrough.c
+++ b/hw/tpm_passthrough.c
@@ -22,6 +22,8 @@ 
  * License along with this library; if not, see <http://www.gnu.org/licenses/>
  */
 
+#include <dirent.h>
+
 #include "qemu-common.h"
 #include "qemu-error.h"
 #include "qemu_socket.h"
@@ -57,11 +59,18 @@  struct TPMPassthruState {
 
     char *tpm_dev;
     int tpm_fd;
+    bool tpm_executing;
+    bool tpm_op_canceled;
+    int cancel_fd;
     bool had_startup_error;
 };
 
 #define TPM_PASSTHROUGH_DEFAULT_DEVICE "/dev/tpm0"
 
+/* functions */
+
+static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
+
 static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len)
 {
     return send_all(fd, buf, len);
@@ -79,25 +88,34 @@  static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf)
     return be32_to_cpu(resp->len);
 }
 
-static int tpm_passthrough_unix_tx_bufs(int tpm_fd,
+static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
                                         const uint8_t *in, uint32_t in_len,
                                         uint8_t *out, uint32_t out_len)
 {
     int ret;
 
-    ret = tpm_passthrough_unix_write(tpm_fd, in, in_len);
+    tpm_pt->tpm_op_canceled = false;
+    tpm_pt->tpm_executing = true;
+
+    ret = tpm_passthrough_unix_write(tpm_pt->tpm_fd, in, in_len);
     if (ret != in_len) {
-        error_report("tpm_passthrough: error while transmitting data "
-                     "to TPM: %s (%i)\n",
-                     strerror(errno), errno);
+        if (!tpm_pt->tpm_op_canceled ||
+            (tpm_pt->tpm_op_canceled && errno != ECANCELED)) {
+            error_report("tpm_passthrough: error while transmitting data "
+                         "to TPM: %s (%i)\n",
+                         strerror(errno), errno);
+        }
         goto err_exit;
     }
 
-    ret = tpm_passthrough_unix_read(tpm_fd, out, out_len);
+    ret = tpm_passthrough_unix_read(tpm_pt->tpm_fd, out, out_len);
     if (ret < 0) {
-        error_report("tpm_passthrough: error while reading data from "
-                     "TPM: %s (%i)\n",
-                     strerror(errno), errno);
+        if (!tpm_pt->tpm_op_canceled ||
+            (tpm_pt->tpm_op_canceled && errno != ECANCELED)) {
+            error_report("tpm_passthrough: error while reading data from "
+                         "TPM: %s (%i)\n",
+                         strerror(errno), errno);
+        }
     } else if (ret < sizeof(struct tpm_resp_hdr) ||
                tpm_passthrough_get_size_from_buffer(out) != ret) {
         ret = -1;
@@ -110,13 +128,15 @@  err_exit:
         tpm_write_fatal_error_response(out, out_len);
     }
 
+    tpm_pt->tpm_executing = false;
+
     return ret;
 }
 
-static int tpm_passthrough_unix_transfer(int tpm_fd,
+static int tpm_passthrough_unix_transfer(TPMPassthruState *tpm_pt,
                                          const TPMLocality *locty_data)
 {
-    return tpm_passthrough_unix_tx_bufs(tpm_fd,
+    return tpm_passthrough_unix_tx_bufs(tpm_pt,
                                         locty_data->w_buffer.buffer,
                                         locty_data->w_offset,
                                         locty_data->r_buffer.buffer,
@@ -134,7 +154,7 @@  static void tpm_passthrough_worker_thread(gpointer data,
 
     switch (cmd) {
     case TPM_BACKEND_CMD_PROCESS_CMD:
-        tpm_passthrough_unix_transfer(tpm_pt->tpm_fd,
+        tpm_passthrough_unix_transfer(tpm_pt,
                                       thr_parms->tpm_state->locty_data);
 
         thr_parms->recv_data_callback(thr_parms->tpm_state,
@@ -174,6 +194,8 @@  static void tpm_passthrough_reset(TPMBackend *tb)
 
     tpm_backend_thread_end(&tpm_pt->tbt);
 
+    tpm_passthrough_cancel_cmd(tb);
+
     tpm_pt->had_startup_error = false;
 }
 
@@ -221,7 +243,24 @@  static void tpm_passthrough_deliver_request(TPMBackend *tb)
 
 static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
 {
-    /* cancelling an ongoing command is known not to work with some TPMs */
+    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
+    int n;
+
+    /*
+     * As of Linux 3.7 the tpm_tis driver does not properly cancel
+     * commands for all TPMs.
+     * Only cancel if we're busy so we don't cancel someone else's
+     * command, e.g., a command executed on the host.
+     */
+    if (tpm_pt->cancel_fd >= 0 && tpm_pt->tpm_executing) {
+        n = write(tpm_pt->cancel_fd, "-", 1);
+        if (n != 1) {
+            error_report("Canceling TPM command failed: %s\n",
+                         strerror(errno));
+        } else {
+            tpm_pt->tpm_op_canceled = true;
+        }
+    }
 }
 
 static const char *tpm_passthrough_create_desc(void)
@@ -281,6 +320,103 @@  static int tpm_passthrough_test_tpmdev(int fd)
     return 0;
 }
 
+/*
+ * Check whether the given base path, e.g.,  /sys/devices/platfrom/tpm_tis,
+ * is the sysfs entry of a TPM. A TPM sysfs entry should be uniquely
+ * recognizable by the file entries 'pcrs' and 'cancel'.
+ * Upon success 'true' is returned and the basebath buffer has '/cancel'
+ * appended.
+ */
+static bool tpm_passthrough_check_sysfs_cancel(char *basepath, size_t bufsz)
+{
+    char path[PATH_MAX];
+    struct stat statbuf;
+
+    snprintf(path, sizeof(path), "%s/pcrs", basepath);
+    if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) {
+        return false;
+    }
+
+    snprintf(path, sizeof(path), "%s/cancel", basepath);
+    if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) {
+        return false;
+    }
+
+    strncpy(basepath, path, bufsz);
+
+    return true;
+}
+
+static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb)
+{
+    int fd = -1;
+    unsigned int i = 0;
+    DIR *pnp_dir;
+    char path[PATH_MAX];
+    struct dirent entry, *result;
+    bool found = false;
+
+    while (!found) {
+        snprintf(path, sizeof(path), "/sys/devices/pnp%u", i);
+        pnp_dir = opendir(path);
+        if (pnp_dir != NULL) {
+            while (readdir_r(pnp_dir, &entry, &result) == 0 &&
+                   result != NULL) {
+                if (!strcmp(entry.d_name, ".") ||
+                    !strcmp(entry.d_name, "..")) {
+                    continue;
+                }
+                snprintf(path, sizeof(path), "/sys/devices/pnp%u/%s",
+                         i, entry.d_name);
+                if (!tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
+                    continue;
+                }
+
+                fd = open(path, O_WRONLY);
+                found = true;
+                break;
+            }
+            closedir(pnp_dir);
+        } else {
+            break;
+        }
+        i++;
+    }
+
+    if (fd >= 0) {
+        goto exit;
+    }
+
+    /*
+     * alternative path if ACPI for TPM was not available:
+     *   modprobe tpm_tis force=1
+     */
+    snprintf(path, sizeof(path), "/sys/devices/platform/tpm_tis");
+    if (tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
+        fd = open(path, O_WRONLY);
+        goto exit;
+    }
+
+    /*
+     * one more thing to try: /sys/class/misc/tpm%u/device/cancel
+     */
+    for (i = 0; i < 9; i++) {
+        snprintf(path, sizeof(path), "/sys/class/misc/tpm%u/device", i);
+        if (!tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
+            continue;
+        }
+        fd = open(path, O_WRONLY);
+        break;
+    }
+
+exit:
+    if (fd >= 0) {
+        tb->cancel_path = g_strdup(path);
+    }
+
+    return fd;
+}
+
 static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
 {
     const char *value;
@@ -337,6 +473,8 @@  static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
         goto err_exit;
     }
 
+    tb->s.tpm_pt->cancel_fd = tpm_passthrough_open_sysfs_cancel(tb);
+
     return tb;
 
 err_exit:
@@ -351,12 +489,16 @@  static void tpm_passthrough_destroy(TPMBackend *tb)
 {
     TPMPassthruState *tpm_pt = tb->s.tpm_pt;
 
+    tpm_passthrough_cancel_cmd(tb);
+
     tpm_backend_thread_end(&tpm_pt->tbt);
 
     close(tpm_pt->tpm_fd);
+    close(tb->s.tpm_pt->cancel_fd);
 
     g_free(tb->id);
     g_free(tb->path);
+    g_free(tb->cancel_path);
     g_free(tb->s.tpm_pt->tpm_dev);
     g_free(tb->s.tpm_pt);
     g_free(tb);