[PATCH v10 08/17] tpm: call tpm2_flush_space() on error in tpm_try_transmit()

[Jarkko Sakkinen]

On Tue, Jan 29, 2019 at 09:06:01AM -0800, James Bottomley wrote:
> On Wed, 2019-01-16 at 23:23 +0200, Jarkko Sakkinen wrote:
> [...]
> > -	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
> > +out_space:
> > +	if (rc)
> > +		tpm2_flush_space(chip);
> > +	else
> > +		rc = tpm2_commit_space(chip, space, ordinal, buf,
> > &len);
> 
> I don't think this is quite right.  tpm2_flush_space only flushes the
> handles it knows about and those are the ones from before the TPM
> operation was attempted.  If the operation has altered the internal
> state we could miss a created handle in this flush and it would
> effectively reside forever in the TPM.  We should be able to rely on
> the TPM preserving the original state if it returns an error, so I
> think your patch works for that part.  However rc is also set to
> -EFAULT on a transmission error and if that's on the receive path, the
> TPM may have changed state before the error occurred.

If TPM is working properly in the first place, tpm2_commit_space() is
always called (e.g. in a situation where TPM gives a TPM error). Your
deduction about the opposite is absolutely correct. Thanks!

> If the object is to move the TPM back to where it was before the error
> occurred, even in the case of transmit errors, then I think we need to
> invent a new kind of flush that queries the current TPM state and then
> flushes everything.

I think this consideration is anyway out of scope for this patch set.
I'd hope you would also skim through v11 as soon as I get it prepared,
at least the patches where I've added an explicit CC (one or two at
most).

Thanks again.

/Jarkko