diff mbox series

[2/5] tpm: Fix event log types for TPM2

Message ID 20190109014847.39980-3-matthewgarrett@google.com (mailing list archive)
State New, archived
Headers show
Series [1/5] tpm: Abstract crypto agile event size calculations | expand

Commit Message

Matthew Garrett Jan. 9, 2019, 1:48 a.m. UTC 
From: Matthew Garrett <mjg59@google.com>

These structs are defined as having variable length members, not fixed
size ones. Fix that up and rename them to more accurately describe the
only safe way they can be used, and fix the code that relied on having
fixed structures.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 drivers/char/tpm/eventlog/tpm2.c |  8 ++++----
 drivers/char/tpm/tpm-interface.c | 32 ++++++++++++++++++++++++--------
 drivers/char/tpm/tpm.h           |  3 ++-
 drivers/char/tpm/tpm2-cmd.c      | 15 ++++++++++++++-
 include/linux/tpm_eventlog.h     | 27 +++++++++++++++++++++------
 5 files changed, 65 insertions(+), 20 deletions(-)

Comments

Jarkko Sakkinen Jan. 16, 2019, 9:42 p.m. UTC | #1 
On Tue, Jan 08, 2019 at 05:48:44PM -0800, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@google.com>
> 
> These structs are defined as having variable length members, not fixed
> size ones. Fix that up and rename them to more accurately describe the
> only safe way they can be used, and fix the code that relied on having
> fixed structures.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>

This collides with Roberto's work. How this must be worked out is to
first merge Roberto's patches and then this patch set should be aligned
with that baseline.

Roberto, are you ready to send soon an updated version? When you do,
please cc the patches to Matthew. Matthew, what you should do is to
review Roberto's patches and give him feedback so that his changes
will work for you. Right now the only "customer" is IMA.

So yeah, this how I would like to prioritize things. I'll hold on for
detailed review for your patches up until Roberto's patches have been
merged because there is just too much intersection.

/Jarkko
Roberto Sassu Jan. 17, 2019, 7:53 a.m. UTC | #2 
On 1/16/2019 10:42 PM, Jarkko Sakkinen wrote:
> On Tue, Jan 08, 2019 at 05:48:44PM -0800, Matthew Garrett wrote:
>> From: Matthew Garrett <mjg59@google.com>
>>
>> These structs are defined as having variable length members, not fixed
>> size ones. Fix that up and rename them to more accurately describe the
>> only safe way they can be used, and fix the code that relied on having
>> fixed structures.
>>
>> Signed-off-by: Matthew Garrett <mjg59@google.com>
> 
> This collides with Roberto's work. How this must be worked out is to
> first merge Roberto's patches and then this patch set should be aligned
> with that baseline.
> 
> Roberto, are you ready to send soon an updated version? When you do,
> please cc the patches to Matthew. Matthew, what you should do is to
> review Roberto's patches and give him feedback so that his changes
> will work for you. Right now the only "customer" is IMA.

I will check your comment about patch 5/5 and reply.

Roberto


> So yeah, this how I would like to prioritize things. I'll hold on for
> detailed review for your patches up until Roberto's patches have been
> merged because there is just too much intersection.
> 
> /Jarkko
>
Jarkko Sakkinen Jan. 18, 2019, 3:15 p.m. UTC | #3 
On Thu, Jan 17, 2019 at 08:53:18AM +0100, Roberto Sassu wrote:
> On 1/16/2019 10:42 PM, Jarkko Sakkinen wrote:
> > On Tue, Jan 08, 2019 at 05:48:44PM -0800, Matthew Garrett wrote:
> > > From: Matthew Garrett <mjg59@google.com>
> > > 
> > > These structs are defined as having variable length members, not fixed
> > > size ones. Fix that up and rename them to more accurately describe the
> > > only safe way they can be used, and fix the code that relied on having
> > > fixed structures.
> > > 
> > > Signed-off-by: Matthew Garrett <mjg59@google.com>
> > 
> > This collides with Roberto's work. How this must be worked out is to
> > first merge Roberto's patches and then this patch set should be aligned
> > with that baseline.
> > 
> > Roberto, are you ready to send soon an updated version? When you do,
> > please cc the patches to Matthew. Matthew, what you should do is to
> > review Roberto's patches and give him feedback so that his changes
> > will work for you. Right now the only "customer" is IMA.
> 
> I will check your comment about patch 5/5 and reply.

Great, thank you. And CC everything to Matthew.

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
index 5023f7f284ef..1343179c0a0e 100644
--- a/drivers/char/tpm/eventlog/tpm2.c
+++ b/drivers/char/tpm/eventlog/tpm2.c
@@ -37,7 +37,7 @@ 
  *
  * Returns size of the event. If it is an invalid event, returns 0.
  */
