On 10/09/2017 06:56 PM, Marc-André Lureau wrote:
> The TPM backend processing thread has common shared variable race
> issues. (they should not be so easy to reach since guest interaction
> with the device is slow compared to host emulation)
>
> An obvious one is setting op_cancelled from device thread after
> calling write(cancel_fd). The backend thread may return before the
> device thread has set the variable. Instead set it before
> cancellation. Even if the write() failed, the end result is command
> get possibly cancelled (even if cancellation came from external
> sources it doesn't matter much).
>
> It's worth to consider removing the backend processing thread for now.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> hw/tpm/tpm_passthrough.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 0806cf86af..d71d64e8aa 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -89,6 +89,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
> bool is_selftest;
> const struct tpm_resp_hdr *hdr;
>
> + /* FIXME: protect shared variables or use other sync mechanism */
> tpm_pt->tpm_op_canceled = false;
> tpm_pt->tpm_executing = true;
> *selftest_done = false;
> @@ -178,12 +179,11 @@ static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
> */
> if (tpm_pt->tpm_executing) {
> if (tpm_pt->cancel_fd >= 0) {
> + tpm_pt->tpm_op_canceled = true;
> n = write(tpm_pt->cancel_fd, "-", 1);
> if (n != 1) {
> error_report("Canceling TPM command failed: %s",
> strerror(errno));
Shouldn't we set it to false here ?
> - } else {
> - tpm_pt->tpm_op_canceled = true;
> }
> } else {
> error_report("Cannot cancel TPM command due to missing "