On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <[email protected]> wrote: > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <[email protected]> > > wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange > > > > for the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's > > > no TPM rather than waiting until first use to find out it can never > > > work. Is there some piece of user code that depends on the > > > successful insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > That's a nasty dependency caused by every key type module exporting a > symbol for its key type. It really seems that key types should be > looked up by name not symbol to prevent more of these dependency issues > from spreading. Something like this (untested and definitely won't > work without doing an EXPORT_SYMBOL on key_type_lookup). > > If it does look acceptable we can also disentangle the nasty module > dependencies in the encrypted key code around masterkey which alone > should be a huge improvement because that code is too hacky to live. > > James > > --- > diff --git a/security/keys/encrypted-keys/masterkey_trusted.c > b/security/keys/encrypted-keys/masterkey_trusted.c > index dc3d18cae642..b98416f091e2 100644 > --- a/security/keys/encrypted-keys/masterkey_trusted.c > +++ b/security/keys/encrypted-keys/masterkey_trusted.c > @@ -19,6 +19,7 @@ > #include <keys/trusted-type.h> > #include <keys/encrypted-type.h> > #include "encrypted.h" > +#include "../internal.h" > > /* > * request_trusted_key - request the trusted key > @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char *trusted_desc, > { > struct trusted_key_payload *tpayload; > struct key *tkey; > + struct key_type *type; > > - tkey = request_key(&key_type_trusted, trusted_desc, NULL); > + type = key_type_lookup("trusted"); > + if (IS_ERR(type)) { > + tkey = (struct key *)type; > + goto error; > + } > + tkey = request_key(type, trusted_desc, NULL); > if (IS_ERR(tkey)) > goto error;
This falls over with the need to pin the module while any key that needs service from the hosting key_type operations might be live in the system. I could hang a "struct module *" off of the key_type so the host module can be pinned, but that requires teaching all consumers of the key_type module lifetime. Not impossible, but I think too big for a fix, and I'd rather go with this local fixup to drop the dependency on tpm_default_chip() successfully enumerating a TPM. _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