-static int calc_tpm2_event_size(struct tcg_pcr_event2 *event,
+static int calc_tpm2_event_size(struct tcg_pcr_event2_hdr *event,
 				struct tcg_pcr_event *event_header)
 {
 	return _calc_tpm2_event_size(event, event_header);
@@ -50,7 +50,7 @@  static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
 	void *addr = log->bios_event_log;
 	void *limit = log->bios_event_log_end;
 	struct tcg_pcr_event *event_header;
-	struct tcg_pcr_event2 *event;
+	struct tcg_pcr_event2_hdr *event;
 	size_t size;
 	int i;
 
@@ -91,7 +91,7 @@  static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
 					 loff_t *pos)
 {
 	struct tcg_pcr_event *event_header;
-	struct tcg_pcr_event2 *event;
+	struct tcg_pcr_event2_hdr *event;
 	struct tpm_chip *chip = m->private;
 	struct tpm_bios_log *log = &chip->log;
 	void *limit = log->bios_event_log_end;
@@ -135,7 +135,7 @@  static int tpm2_binary_bios_measurements_show(struct seq_file *m, void *v)
 	struct tpm_chip *chip = m->private;
 	struct tpm_bios_log *log = &chip->log;
 	struct tcg_pcr_event *event_header = log->bios_event_log;
-	struct tcg_pcr_event2 *event = v;
+	struct tcg_pcr_event2_hdr *event = v;
 	void *temp_ptr;
 	size_t size;
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d9439f9abe78..33abe5406d38 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -488,21 +488,37 @@  EXPORT_SYMBOL_GPL(tpm_pcr_read);
 int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
 {
 	int rc;
-	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
 	u32 count = 0;
-	int i;
+	int i, size = 0;
+	void *tmp, *digest_list = NULL;
 
 	chip = tpm_find_get_ops(chip);
 	if (!chip)
 		return -ENODEV;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		memset(digest_list, 0, sizeof(digest_list));
-
-		for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
-			    chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
-			digest_list[i].alg_id = chip->active_banks[i];
-			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
+		for (i = 0; i < ARRAY_SIZE(chip->active_banks); i++) {
+			int digest_size;
+			struct tpm2_digest_hdr *digest;
+
+			if (chip->active_banks[i] == TPM2_ALG_ERROR)
+				continue;
+
+			digest_size = tpm2_digest_size(chip->active_banks[i]);
+			if (digest_size < 0)
+				continue;
+
+			digest = (struct tpm2_digest_hdr *)(digest_list + size);
+			size += sizeof(struct tpm2_digest_hdr) + digest_size;
+
+			tmp = krealloc(digest_list, size, GFP_KERNEL);
+			if (!tmp) {
+				kfree(digest_list);
+				return -ENOMEM;
+			}
+			digest_list = tmp;
+			digest->alg_id = chip->active_banks[i];
+			memcpy(digest->digest, hash, digest_size);
 			count++;
 		}
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f27d1f38a93d..178c47763d0c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -560,7 +560,7 @@  static inline u32 tpm2_rc_value(u32 rc)
 int tpm2_get_timeouts(struct tpm_chip *chip);
 int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
-		    struct tpm2_digest *digests);
+		    struct tpm2_digest_hdr *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
 void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 			    unsigned int flags);
@@ -584,6 +584,7 @@  int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 		       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 		      u32 cc, u8 *buf, size_t *bufsiz);
+int tpm2_digest_size(int algo_id);
 
 int tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a6bec13afa69..c91a0c1ef192 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -226,7 +226,7 @@  struct tpm2_null_auth_area {
  * Return: Same as with tpm_transmit_cmd.
  */
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
-		    struct tpm2_digest *digests)
+		    struct tpm2_digest_hdr *digests)
 {
 	struct tpm_buf buf;
 	struct tpm2_null_auth_area auth_area;
@@ -1027,3 +1027,16 @@  int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
 
 	return -1;
 }
+
+int tpm2_digest_size(int algo_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		if (algo_id != tpm2_hash_map[i].tpm_id)
+			continue;
+		return hash_digest_size[tpm2_hash_map[i].crypto_id];
+	}
+
+	return -1;
+}
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index b43bd2ea8bab..6363c8b08908 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -108,20 +108,35 @@  struct tcg_event_field {
 	u8 event[0];
 } __packed;
 
-struct tpm2_digest {
+struct tpm2_digest_hdr {
 	u16 alg_id;
-	u8 digest[SHA512_DIGEST_SIZE];
+	u8 digest[0];
 } __packed;
 
-struct tcg_pcr_event2 {
+struct tcg_pcr_event2_hdr {
 	u32 pcr_idx;
 	u32 event_type;
 	u32 count;
-	struct tpm2_digest digests[TPM2_ACTIVE_PCR_BANKS];
-	struct tcg_event_field event;
+	struct tpm2_digest_hdr digests[0];
 } __packed;
 
-static inline int _calc_tpm2_event_size(struct tcg_pcr_event2 *event,
+struct tcg_algorithm_size {
+	u16 algorithm_id;
+	u16 algorithm_size;
+};
+
+struct tcg_algorithm_info {
+	u8 signature[16];
+	u32 platform_class;
+	u8 spec_version_minor;
+	u8 spec_version_major;
+	u8 spec_errata;
+	u8 uintn_size;
+	u32 number_of_algorithms;
+	struct tcg_algorithm_size digest_sizes[];
+};
+
+static inline int _calc_tpm2_event_size(struct tcg_pcr_event2_hdr *event,
 					struct tcg_pcr_event *event_header)
 {
 	struct tcg_efi_specid_event *efispecid;