[PATCH v2 0/2] move TPM-specific fields into trusted_tpm_options

[PATCH v2 0/2] move TPM-specific fields into trusted_tpm_options

[Srish Srinivasan]

The backend-agnostic trusted_key_options structure carries TPM-specific
fields. With the recent addition of a backend-private pointer, these fields
can be moved out of the generic options structure.

This patch series intends to reloacte all TPM-spcific fields into a newly
defined trusted_tpm_options structure. A pointer to the trusted_tpm_options
struct is stored in trusted_key_option's private.

Along with the migration of TPM-specific fields, this patch series includes
a preparatory clean-up patch: replacing pr_info() with pr_debug() and using
KERN_DEBUG for print_hex_dump() for logging debug information.

Testing covered both TPM 1.2 and TPM 2.0 backends (virtual environment),
including trusted key creation, revocation, unlinking, invalidation, and
loading keys from encrypted blobs. I would welcome any additional testing
from upstream to further strengthen the validation.

Changelog:

v2:
 * Exclude the bug-fix patch as it has already been applied to 6.19-rc7
 * Rename instances of trusted_tpm_options from tpm_opts to private
 * Use pr_debug and KERN_DEBUG for logging debug messages (preparatory
   clean up patch)
 * Address comments from Jarkko

Srish Srinivasan (2):
  keys/trusted_keys: clean up debug message logging in the tpm backend
  keys/trusted_keys: move TPM-specific fields into trusted_tpm_options

 include/keys/trusted-type.h               |  11 --
 include/keys/trusted_tpm.h                |  14 +++
 security/keys/trusted-keys/trusted_tpm1.c | 129 ++++++++++++----------
 security/keys/trusted-keys/trusted_tpm2.c |  51 +++++----
 4 files changed, 111 insertions(+), 94 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

[Srish Srinivasan]

The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
for logging debug information.

Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
Remove TPM_DEBUG.

No functional change intended.

Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
 security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index c865c97aa1b4..216caef97ffc 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -46,28 +46,25 @@ enum {
 	SRK_keytype = 4
 };
 
-#define TPM_DEBUG 0
-
-#if TPM_DEBUG
 static inline void dump_options(struct trusted_key_options *o)
 {
-	pr_info("sealing key type %d\n", o->keytype);
-	pr_info("sealing key handle %0X\n", o->keyhandle);
-	pr_info("pcrlock %d\n", o->pcrlock);
-	pr_info("pcrinfo %d\n", o->pcrinfo_len);
-	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
+	pr_debug("sealing key type %d\n", o->keytype);
+	pr_debug("sealing key handle %0X\n", o->keyhandle);
+	pr_debug("pcrlock %d\n", o->pcrlock);
+	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
 		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
 }
 
 static inline void dump_sess(struct osapsess *s)
 {
-	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
+	print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
 		       16, 1, &s->handle, 4, 0);
-	pr_info("secret:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("secret:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
 		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
-	pr_info("trusted-key: enonce:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("trusted-key: enonce:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
 		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
 }
 
@@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
 {
 	int len;
 
-	pr_info("\ntpm buffer\n");
+	pr_debug("\ntpm buffer\n");
 	len = LOAD32(buf, TPM_SIZE_OFFSET);
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
-}
-#else
-static inline void dump_options(struct trusted_key_options *o)
-{
-}
-
-static inline void dump_sess(struct osapsess *s)
-{
-}
-
-static inline void dump_tpm_buf(unsigned char *buf)
-{
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
 }
-#endif
 
 static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
 		       unsigned int keylen, ...)
-- 
2.43.0

[PATCH v2 2/2] keys/trusted_keys: move TPM-specific fields into trusted_tpm_options

[Srish Srinivasan]

The trusted_key_options struct contains TPM-specific fields (keyhandle,
keyauth, blobauth_len, blobauth, pcrinfo_len, pcrinfo, pcrlock, hash,
policydigest_len, policydigest, and policyhandle). This leads to the
accumulation of backend-specific fields in the generic options structure.

Define trusted_tpm_options structure and move the TPM-specific fields
there. Store a pointer to trusted_tpm_options in trusted_key_options's
private.

No functional change intended.

Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
 include/keys/trusted-type.h               | 11 ---
 include/keys/trusted_tpm.h                | 14 ++++
 security/keys/trusted-keys/trusted_tpm1.c | 95 ++++++++++++++---------
 security/keys/trusted-keys/trusted_tpm2.c | 51 ++++++------
 4 files changed, 102 insertions(+), 69 deletions(-)

diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 03527162613f..b80f250305b8 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -39,17 +39,6 @@ struct trusted_key_payload {
 
 struct trusted_key_options {
 	uint16_t keytype;
-	uint32_t keyhandle;
-	unsigned char keyauth[TPM_DIGEST_SIZE];
-	uint32_t blobauth_len;
-	unsigned char blobauth[TPM_DIGEST_SIZE];
-	uint32_t pcrinfo_len;
-	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
-	int pcrlock;
-	uint32_t hash;
-	uint32_t policydigest_len;
-	unsigned char policydigest[MAX_DIGEST_SIZE];
-	uint32_t policyhandle;
 	void *private;
 };
 
diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
index 0fadc6a4f166..355ebd36cbfd 100644
--- a/include/keys/trusted_tpm.h
+++ b/include/keys/trusted_tpm.h
@@ -7,6 +7,20 @@
 
 extern struct trusted_key_ops trusted_key_tpm_ops;
 
+struct trusted_tpm_options {
+	uint32_t keyhandle;
+	unsigned char keyauth[TPM_DIGEST_SIZE];
+	uint32_t blobauth_len;
+	unsigned char blobauth[TPM_DIGEST_SIZE];
+	uint32_t pcrinfo_len;
+	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
+	int pcrlock;
+	uint32_t hash;
+	uint32_t policydigest_len;
+	unsigned char policydigest[MAX_DIGEST_SIZE];
+	uint32_t policyhandle;
+};
+
 int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options);
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 216caef97ffc..741b1d47d9f8 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -48,12 +48,14 @@ enum {
 
 static inline void dump_options(struct trusted_key_options *o)
 {
+	struct trusted_tpm_options *private = o->private;
+
 	pr_debug("sealing key type %d\n", o->keytype);
-	pr_debug("sealing key handle %0X\n", o->keyhandle);
-	pr_debug("pcrlock %d\n", o->pcrlock);
-	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+	pr_debug("sealing key handle %0X\n", private->keyhandle);
+	pr_debug("pcrlock %d\n", private->pcrlock);
+	pr_debug("pcrinfo %d\n", private->pcrinfo_len);
 	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
-		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
+		       16, 1, private->pcrinfo, private->pcrinfo_len, 0);
 }
 
 static inline void dump_sess(struct osapsess *s)
