On Mon, 2020-09-28 at 20:07 +0300, Jarkko Sakkinen wrote: > On Mon, Sep 28, 2020 at 07:31:18PM +0300, Jarkko Sakkinen wrote: > > > Well, um, that's precisely what this function originally did when > > > it was inside drivers/char/tpm. You told the guy who did the > > > move into security/keys/trusted-keys to convert everything to use > > > tpm_send which encapsulates the get/put operation, which is why > > > we now have the flush bug. If you really want it done like this, > > > then I'd recommend moving everything back to drivers/char/tpm so > > > we don't have to do a global exposure of a load of tpm internal > > > functions (i.e. move them from drivers/char/tmp.h to > > > include/linux/tpm.h and do an export on them). > > > > My BuildRoot test image did not include the patch. I was wondering > > why I did not bump into deadlock with the fix candidate :-/ Forgot > > export LINUX_OVERRIDE_SRCDIR. > > > > But you are absolutely correct, thanks for recalling. I made a > > mistake there. > > > > I do disagree though that this should be moved back to > > drivers/char/tpm, as also TPM 1.x code lives in trusted-keys. It is > > good to have API for doing sequences TPM commands and keep the core > > in drivers/char/tpm. I think tpm2_load_cmd is likely going to have to move back anyway just because more things than trusted keys need to use it. I can't really see any other use for the seal/unseal so they can stay in trusted keys until someone finds a use for them. > > If you look at tpm_send() it is in essence just simply locking TPM > > and and calling tpm_transmit_cmd(). And tpm_transmit_cmd() is > > already an exported symbol. It only needs to be declared in > > include/linux/tpm.h. > > > > I'd suggest that I refine my series to call tpm_transmit_cmd() and > > we have a fairly clean solution where the load sequence is atomic. > > I see that it is perfectly fine to make tpm_transmit_cmd() globally > callable. It is already used by tpm_vtpm_proxy and does have clear > semantics. > > The way you use it is just: > > 1. tpm_try_get_ops > 2. Use tpm_transmit_cmd() N times. > 3. tpm_put_ops > > If we moved TPM 2.x trusted keys code back to drivers/char/tpm,for > the sake of consistency the same would have to be done for TPM 1.2 > code. I'd rather fix the regression and be done with it. > > Or if reverted like that, also asym_tpm.c code should also live > inside the TPM driver directory. > > All this work with tpm_buf and the locking functions makes most sense > if it gives ability for callers to build their own TPM commands > > I'm right now building test image with v3 of my fixes (this time > properly included to the kernel image). I also uploaded the > (untested) patches over here: > > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/?h=trusted-fix I think we can do that ... in which case the fix for the tis interrupt trigger also becomes a get/put ops around the tpm2_get_tpm_pt. After the transformation is complete, tpm_send() becomes obsolete, doesn't it, so it can be removed? James
