Re: [Qemu-devel] [PATCH V27 1/7] Support for TPM command line options

[Markus Armbruster]

Stefan Berger <address@hidden> writes:

> On 03/19/2013 03:45 AM, Markus Armbruster wrote:
>> Stefan Berger <address@hidden> writes:
>>
>>> On 03/18/2013 09:10 AM, Markus Armbruster wrote:
>>>>> In case the TPM is currently not operating on a command there will be
>>>>> no impact. In case the TPM is operating on a command, it will hold the
>>>>> thread inside /dev/tpm0 until the command has finished. The solution
>>>>> here is to write a byte into sysfs file to terminate the TPM from
>>>>> further executing the command and return; this code exists and is
>>>>> being invoked in all other cases than abnormal termination obviously.
>>>> Unlike other kernel resources such as file descriptors, this one isn't
>>>> tied to a process and cleaned up automatically on process termination.
>>>> Correct?
>>> No, it's also a file descriptor and will be cleaned up upon process
>>> termination.
>> Closing the file descriptor does *not* free kernel resources associated
>> with it?!?
>
> That's not what I said. You suggested "this one isn't tied to a
> process and cleaned up automatically on process termination", which is
> not true but the problem lies somewhere else.

Well, if there's kernel action needed beyond the automatic closing of
file descriptors on process termination, there's something weird going
on.  I'm trying to understand what.  Please bear with me.

>>>> Therefore, we try to clean it up manually.  Actual cleanup code is
>>>> tpm_passthrough_cancel_cmd().  Correct?
>>> No, tpm_passthrough_cancel_cmd() sends a byte to the TPM's sysfs entry
>>> for canceling an ongoing command. QEMU has a thread that sends the
>>> commands to the TPM via write() and gets the responses in read(). The
>>> issue being the thread is held inside the write() (!) until the
>>> command has completed inside the host TPM.
>> You mean write() blocks?
>
> Yep.

Got it.

>>>                                             Inside the passthrough
>>> driver we set a flag while the command is executing and if a command
>>> is executing when for example QEMU is reset, then the sysfs entry is
>>> written to to get the thread out of the write() due to termination of
>>> the ongoing TPM command.
>> I can understand a need to abort the write() on guest reset, but why on
>> QEMU exit()?  Shouldn't process termination abort the write() just fine?
>
> Nope, process termination won't get you out of write(). Even if this
> was to be changed today in the kernel driver, the old kernels are out
> there for a while to be.

Now I'm confused.  Are you telling me that exit() hangs until the thread
currently blocked in write() to TPM is made runnable by the completion
of the TPM command?

>>>                           Depending on which version of Fedora is run,
>>> the termination of the command may or may not get invoked.
>> May or may not get invoked on what condition?
>
> Behavior of the OS upon shutdown itself. Whether it waits for all
> processes to terminate or not.

I'm getting more confused.

>>>                                                             F14 for
>>> example does not seem to care about waiting until all  user-level
>>> processes have properly shut down and would reset  while F16 seems to
>>> wait for all processes to properly terminate.
>> F14 is more than nine months past it's shelf date.  Why should we
>> support it when its makers don't?
>
> F14 is just an example of one OS that can be run just happens to be
> old by by now; there may be others that behave the same way.
>
>>> The cancellation of an ongoing command may be initiated inside the
>>> guest as well and will also cause tpm_passthrough_cancel_cmd() to be
>>> invoked if a command is executed by the host TPM on behalf of the
>>> guest.
>> Passing through the guest's cancellation makes sense to me.
>>
>>> There were/are(?) some issues with cancellation of ongoing
>>> commands. Different vendors seem to set different bits to indicate to
>>> the driver as to when a command has been canceled. The specs are not
>>> clear enough in that area. We fixed this for the TPMs that we have
>>> access (to wake up the thread inside the write()).
>> I'm not doubting the need to cancel commands.  I'm just trying to
>> understand why we do it on exit().
>
> I can write an OS that sends a command to the TPM and then shuts down
> without waiting for the program to terminate or the even the response
> from the TPM to come back . In this case we would need to cancel the
> command on exit()l.

Aha, now we're getting somewhere.

Are you talking about the following scenario?

* Guest sends TPM command, QEMU passes it through, QEMU thread blocks in
  write() to TPM.

* Guest powers down, QEMU is configured to terminate on powerdown, calls
  exit().

* Unless you explicitly cancel the TPM command, $BAD_THINGS can happen.

If yes, please describe at least one instance of $BAD_THINGS for me
again.

>>>> The only existing cleanup function tpm_passthrough_destroy() does a
>>>> whole lot more.  Just to make sure I understand what's going on:
>>>> everything but tpm_passthrough_cancel_cmd() is effectively a no-op
>>>> there, correct?
>>> It depends on what you mean with no-op. In tpm_passthrough_destroy() a
>>> possibly ongoing command is canceled, then the thread is signaled to
>>> terminate and join()ed. The rest is closing of file descriptors and
>>> freeing of memory.
>> Closing file descriptors and freeing memory on exit() is a no-op at
>> best, and a source of bugs otherwise.  The kernel already cleans these
>> up just fine.
>>
>> Unless the thread does something interesting in response to its
>> termination signal, making it terminate is also a no-op.  I can't see
>> anything interesting happening, please correct me if I'm wrong.
>>
>
> Per what you are saying only no-ops are happening.

Thanks.