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

[Stefan Berger]

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.

> > > 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.


> >                                             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.

> >                           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.


> >                                                             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.


> > > 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.

   Stefan