Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] KEYS: trusted: correctly initialize digests and fix locking issue
@ 2019-09-04 18:50 Roberto Sassu
  2019-09-07 19:02 ` Jarkko Sakkinen
  0 siblings, 1 reply; 3+ messages in thread
From: Roberto Sassu @ 2019-09-04 18:50 UTC (permalink / raw)
  To: jarkko.sakkinen, zohar
  Cc: linux-integrity, linux-security-module, keyrings, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch fixes two issues introduced with commit 0b6cf6b97b7e ("tpm: pass
an array of tpm_extend_digest structures to tpm_pcr_extend()").

It initializes the algorithm in init_digests() for trusted keys, and moves
the algorithm check in tpm_pcr_extend() before locks are taken in
tpm_find_get_ops().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Fixes: 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()")
---
 drivers/char/tpm/tpm-interface.c | 8 ++++----
 security/keys/trusted.c          | 5 +++++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1b4f95c13e00..1fffa91fc148 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -316,14 +316,14 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 	int rc;
 	int i;
 
-	chip = tpm_find_get_ops(chip);
-	if (!chip)
-		return -ENODEV;
-
 	for (i = 0; i < chip->nr_allocated_banks; i++)
 		if (digests[i].alg_id != chip->allocated_banks[i].alg_id)
 			return -EINVAL;
 
+	chip = tpm_find_get_ops(chip);
+	if (!chip)
+		return -ENODEV;
+
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 		rc = tpm2_pcr_extend(chip, pcr_idx, digests);
 		tpm_put_ops(chip);
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index ade699131065..1fbd77816610 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1228,11 +1228,16 @@ static int __init trusted_shash_alloc(void)
 
 static int __init init_digests(void)
 {
+	int i;
+
 	digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
 			  GFP_KERNEL);
 	if (!digests)
 		return -ENOMEM;
 
+	for (i = 0; i < chip->nr_allocated_banks; i++)
+		digests[i].alg_id = chip->allocated_banks[i].alg_id;
+
 	return 0;
 }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] KEYS: trusted: correctly initialize digests and fix locking issue
  2019-09-04 18:50 [PATCH v2] KEYS: trusted: correctly initialize digests and fix locking issue Roberto Sassu
@ 2019-09-07 19:02 ` Jarkko Sakkinen
  2019-09-07 19:03   ` Jarkko Sakkinen
  0 siblings, 1 reply; 3+ messages in thread
From: Jarkko Sakkinen @ 2019-09-07 19:02 UTC (permalink / raw)
  To: Roberto Sassu, zohar
  Cc: linux-integrity, linux-security-module, keyrings, linux-kernel,
	silviu.vlasceanu

On Wed, 2019-09-04 at 20:50 +0200, Roberto Sassu wrote:
> This patch fixes two issues introduced with commit 0b6cf6b97b7e ("tpm: pass
> an array of tpm_extend_digest structures to tpm_pcr_extend()").
> 
> It initializes the algorithm in init_digests() for trusted keys, and moves
> the algorithm check in tpm_pcr_extend() before locks are taken in
> tpm_find_get_ops().
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Fixes: 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()")
> ---

The changelog is missing. You should place it right after these three
dashes before diffstat. So, why did you do v2?

I don't see any description of the two issues. The commit messages
goes on explaining right away what this patch does. Would be nice
to have one paragraph describing both of the issues at first before
striving into solutions.

Also, the granularity should be one patch per one issue so this will
require two patches in total.

/Jarkko


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] KEYS: trusted: correctly initialize digests and fix locking issue
  2019-09-07 19:02 ` Jarkko Sakkinen
@ 2019-09-07 19:03   ` Jarkko Sakkinen
  0 siblings, 0 replies; 3+ messages in thread
From: Jarkko Sakkinen @ 2019-09-07 19:03 UTC (permalink / raw)
  To: Roberto Sassu, zohar
  Cc: linux-integrity, linux-security-module, keyrings, linux-kernel,
	silviu.vlasceanu

On Sat, 2019-09-07 at 22:02 +0300, Jarkko Sakkinen wrote:
> On Wed, 2019-09-04 at 20:50 +0200, Roberto Sassu wrote:
> > This patch fixes two issues introduced with commit 0b6cf6b97b7e ("tpm: pass
> > an array of tpm_extend_digest structures to tpm_pcr_extend()").
> > 
> > It initializes the algorithm in init_digests() for trusted keys, and moves
> > the algorithm check in tpm_pcr_extend() before locks are taken in
> > tpm_find_get_ops().
> > 
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > Fixes: 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()")
> > ---
> 
> The changelog is missing. You should place it right after these three
> dashes before diffstat. So, why did you do v2?
> 
> I don't see any description of the two issues. The commit messages
> goes on explaining right away what this patch does. Would be nice
> to have one paragraph describing both of the issues at first before
> striving into solutions.
> 
> Also, the granularity should be one patch per one issue so this will
> require two patches in total.

Actually taking my words back as far as the last paragraph goes. Since
the fixes tag is the same I'm cool with one patch as long as the commit
message describes better what you're doing and why.

/Jarkko



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-04 18:50 [PATCH v2] KEYS: trusted: correctly initialize digests and fix locking issue Roberto Sassu
2019-09-07 19:02 ` Jarkko Sakkinen
2019-09-07 19:03   ` Jarkko Sakkinen

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org linux-security-module@archiver.kernel.org
	public-inbox-index linux-security-module


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/ public-inbox