@@ -609,6 +611,7 @@ static int tpm_unseal(struct tpm_buf *tb,
 static int key_seal(struct trusted_key_payload *p,
 		    struct trusted_key_options *o)
 {
+	struct trusted_tpm_options *private = o->private;
 	struct tpm_buf tb;
 	int ret;
 
@@ -619,9 +622,10 @@ static int key_seal(struct trusted_key_payload *p,
 	/* include migratable flag at end of sealed key */
 	p->key[p->key_len] = p->migratable;
 
-	ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth,
+	ret = tpm_seal(&tb, o->keytype, private->keyhandle, private->keyauth,
 		       p->key, p->key_len + 1, p->blob, &p->blob_len,
-		       o->blobauth, o->pcrinfo, o->pcrinfo_len);
+		       private->blobauth, private->pcrinfo,
+		       private->pcrinfo_len);
 	if (ret < 0)
 		pr_info("srkseal failed (%d)\n", ret);
 
@@ -635,6 +639,7 @@ static int key_seal(struct trusted_key_payload *p,
 static int key_unseal(struct trusted_key_payload *p,
 		      struct trusted_key_options *o)
 {
+	struct trusted_tpm_options *private = o->private;
 	struct tpm_buf tb;
 	int ret;
 
@@ -642,8 +647,8 @@ static int key_unseal(struct trusted_key_payload *p,
 	if (ret)
 		return ret;
 
-	ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
-			 o->blobauth, p->key, &p->key_len);
+	ret = tpm_unseal(&tb, private->keyhandle, private->keyauth, p->blob,
+			 p->blob_len, private->blobauth, p->key, &p->key_len);
 	if (ret < 0)
 		pr_info("srkunseal failed (%d)\n", ret);
 	else
@@ -680,6 +685,7 @@ static const match_table_t key_tokens = {
 static int getoptions(char *c, struct trusted_key_payload *pay,
 		      struct trusted_key_options *opt)
 {
+	struct trusted_tpm_options *private = opt->private;
 	substring_t args[MAX_OPT_ARGS];
 	char *p = c;
 	int token;
@@ -695,7 +701,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 	if (tpm2 < 0)
 		return tpm2;
 
-	opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
+	private->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
 
 	if (!c)
 		return 0;
@@ -709,11 +715,11 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 
 		switch (token) {
 		case Opt_pcrinfo:
-			opt->pcrinfo_len = strlen(args[0].from) / 2;
-			if (opt->pcrinfo_len > MAX_PCRINFO_SIZE)
+			private->pcrinfo_len = strlen(args[0].from) / 2;
+			if (private->pcrinfo_len > MAX_PCRINFO_SIZE)
 				return -EINVAL;
-			res = hex2bin(opt->pcrinfo, args[0].from,
-				      opt->pcrinfo_len);
+			res = hex2bin(private->pcrinfo, args[0].from,
+				      private->pcrinfo_len);
 			if (res < 0)
 				return -EINVAL;
 			break;
@@ -722,12 +728,12 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			if (res < 0)
 				return -EINVAL;
 			opt->keytype = SEAL_keytype;
-			opt->keyhandle = handle;
+			private->keyhandle = handle;
 			break;
 		case Opt_keyauth:
 			if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE)
 				return -EINVAL;
-			res = hex2bin(opt->keyauth, args[0].from,
+			res = hex2bin(private->keyauth, args[0].from,
 				      SHA1_DIGEST_SIZE);
 			if (res < 0)
 				return -EINVAL;
@@ -738,21 +744,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			 * hex strings.  TPM 2.0 authorizations are simple
 			 * passwords (although it can take a hash as well)
 			 */
-			opt->blobauth_len = strlen(args[0].from);
+			private->blobauth_len = strlen(args[0].from);
 
-			if (opt->blobauth_len == 2 * TPM_DIGEST_SIZE) {
-				res = hex2bin(opt->blobauth, args[0].from,
+			if (private->blobauth_len == 2 * TPM_DIGEST_SIZE) {
+				res = hex2bin(private->blobauth, args[0].from,
 					      TPM_DIGEST_SIZE);
 				if (res < 0)
 					return -EINVAL;
 
-				opt->blobauth_len = TPM_DIGEST_SIZE;
+				private->blobauth_len = TPM_DIGEST_SIZE;
 				break;
 			}
 
-			if (tpm2 && opt->blobauth_len <= sizeof(opt->blobauth)) {
-				memcpy(opt->blobauth, args[0].from,
-				       opt->blobauth_len);
+			if (tpm2 &&
+			    private->blobauth_len <=
+			    sizeof(private->blobauth)) {
+				memcpy(private->blobauth, args[0].from,
+				       private->blobauth_len);
 				break;
 			}
 
@@ -770,14 +778,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			res = kstrtoul(args[0].from, 10, &lock);
 			if (res < 0)
 				return -EINVAL;
-			opt->pcrlock = lock;
+			private->pcrlock = lock;
 			break;
 		case Opt_hash:
 			if (test_bit(Opt_policydigest, &token_mask))
 				return -EINVAL;
 			for (i = 0; i < HASH_ALGO__LAST; i++) {
 				if (!strcmp(args[0].from, hash_algo_name[i])) {
-					opt->hash = i;
+					private->hash = i;
 					break;
 				}
 			}
@@ -789,14 +797,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			}
 			break;
 		case Opt_policydigest:
-			digest_len = hash_digest_size[opt->hash];
+			digest_len = hash_digest_size[private->hash];
 			if (!tpm2 || strlen(args[0].from) != (2 * digest_len))
 				return -EINVAL;
-			res = hex2bin(opt->policydigest, args[0].from,
+			res = hex2bin(private->policydigest, args[0].from,
 				      digest_len);
 			if (res < 0)
 				return -EINVAL;
-			opt->policydigest_len = digest_len;
+			private->policydigest_len = digest_len;
 			break;
 		case Opt_policyhandle:
 			if (!tpm2)
@@ -804,7 +812,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			res = kstrtoul(args[0].from, 16, &handle);
 			if (res < 0)
 				return -EINVAL;
-			opt->policyhandle = handle;
+			private->policyhandle = handle;
 			break;
 		default:
 			return -EINVAL;
@@ -815,6 +823,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 
 static struct trusted_key_options *trusted_options_alloc(void)
 {
+	struct trusted_tpm_options *private;
 	struct trusted_key_options *options;
 	int tpm2;
 
@@ -827,14 +836,23 @@ static struct trusted_key_options *trusted_options_alloc(void)
 		/* set any non-zero defaults */
 		options->keytype = SRK_keytype;
 
-		if (!tpm2)
-			options->keyhandle = SRKHANDLE;
+		private = kzalloc(sizeof(*private), GFP_KERNEL);
+		if (!private) {
+			kfree_sensitive(options);
+			options = NULL;
+		} else {
+			if (!tpm2)
+				private->keyhandle = SRKHANDLE;
+
+			options->private = private;
+		}
 	}
 	return options;
 }
 
 static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
 {
+	struct trusted_tpm_options *private = NULL;
 	struct trusted_key_options *options = NULL;
 	int ret = 0;
 	int tpm2;
@@ -852,7 +870,8 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
 		goto out;
 	dump_options(options);
 
-	if (!options->keyhandle && !tpm2) {
+	private = options->private;
+	if (!private->keyhandle && !tpm2) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -866,20 +885,22 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
 		goto out;
 	}
 
-	if (options->pcrlock) {
-		ret = pcrlock(options->pcrlock);
+	if (private->pcrlock) {
+		ret = pcrlock(private->pcrlock);
 		if (ret < 0) {
 			pr_info("pcrlock failed (%d)\n", ret);
 			goto out;
 		}
 	}
 out:
+	kfree_sensitive(options->private);
 	kfree_sensitive(options);
 	return ret;
 }
 
 static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
 {
+	struct trusted_tpm_options *private = NULL;
 	struct trusted_key_options *options = NULL;
 	int ret = 0;
 	int tpm2;
@@ -897,7 +918,8 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
 		goto out;
 	dump_options(options);
 
-	if (!options->keyhandle && !tpm2) {
+	private = options->private;
+	if (!private->keyhandle && !tpm2) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -909,14 +931,15 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
 	if (ret < 0)
 		pr_info("key_unseal failed (%d)\n", ret);
 
-	if (options->pcrlock) {
-		ret = pcrlock(options->pcrlock);
+	if (private->pcrlock) {
+		ret = pcrlock(private->pcrlock);
 		if (ret < 0) {
 			pr_info("pcrlock failed (%d)\n", ret);
 			goto out;
 		}
 	}
 out:
+	kfree_sensitive(options->private);
 	kfree_sensitive(options);
 	return ret;
 }
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 6340823f8b53..94e01249b921 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -24,6 +24,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
 			   u8 *src, u32 len)
 {
+	struct trusted_tpm_options *private = options->private;
 	const int SCRATCH_SIZE = PAGE_SIZE;
 	u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
 	u8 *work = scratch, *work1;
@@ -46,7 +47,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 	work = asn1_encode_oid(work, end_work, tpm2key_oid,
 			       asn1_oid_len(tpm2key_oid));
 
-	if (options->blobauth_len == 0) {
+	if (private->blobauth_len == 0) {
 		unsigned char bool[3], *w = bool;
 		/* tag 0 is emptyAuth */
 		w = asn1_encode_boolean(w, w + sizeof(bool), true);
@@ -69,7 +70,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 		goto err;
 	}
 
-	work = asn1_encode_integer(work, end_work, options->keyhandle);
+	work = asn1_encode_integer(work, end_work, private->keyhandle);
 	work = asn1_encode_octet_string(work, end_work, pub, pub_len);
 	work = asn1_encode_octet_string(work, end_work, priv, priv_len);
 
@@ -102,6 +103,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
 			   u8 **buf)
 {
+	struct trusted_tpm_options *private = options->private;
 	int ret;
 	struct tpm2_key_context ctx;
 	u8 *blob;
@@ -121,7 +123,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
 		return -ENOMEM;
 
 	*buf = blob;
-	options->keyhandle = ctx.parent;
+	private->keyhandle = ctx.parent;
 
 	memcpy(blob, ctx.priv, ctx.priv_len);
 	blob += ctx.priv_len;
@@ -233,6 +235,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options)
 {
+	struct trusted_tpm_options *private = options->private;
 	off_t offset = TPM_HEADER_SIZE;
 	struct tpm_buf buf, sized;
 	int blob_len = 0;
@@ -240,11 +243,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	u32 flags;
 	int rc;
 
-	hash = tpm2_find_hash_alg(options->hash);
+	hash = tpm2_find_hash_alg(private->hash);
 	if (hash < 0)
 		return hash;
 
-	if (!options->keyhandle)
+	if (!private->keyhandle)
 		return -EINVAL;
 
 	rc = tpm_try_get_ops(chip);
@@ -268,18 +271,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out_put;
 	}
 
-	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+	rc = tpm_buf_append_name(chip, &buf, private->keyhandle, NULL);
 	if (rc)
 		goto out;
 
 	tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
-				    options->keyauth, TPM_DIGEST_SIZE);
+				    private->keyauth, TPM_DIGEST_SIZE);
 
 	/* sensitive */
-	tpm_buf_append_u16(&sized, options->blobauth_len);
+	tpm_buf_append_u16(&sized, private->blobauth_len);
 
-	if (options->blobauth_len)
-		tpm_buf_append(&sized, options->blobauth, options->blobauth_len);
+	if (private->blobauth_len)
+		tpm_buf_append(&sized, private->blobauth,
+			       private->blobauth_len);
 
 	tpm_buf_append_u16(&sized, payload->key_len);
 	tpm_buf_append(&sized, payload->key, payload->key_len);
@@ -292,14 +296,15 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 
 	/* key properties */
 	flags = 0;
-	flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
+	flags |= private->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
 	flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT);
 	tpm_buf_append_u32(&sized, flags);
 
 	/* policy */
-	tpm_buf_append_u16(&sized, options->policydigest_len);
-	if (options->policydigest_len)
-		tpm_buf_append(&sized, options->policydigest, options->policydigest_len);
+	tpm_buf_append_u16(&sized, private->policydigest_len);
+	if (private->policydigest_len)
+		tpm_buf_append(&sized, private->policydigest,
+			       private->policydigest_len);
 
 	/* public parameters */
 	tpm_buf_append_u16(&sized, TPM_ALG_NULL);
@@ -373,6 +378,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 			 u32 *blob_handle)
 {
 	u8 *blob_ref __free(kfree) = NULL;
+	struct trusted_tpm_options *private = options->private;
 	struct tpm_buf buf;
 	unsigned int private_len;
 	unsigned int public_len;
@@ -392,7 +398,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	}
 
 	/* new format carries keyhandle but old format doesn't */
-	if (!options->keyhandle)
+	if (!private->keyhandle)
 		return -EINVAL;
 
 	/* must be big enough for at least the two be16 size counts */
@@ -433,11 +439,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		return rc;
 	}
 
-	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+	rc = tpm_buf_append_name(chip, &buf, private->keyhandle, NULL);
 	if (rc)
 		goto out;
 
-	tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
+	tpm_buf_append_hmac_session(chip, &buf, 0, private->keyauth,
 				    TPM_DIGEST_SIZE);
 
 	tpm_buf_append(&buf, blob, blob_len);
@@ -481,6 +487,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			   struct trusted_key_options *options,
 			   u32 blob_handle)
 {
+	struct trusted_tpm_options *private = options->private;
 	struct tpm_header *head;
 	struct tpm_buf buf;
 	u16 data_len;
@@ -502,10 +509,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	if (rc)
 		goto out;
 
-	if (!options->policyhandle) {
+	if (!private->policyhandle) {
 		tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
-					    options->blobauth,
-					    options->blobauth_len);
+					    private->blobauth,
+					    private->blobauth_len);
 	} else {
 		/*
 		 * FIXME: The policy session was generated outside the
@@ -518,9 +525,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 		 * could repeat our actions with the exfiltrated
 		 * password.
 		 */
-		tpm2_buf_append_auth(&buf, options->policyhandle,
+		tpm2_buf_append_auth(&buf, private->policyhandle,
 				     NULL /* nonce */, 0, 0,
-				     options->blobauth, options->blobauth_len);
+				     private->blobauth, private->blobauth_len);
 		if (tpm2_chip_auth(chip)) {
 			tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
 		} else  {
-- 
2.43.0

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

[Jarkko Sakkinen]

On Sat, Feb 21, 2026 at 12:04:25AM +0530, Srish Srinivasan wrote:
> The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> for logging debug information.
> 
> Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
> Remove TPM_DEBUG.
> 
> No functional change intended.
> 
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index c865c97aa1b4..216caef97ffc 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -46,28 +46,25 @@ enum {
>  	SRK_keytype = 4
>  };
>  
> -#define TPM_DEBUG 0
> -
> -#if TPM_DEBUG
>  static inline void dump_options(struct trusted_key_options *o)
>  {
> -	pr_info("sealing key type %d\n", o->keytype);
> -	pr_info("sealing key handle %0X\n", o->keyhandle);
> -	pr_info("pcrlock %d\n", o->pcrlock);
> -	pr_info("pcrinfo %d\n", o->pcrinfo_len);
> -	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> +	pr_debug("sealing key type %d\n", o->keytype);
> +	pr_debug("sealing key handle %0X\n", o->keyhandle);
> +	pr_debug("pcrlock %d\n", o->pcrlock);
> +	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> +	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
>  		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
>  }
>  
>  static inline void dump_sess(struct osapsess *s)
>  {
> -	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> +	print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
>  		       16, 1, &s->handle, 4, 0);
> -	pr_info("secret:\n");
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> +	pr_debug("secret:\n");
> +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
>  		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> -	pr_info("trusted-key: enonce:\n");
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> +	pr_debug("trusted-key: enonce:\n");
> +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
>  		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
>  }
>  
> @@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
>  {
>  	int len;
>  
> -	pr_info("\ntpm buffer\n");
> +	pr_debug("\ntpm buffer\n");
>  	len = LOAD32(buf, TPM_SIZE_OFFSET);
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> -}
> -#else
> -static inline void dump_options(struct trusted_key_options *o)
> -{
> -}
> -
> -static inline void dump_sess(struct osapsess *s)
> -{
> -}
> -
> -static inline void dump_tpm_buf(unsigned char *buf)
> -{
> +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
>  }
> -#endif
>  
>  static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
>  		       unsigned int keylen, ...)
> -- 
> 2.43.0
>

Applied.


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v2 2/2] keys/trusted_keys: move TPM-specific fields into trusted_tpm_options

[Jarkko Sakkinen]

On Sat, Feb 21, 2026 at 12:04:26AM +0530, Srish Srinivasan wrote:
> The trusted_key_options struct contains TPM-specific fields (keyhandle,
> keyauth, blobauth_len, blobauth, pcrinfo_len, pcrinfo, pcrlock, hash,
> policydigest_len, policydigest, and policyhandle). This leads to the
> accumulation of backend-specific fields in the generic options structure.
> 
> Define trusted_tpm_options structure and move the TPM-specific fields
> there. Store a pointer to trusted_tpm_options in trusted_key_options's
> private.
> 
> No functional change intended.
> 
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  include/keys/trusted-type.h               | 11 ---
>  include/keys/trusted_tpm.h                | 14 ++++
>  security/keys/trusted-keys/trusted_tpm1.c | 95 ++++++++++++++---------
>  security/keys/trusted-keys/trusted_tpm2.c | 51 ++++++------
>  4 files changed, 102 insertions(+), 69 deletions(-)
> 
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 03527162613f..b80f250305b8 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -39,17 +39,6 @@ struct trusted_key_payload {
>  
>  struct trusted_key_options {
>  	uint16_t keytype;
> -	uint32_t keyhandle;
> -	unsigned char keyauth[TPM_DIGEST_SIZE];
> -	uint32_t blobauth_len;
> -	unsigned char blobauth[TPM_DIGEST_SIZE];
> -	uint32_t pcrinfo_len;
> -	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
> -	int pcrlock;
> -	uint32_t hash;
> -	uint32_t policydigest_len;
> -	unsigned char policydigest[MAX_DIGEST_SIZE];
> -	uint32_t policyhandle;
>  	void *private;
>  };
>  
> diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> index 0fadc6a4f166..355ebd36cbfd 100644
> --- a/include/keys/trusted_tpm.h
> +++ b/include/keys/trusted_tpm.h
> @@ -7,6 +7,20 @@
>  
>  extern struct trusted_key_ops trusted_key_tpm_ops;
>  
> +struct trusted_tpm_options {
> +	uint32_t keyhandle;
> +	unsigned char keyauth[TPM_DIGEST_SIZE];
> +	uint32_t blobauth_len;
> +	unsigned char blobauth[TPM_DIGEST_SIZE];
> +	uint32_t pcrinfo_len;
> +	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
> +	int pcrlock;
> +	uint32_t hash;
> +	uint32_t policydigest_len;
> +	unsigned char policydigest[MAX_DIGEST_SIZE];
> +	uint32_t policyhandle;
> +};
> +
>  int tpm2_seal_trusted(struct tpm_chip *chip,
>  		      struct trusted_key_payload *payload,
>  		      struct trusted_key_options *options);
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 216caef97ffc..741b1d47d9f8 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -48,12 +48,14 @@ enum {
>  
>  static inline void dump_options(struct trusted_key_options *o)
>  {
> +	struct trusted_tpm_options *private = o->private;
> +
>  	pr_debug("sealing key type %d\n", o->keytype);
> -	pr_debug("sealing key handle %0X\n", o->keyhandle);
> -	pr_debug("pcrlock %d\n", o->pcrlock);
> -	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> +	pr_debug("sealing key handle %0X\n", private->keyhandle);
> +	pr_debug("pcrlock %d\n", private->pcrlock);
> +	pr_debug("pcrinfo %d\n", private->pcrinfo_len);
>  	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
> -		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> +		       16, 1, private->pcrinfo, private->pcrinfo_len, 0);
>  }
>  
>  static inline void dump_sess(struct osapsess *s)
> @@ -609,6 +611,7 @@ static int tpm_unseal(struct tpm_buf *tb,
>  static int key_seal(struct trusted_key_payload *p,
>  		    struct trusted_key_options *o)
>  {
> +	struct trusted_tpm_options *private = o->private;
>  	struct tpm_buf tb;
>  	int ret;
>  
> @@ -619,9 +622,10 @@ static int key_seal(struct trusted_key_payload *p,
>  	/* include migratable flag at end of sealed key */
>  	p->key[p->key_len] = p->migratable;
>  
> -	ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth,
> +	ret = tpm_seal(&tb, o->keytype, private->keyhandle, private->keyauth,
>  		       p->key, p->key_len + 1, p->blob, &p->blob_len,
> -		       o->blobauth, o->pcrinfo, o->pcrinfo_len);
> +		       private->blobauth, private->pcrinfo,
> +		       private->pcrinfo_len);
>  	if (ret < 0)
>  		pr_info("srkseal failed (%d)\n", ret);
>  
> @@ -635,6 +639,7 @@ static int key_seal(struct trusted_key_payload *p,
>  static int key_unseal(struct trusted_key_payload *p,
>  		      struct trusted_key_options *o)
>  {
> +	struct trusted_tpm_options *private = o->private;
>  	struct tpm_buf tb;
>  	int ret;
>  
> @@ -642,8 +647,8 @@ static int key_unseal(struct trusted_key_payload *p,
>  	if (ret)
>  		return ret;
>  
> -	ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
> -			 o->blobauth, p->key, &p->key_len);
> +	ret = tpm_unseal(&tb, private->keyhandle, private->keyauth, p->blob,
> +			 p->blob_len, private->blobauth, p->key, &p->key_len);
>  	if (ret < 0)
>  		pr_info("srkunseal failed (%d)\n", ret);
>  	else
> @@ -680,6 +685,7 @@ static const match_table_t key_tokens = {
>  static int getoptions(char *c, struct trusted_key_payload *pay,
>  		      struct trusted_key_options *opt)
>  {
> +	struct trusted_tpm_options *private = opt->private;
>  	substring_t args[MAX_OPT_ARGS];
>  	char *p = c;
>  	int token;
> @@ -695,7 +701,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  	if (tpm2 < 0)
>  		return tpm2;
>  
> -	opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
> +	private->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
>  
>  	if (!c)
>  		return 0;
> @@ -709,11 +715,11 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  
>  		switch (token) {
>  		case Opt_pcrinfo:
> -			opt->pcrinfo_len = strlen(args[0].from) / 2;
> -			if (opt->pcrinfo_len > MAX_PCRINFO_SIZE)
> +			private->pcrinfo_len = strlen(args[0].from) / 2;
> +			if (private->pcrinfo_len > MAX_PCRINFO_SIZE)
>  				return -EINVAL;
> -			res = hex2bin(opt->pcrinfo, args[0].from,
> -				      opt->pcrinfo_len);
> +			res = hex2bin(private->pcrinfo, args[0].from,
> +				      private->pcrinfo_len);
>  			if (res < 0)
>  				return -EINVAL;
>  			break;
> @@ -722,12 +728,12 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  			if (res < 0)
>  				return -EINVAL;
>  			opt->keytype = SEAL_keytype;
> -			opt->keyhandle = handle;
> +			private->keyhandle = handle;
>  			break;
>  		case Opt_keyauth:
>  			if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE)
>  				return -EINVAL;
> -			res = hex2bin(opt->keyauth, args[0].from,
> +			res = hex2bin(private->keyauth, args[0].from,
>  				      SHA1_DIGEST_SIZE);
>  			if (res < 0)
>  				return -EINVAL;
> @@ -738,21 +744,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  			 * hex strings.  TPM 2.0 authorizations are simple
>  			 * passwords (although it can take a hash as well)
>  			 */
> -			opt->blobauth_len = strlen(args[0].from);
> +			private->blobauth_len = strlen(args[0].from);
>  
> -			if (opt->blobauth_len == 2 * TPM_DIGEST_SIZE) {
> -				res = hex2bin(opt->blobauth, args[0].from,
> +			if (private->blobauth_len == 2 * TPM_DIGEST_SIZE) {
> +				res = hex2bin(private->blobauth, args[0].from,
>  					      TPM_DIGEST_SIZE);
>  				if (res < 0)
>  					return -EINVAL;
>  
> -				opt->blobauth_len = TPM_DIGEST_SIZE;
> +				private->blobauth_len = TPM_DIGEST_SIZE;
>  				break;
>  			}
>  
> -			if (tpm2 && opt->blobauth_len <= sizeof(opt->blobauth)) {
> -				memcpy(opt->blobauth, args[0].from,
> -				       opt->blobauth_len);
> +			if (tpm2 &&
> +			    private->blobauth_len <=
> +			    sizeof(private->blobauth)) {
> +				memcpy(private->blobauth, args[0].from,
> +				       private->blobauth_len);
>  				break;
>  			}
>  
> @@ -770,14 +778,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  			res = kstrtoul(args[0].from, 10, &lock);
>  			if (res < 0)
>  				return -EINVAL;
> -			opt->pcrlock = lock;
> +			private->pcrlock = lock;
>  			break;
>  		case Opt_hash:
>  			if (test_bit(Opt_policydigest, &token_mask))
>  				return -EINVAL;
>  			for (i = 0; i < HASH_ALGO__LAST; i++) {
>  				if (!strcmp(args[0].from, hash_algo_name[i])) {
> -					opt->hash = i;
> +					private->hash = i;
>  					break;
>  				}
>  			}
> @@ -789,14 +797,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  			}
>  			break;
>  		case Opt_policydigest:
> -			digest_len = hash_digest_size[opt->hash];
> +			digest_len = hash_digest_size[private->hash];
>  			if (!tpm2 || strlen(args[0].from) != (2 * digest_len))
>  				return -EINVAL;
> -			res = hex2bin(opt->policydigest, args[0].from,
> +			res = hex2bin(private->policydigest, args[0].from,
>  				      digest_len);
>  			if (res < 0)
>  				return -EINVAL;
> -			opt->policydigest_len = digest_len;
> +			private->policydigest_len = digest_len;
>  			break;
>  		case Opt_policyhandle:
>  			if (!tpm2)
> @@ -804,7 +812,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  			res = kstrtoul(args[0].from, 16, &handle);
>  			if (res < 0)
>  				return -EINVAL;
> -			opt->policyhandle = handle;
> +			private->policyhandle = handle;
>  			break;
>  		default:
>  			return -EINVAL;
> @@ -815,6 +823,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  
>  static struct trusted_key_options *trusted_options_alloc(void)
>  {
> +	struct trusted_tpm_options *private;
>  	struct trusted_key_options *options;
>  	int tpm2;
>  
> @@ -827,14 +836,23 @@ static struct trusted_key_options *trusted_options_alloc(void)
>  		/* set any non-zero defaults */
>  		options->keytype = SRK_keytype;
>  
> -		if (!tpm2)
> -			options->keyhandle = SRKHANDLE;
> +		private = kzalloc(sizeof(*private), GFP_KERNEL);
> +		if (!private) {
> +			kfree_sensitive(options);
> +			options = NULL;
> +		} else {
> +			if (!tpm2)
> +				private->keyhandle = SRKHANDLE;
> +
> +			options->private = private;
> +		}
>  	}
>  	return options;
>  }
>  
>  static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
>  {
> +	struct trusted_tpm_options *private = NULL;
>  	struct trusted_key_options *options = NULL;
>  	int ret = 0;
>  	int tpm2;
> @@ -852,7 +870,8 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
>  		goto out;
>  	dump_options(options);
>  
> -	if (!options->keyhandle && !tpm2) {
> +	private = options->private;
> +	if (!private->keyhandle && !tpm2) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -866,20 +885,22 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
>  		goto out;
>  	}
>  
> -	if (options->pcrlock) {
> -		ret = pcrlock(options->pcrlock);
> +	if (private->pcrlock) {
> +		ret = pcrlock(private->pcrlock);
>  		if (ret < 0) {
>  			pr_info("pcrlock failed (%d)\n", ret);
>  			goto out;
>  		}
>  	}
>  out:
> +	kfree_sensitive(options->private);
>  	kfree_sensitive(options);
>  	return ret;
>  }
>  
>  static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
>  {
> +	struct trusted_tpm_options *private = NULL;
>  	struct trusted_key_options *options = NULL;
>  	int ret = 0;
>  	int tpm2;
> @@ -897,7 +918,8 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
>  		goto out;
>  	dump_options(options);
>  
> -	if (!options->keyhandle && !tpm2) {
> +	private = options->private;
> +	if (!private->keyhandle && !tpm2) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -909,14 +931,15 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
>  	if (ret < 0)
>  		pr_info("key_unseal failed (%d)\n", ret);
>  
> -	if (options->pcrlock) {
> -		ret = pcrlock(options->pcrlock);
> +	if (private->pcrlock) {
> +		ret = pcrlock(private->pcrlock);
>  		if (ret < 0) {
>  			pr_info("pcrlock failed (%d)\n", ret);
>  			goto out;
>  		}
>  	}
>  out:
> +	kfree_sensitive(options->private);
>  	kfree_sensitive(options);
>  	return ret;
>  }
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 6340823f8b53..94e01249b921 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -24,6 +24,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>  			   struct trusted_key_options *options,
>  			   u8 *src, u32 len)
>  {
> +	struct trusted_tpm_options *private = options->private;
>  	const int SCRATCH_SIZE = PAGE_SIZE;
>  	u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
>  	u8 *work = scratch, *work1;
> @@ -46,7 +47,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>  	work = asn1_encode_oid(work, end_work, tpm2key_oid,
>  			       asn1_oid_len(tpm2key_oid));
>  
> -	if (options->blobauth_len == 0) {
> +	if (private->blobauth_len == 0) {
>  		unsigned char bool[3], *w = bool;
>  		/* tag 0 is emptyAuth */
>  		w = asn1_encode_boolean(w, w + sizeof(bool), true);
> @@ -69,7 +70,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>  		goto err;
>  	}
>  
> -	work = asn1_encode_integer(work, end_work, options->keyhandle);
> +	work = asn1_encode_integer(work, end_work, private->keyhandle);
>  	work = asn1_encode_octet_string(work, end_work, pub, pub_len);
>  	work = asn1_encode_octet_string(work, end_work, priv, priv_len);
>  
> @@ -102,6 +103,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
>  			   struct trusted_key_options *options,
>  			   u8 **buf)
>  {
> +	struct trusted_tpm_options *private = options->private;
>  	int ret;
>  	struct tpm2_key_context ctx;
>  	u8 *blob;
> @@ -121,7 +123,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
>  		return -ENOMEM;
>  
>  	*buf = blob;
> -	options->keyhandle = ctx.parent;
> +	private->keyhandle = ctx.parent;
>  
>  	memcpy(blob, ctx.priv, ctx.priv_len);
>  	blob += ctx.priv_len;
> @@ -233,6 +235,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  		      struct trusted_key_payload *payload,
>  		      struct trusted_key_options *options)
>  {
> +	struct trusted_tpm_options *private = options->private;
>  	off_t offset = TPM_HEADER_SIZE;
>  	struct tpm_buf buf, sized;
>  	int blob_len = 0;
> @@ -240,11 +243,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  	u32 flags;
>  	int rc;
>  
> -	hash = tpm2_find_hash_alg(options->hash);
> +	hash = tpm2_find_hash_alg(private->hash);
>  	if (hash < 0)
>  		return hash;
>  
> -	if (!options->keyhandle)
> +	if (!private->keyhandle)
>  		return -EINVAL;
>  
>  	rc = tpm_try_get_ops(chip);
> @@ -268,18 +271,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  		goto out_put;
>  	}
>  
> -	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> +	rc = tpm_buf_append_name(chip, &buf, private->keyhandle, NULL);
>  	if (rc)
>  		goto out;
>  
>  	tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> -				    options->keyauth, TPM_DIGEST_SIZE);
> +				    private->keyauth, TPM_DIGEST_SIZE);
>  
>  	/* sensitive */
> -	tpm_buf_append_u16(&sized, options->blobauth_len);
> +	tpm_buf_append_u16(&sized, private->blobauth_len);
>  
> -	if (options->blobauth_len)
> -		tpm_buf_append(&sized, options->blobauth, options->blobauth_len);
> +	if (private->blobauth_len)
> +		tpm_buf_append(&sized, private->blobauth,
> +			       private->blobauth_len);
>  
>  	tpm_buf_append_u16(&sized, payload->key_len);
>  	tpm_buf_append(&sized, payload->key, payload->key_len);
> @@ -292,14 +296,15 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  
>  	/* key properties */
>  	flags = 0;
> -	flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
> +	flags |= private->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
>  	flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT);
>  	tpm_buf_append_u32(&sized, flags);
>  
>  	/* policy */
> -	tpm_buf_append_u16(&sized, options->policydigest_len);
> -	if (options->policydigest_len)
> -		tpm_buf_append(&sized, options->policydigest, options->policydigest_len);
> +	tpm_buf_append_u16(&sized, private->policydigest_len);
> +	if (private->policydigest_len)
> +		tpm_buf_append(&sized, private->policydigest,
> +			       private->policydigest_len);
>  
>  	/* public parameters */
>  	tpm_buf_append_u16(&sized, TPM_ALG_NULL);
> @@ -373,6 +378,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  			 u32 *blob_handle)
>  {
>  	u8 *blob_ref __free(kfree) = NULL;
> +	struct trusted_tpm_options *private = options->private;
>  	struct tpm_buf buf;
>  	unsigned int private_len;
>  	unsigned int public_len;
> @@ -392,7 +398,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  	}
>  
>  	/* new format carries keyhandle but old format doesn't */
> -	if (!options->keyhandle)
> +	if (!private->keyhandle)
>  		return -EINVAL;
>  
>  	/* must be big enough for at least the two be16 size counts */
> @@ -433,11 +439,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  		return rc;
>  	}
>  
> -	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> +	rc = tpm_buf_append_name(chip, &buf, private->keyhandle, NULL);
>  	if (rc)
>  		goto out;
>  
> -	tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
> +	tpm_buf_append_hmac_session(chip, &buf, 0, private->keyauth,
>  				    TPM_DIGEST_SIZE);
>  
>  	tpm_buf_append(&buf, blob, blob_len);
> @@ -481,6 +487,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  			   struct trusted_key_options *options,
>  			   u32 blob_handle)
>  {
> +	struct trusted_tpm_options *private = options->private;
>  	struct tpm_header *head;
>  	struct tpm_buf buf;
>  	u16 data_len;
> @@ -502,10 +509,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  	if (rc)
>  		goto out;
>  
> -	if (!options->policyhandle) {
> +	if (!private->policyhandle) {
>  		tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
> -					    options->blobauth,
> -					    options->blobauth_len);
> +					    private->blobauth,
> +					    private->blobauth_len);
>  	} else {
>  		/*
>  		 * FIXME: The policy session was generated outside the
> @@ -518,9 +525,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  		 * could repeat our actions with the exfiltrated
>  		 * password.
>  		 */
> -		tpm2_buf_append_auth(&buf, options->policyhandle,
> +		tpm2_buf_append_auth(&buf, private->policyhandle,
>  				     NULL /* nonce */, 0, 0,
> -				     options->blobauth, options->blobauth_len);
> +				     private->blobauth, private->blobauth_len);
>  		if (tpm2_chip_auth(chip)) {
>  			tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
>  		} else  {
> -- 
> 2.43.0
> 

Applied.


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

[Nayna Jain]

On 2/20/26 1:34 PM, Srish Srinivasan wrote:
> The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> for logging debug information.
>
> Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
> Remove TPM_DEBUG.
>
> No functional change intended.
There is functional change here.  This change allows secret and nonce in 
the function dump_sess() to be logged to kernel logs when dynamic debug 
is enabled. Previously, it was possible only in the debug builds and not 
the production builds at runtime. With this change, it is always there 
in production build. This can result in possible attack.

Instead of doing this change, I think add a comment to prevent this sort 
of change in the future.

Thanks & Regards,

     - Nayna

>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>   security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
>   1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index c865c97aa1b4..216caef97ffc 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -46,28 +46,25 @@ enum {
>   	SRK_keytype = 4
>   };
>   
> -#define TPM_DEBUG 0
> -
> -#if TPM_DEBUG
>   static inline void dump_options(struct trusted_key_options *o)
>   {
> -	pr_info("sealing key type %d\n", o->keytype);
> -	pr_info("sealing key handle %0X\n", o->keyhandle);
> -	pr_info("pcrlock %d\n", o->pcrlock);
> -	pr_info("pcrinfo %d\n", o->pcrinfo_len);
> -	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> +	pr_debug("sealing key type %d\n", o->keytype);
> +	pr_debug("sealing key handle %0X\n", o->keyhandle);
> +	pr_debug("pcrlock %d\n", o->pcrlock);
> +	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> +	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
>   		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
>   }
>   
>   static inline void dump_sess(struct osapsess *s)
>   {
> -	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> +	print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
>   		       16, 1, &s->handle, 4, 0);
> -	pr_info("secret:\n");
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> +	pr_debug("secret:\n");
> +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
>   		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> -	pr_info("trusted-key: enonce:\n");
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> +	pr_debug("trusted-key: enonce:\n");
> +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
>   		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
>   }
>   
> @@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
>   {
>   	int len;
>   
> -	pr_info("\ntpm buffer\n");
> +	pr_debug("\ntpm buffer\n");
>   	len = LOAD32(buf, TPM_SIZE_OFFSET);
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> -}
> -#else
> -static inline void dump_options(struct trusted_key_options *o)
> -{
> -}
> -
> -static inline void dump_sess(struct osapsess *s)
> -{
> -}
> -
> -static inline void dump_tpm_buf(unsigned char *buf)
> -{
> +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
>   }
> -#endif
>   
>   static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
>   		       unsigned int keylen, ...)

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

[Srish Srinivasan]

On 3/10/26 4:15 AM, Nayna Jain wrote:
>
> On 2/20/26 1:34 PM, Srish Srinivasan wrote:
>> The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
>> for logging debug information.
>>
>> Replace pr_info() with pr_debug(), and use KERN_DEBUG for 
>> print_hex_dump().
>> Remove TPM_DEBUG.
>>
>> No functional change intended.
> There is functional change here.  This change allows secret and nonce 
> in the function dump_sess() to be logged to kernel logs when dynamic 
> debug is enabled. Previously, it was possible only in the debug builds 
> and not the production builds at runtime. With this change, it is 
> always there in production build. This can result in possible attack.


Hi Jarkko,
Could you please let us know your thoughts on this one?

And Nayna,
thanks for bringing this up.

thanks,
Srish.


>
> Instead of doing this change, I think add a comment to prevent this 
> sort of change in the future.
>
> Thanks & Regards,
>
>     - Nayna
>
>>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>>   security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
>>   1 file changed, 12 insertions(+), 28 deletions(-)
>>
>> diff --git a/security/keys/trusted-keys/trusted_tpm1.c 
>> b/security/keys/trusted-keys/trusted_tpm1.c
>> index c865c97aa1b4..216caef97ffc 100644
>> --- a/security/keys/trusted-keys/trusted_tpm1.c
>> +++ b/security/keys/trusted-keys/trusted_tpm1.c
>> @@ -46,28 +46,25 @@ enum {
>>       SRK_keytype = 4
>>   };
>>   -#define TPM_DEBUG 0
>> -
>> -#if TPM_DEBUG
>>   static inline void dump_options(struct trusted_key_options *o)
>>   {
>> -    pr_info("sealing key type %d\n", o->keytype);
>> -    pr_info("sealing key handle %0X\n", o->keyhandle);
>> -    pr_info("pcrlock %d\n", o->pcrlock);
>> -    pr_info("pcrinfo %d\n", o->pcrinfo_len);
>> -    print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
>> +    pr_debug("sealing key type %d\n", o->keytype);
>> +    pr_debug("sealing key handle %0X\n", o->keyhandle);
>> +    pr_debug("pcrlock %d\n", o->pcrlock);
>> +    pr_debug("pcrinfo %d\n", o->pcrinfo_len);
>> +    print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
>>                  16, 1, o->pcrinfo, o->pcrinfo_len, 0);
>>   }
>>     static inline void dump_sess(struct osapsess *s)
>>   {
>> -    print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
>> +    print_hex_dump(KERN_DEBUG, "trusted-key: handle ", 
>> DUMP_PREFIX_NONE,
>>                  16, 1, &s->handle, 4, 0);
>> -    pr_info("secret:\n");
>> -    print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
>> +    pr_debug("secret:\n");
>> +    print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
>>                  16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
>> -    pr_info("trusted-key: enonce:\n");
>> -    print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
>> +    pr_debug("trusted-key: enonce:\n");
>> +    print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
>>                  16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
>>   }
>>   @@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char 
>> *buf)
>>   {
>>       int len;
>>   -    pr_info("\ntpm buffer\n");
>> +    pr_debug("\ntpm buffer\n");
>>       len = LOAD32(buf, TPM_SIZE_OFFSET);
>> -    print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 
>> 0);
>> -}
>> -#else
>> -static inline void dump_options(struct trusted_key_options *o)
>> -{
>> -}
>> -
>> -static inline void dump_sess(struct osapsess *s)
>> -{
>> -}
>> -
>> -static inline void dump_tpm_buf(unsigned char *buf)
>> -{
>> +    print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, 
>> len, 0);
>>   }
>> -#endif
>>     static int TSS_rawhmac(unsigned char *digest, const unsigned char 
>> *key,
>>                  unsigned int keylen, ...)

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

[Jarkko Sakkinen]

On Mon, Mar 09, 2026 at 06:45:23PM -0400, Nayna Jain wrote:
> 
> On 2/20/26 1:34 PM, Srish Srinivasan wrote:
> > The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> > for logging debug information.
> > 
> > Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
> > Remove TPM_DEBUG.
> > 
> > No functional change intended.
> There is functional change here.  This change allows secret and nonce in the
> function dump_sess() to be logged to kernel logs when dynamic debug is
> enabled. Previously, it was possible only in the debug builds and not the
> production builds at runtime. With this change, it is always there in
> production build. This can result in possible attack.

Good catch, thank you. It's in my master branch still (not in -next).

TPM_DEBUG should be removed in all cases. If you really want to read
a secret, use tracing tools.

This only proves that the print should exist or should be a constant
value, or overwritten same length value.


> Instead of doing this change, I think add a comment to prevent this sort of
> change in the future.
> 
> Thanks & Regards,
> 
>     - Nayna
> 
> > 
> > Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >   security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
> >   1 file changed, 12 insertions(+), 28 deletions(-)
> > 
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index c865c97aa1b4..216caef97ffc 100644
> > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > @@ -46,28 +46,25 @@ enum {
> >   	SRK_keytype = 4
> >   };
> > -#define TPM_DEBUG 0
> > -
> > -#if TPM_DEBUG
> >   static inline void dump_options(struct trusted_key_options *o)
> >   {
> > -	pr_info("sealing key type %d\n", o->keytype);
> > -	pr_info("sealing key handle %0X\n", o->keyhandle);
> > -	pr_info("pcrlock %d\n", o->pcrlock);
> > -	pr_info("pcrinfo %d\n", o->pcrinfo_len);
> > -	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> > +	pr_debug("sealing key type %d\n", o->keytype);
> > +	pr_debug("sealing key handle %0X\n", o->keyhandle);
> > +	pr_debug("pcrlock %d\n", o->pcrlock);
> > +	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> > +	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
> >   		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> >   }
> >   static inline void dump_sess(struct osapsess *s)
> >   {
> > -	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> > +	print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
> >   		       16, 1, &s->handle, 4, 0);
> > -	pr_info("secret:\n");
> > -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> > +	pr_debug("secret:\n");
> > +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
> >   		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> > -	pr_info("trusted-key: enonce:\n");
> > -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> > +	pr_debug("trusted-key: enonce:\n");
> > +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
> >   		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> >   }
> > @@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
> >   {
> >   	int len;
> > -	pr_info("\ntpm buffer\n");
> > +	pr_debug("\ntpm buffer\n");
> >   	len = LOAD32(buf, TPM_SIZE_OFFSET);
> > -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> > -}
> > -#else
> > -static inline void dump_options(struct trusted_key_options *o)
> > -{
> > -}
> > -
> > -static inline void dump_sess(struct osapsess *s)
> > -{
> > -}
> > -
> > -static inline void dump_tpm_buf(unsigned char *buf)
> > -{
> > +	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> >   }
> > -#endif
> >   static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
> >   		       unsigned int keylen, ...)

BR, Jarkko

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

[Jarkko Sakkinen]

On Mon, Mar 23, 2026 at 07:17:51AM +0200, Jarkko Sakkinen wrote:
> On Mon, Mar 09, 2026 at 06:45:23PM -0400, Nayna Jain wrote:
> > 
> > On 2/20/26 1:34 PM, Srish Srinivasan wrote:
> > > The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> > > for logging debug information.
> > > 
> > > Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
> > > Remove TPM_DEBUG.
> > > 
> > > No functional change intended.
> > There is functional change here.  This change allows secret and nonce in the
> > function dump_sess() to be logged to kernel logs when dynamic debug is
> > enabled. Previously, it was possible only in the debug builds and not the
> > production builds at runtime. With this change, it is always there in
> > production build. This can result in possible attack.
> 
> Good catch, thank you. It's in my master branch still (not in -next).
> 
> TPM_DEBUG should be removed in all cases. If you really want to read
> a secret, use tracing tools.
> 
> This only proves that the print should exist or should be a constant
> value, or overwritten same length value.

I dropped the current patches but yeah, a comment "do not touch this,
could be poisonous" won't be an acceptable way to address this.

If you want "some" debug information you can always put F-string or
0-string of same length, so there's options.

BR, Jarkko

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

[Jarkko Sakkinen]

On Tue, Mar 17, 2026 at 08:44:03AM +0530, Srish Srinivasan wrote:
> 
> On 3/10/26 4:15 AM, Nayna Jain wrote:
> > 
> > On 2/20/26 1:34 PM, Srish Srinivasan wrote:
> > > The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> > > for logging debug information.
> > > 
> > > Replace pr_info() with pr_debug(), and use KERN_DEBUG for
> > > print_hex_dump().
> > > Remove TPM_DEBUG.
> > > 
> > > No functional change intended.
> > There is functional change here.  This change allows secret and nonce in
> > the function dump_sess() to be logged to kernel logs when dynamic debug
> > is enabled. Previously, it was possible only in the debug builds and not
> > the production builds at runtime. With this change, it is always there
> > in production build. This can result in possible attack.
> 
> 
> Hi Jarkko,
> Could you please let us know your thoughts on this one?
> 
> And Nayna,
> thanks for bringing this up.

Nayna is absolutely right so I dropped it.

Solution is debatable.

> 
> thanks,
> Srish.

BR, Jarkko