Skip to contentSkip to conversation

SPL: fix printing of image name

PATCH Superseded André Draszik 3 participants 22 messages Oct 3, 2017 – Nov 17, 2017 Library code

lore
Share & tools
lore
#0 lore raw

PATCH 1/5 SPL: fix printing of image name

From: André Draszik <[email protected]>

The maximum length of the name of the image is
obviously not sizeof(), which is just the
ength of a pointer, but IH_NMLEN.

Fixes 62cf11c0921a90c6bd62344f4bc069668e6c698c

("SPL: Limit image name print length")

---
 common/spl/spl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/spl/spl.c +1 -1
index 4afbe97fc1..7c7467ecd2 100644
@@ -154,7 +154,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 		spl_image->os = image_get_os(header);
 		spl_image->name = image_get_name(header);
 		debug("spl: payload image: %.*s load addr: 0x%lx size: %d\n",
-			(int)sizeof(spl_image->name), spl_image->name,
+			IH_NMLEN, spl_image->name,
 			spl_image->load_addr, spl_image->size);
 #else
 		/* LEGACY image not supported */
Signature · 1 line
2.14.2
#1 lore raw

PATCH 2/5 tpm: fix reading of permanent flags

From: André Draszik <[email protected]>

The offset of the permanent flags structure is in a different place in the response compared to what the code is doing, which gives us a completely result.

Fix by replacing hand-crafted code with generic parser infrastructure.

---
 lib/tpm.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
lib/tpm.c +11 -1
index d1cf5a8a16..5659fa5e18 100644
@@ -607,14 +607,24 @@ uint32_t tpm_get_permanent_flags(struct tpm_permanent_flags *pflags)
 		0x0, 0x0, 0x0, 0x4,	/* subcap size */
 		0x0, 0x0, 0x1, 0x8,	/* subcap value */
 	};
+	const size_t data_size_offset = TPM_HEADER_SIZE;
+	const size_t data_offset = TPM_HEADER_SIZE + sizeof (uint32_t);
 	uint8_t response[COMMAND_BUFFER_SIZE];
 	size_t response_length = sizeof(response);
 	uint32_t err;
+	uint32_t data_size;
 
 	err = tpm_sendrecv_command(command, response, &response_length);
 	if (err)
 		return err;
-	memcpy(pflags, response + TPM_HEADER_SIZE, sizeof(*pflags));
+	if (unpack_byte_string(response, response_length, "d",
+			       data_size_offset, &data_size))
+		return TPM_LIB_ERROR;
+	if (data_size < sizeof(*pflags))
+		return TPM_LIB_ERROR;
+	if (unpack_byte_string(response, response_length, "s",
+			       data_offset, pflags, sizeof(*pflags)))
+		return TPM_LIB_ERROR;
 
 	return 0;
 }
Signature · 1 line
2.14.2
#2 lore raw

PATCH 3/5 tpm: add tpm_get_random()

In reply to André Draszik

From: André Draszik <[email protected]>

---
 include/tpm.h | 12 ++++++++++++
 lib/tpm.c     | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
include/tpm.h +12 -0
index f88388f353..2a7528dd48 100644
@@ -651,4 +651,16 @@ uint32_t tpm_flush_specific(uint32_t key_handle, uint32_t resource_type);
 uint32_t tpm_find_key_sha1(const uint8_t auth[20], const uint8_t
 			   pubkey_digest[20], uint32_t *handle);
 #endif /* CONFIG_TPM_LOAD_KEY_BY_SHA1 */
+
+/**
+ * Read random bytes from the TPM RNG. The implementation deals with the fact
+ * that the TPM may legally return fewer bytes than requested by retrying
+ * until @p count bytes have been received.
+ *
+ * @param data		output buffer for the random bytes
+ * @param count		size of output buffer
+ * @return return code of the operation
+ */
+uint32_t tpm_get_random(void *data, uint32_t count);
+
 #endif /* __TPM_H */
lib/tpm.c +43 -0

46 lines not on this page · raw message

Signature · 1 line
2.14.2
#3 lore raw

PATCH 4/5 tpm: add more useful NV storage permission flags

In reply to André Draszik

From: André Draszik <[email protected]>

TPM_NV_PER_PPREAD: physical presence needed for reading
TPM_NV_PER_WRITEDEFINE: persistent write lock by writing size 0
TPM_NV_PER_WRITEALL: write in one go

---
 include/tpm.h | 3 +++
 1 file changed, 3 insertions(+)
include/tpm.h +3 -0
index 2a7528dd48..760d94865c 100644
@@ -84,9 +84,12 @@ enum tpm_capability_areas {
 };
 
 #define TPM_NV_PER_GLOBALLOCK		(1U << 15)
+#define TPM_NV_PER_PPREAD		(1U << 16)
 #define TPM_NV_PER_PPWRITE		(1U << 0)
 #define TPM_NV_PER_READ_STCLEAR		(1U << 31)
 #define TPM_NV_PER_WRITE_STCLEAR	(1U << 14)
+#define TPM_NV_PER_WRITEDEFINE		(1U << 13)
+#define TPM_NV_PER_WRITEALL		(1U << 12)
 
 enum {
 	TPM_PUBEK_SIZE			= 256,
Signature · 1 line
2.14.2
#5 lore raw

PATCH 4/5 tpm: add more useful NV storage permission flags

Quoted 10 lines · André Draszik

From: André Draszik <[email protected]>

TPM_NV_PER_PPREAD: physical presence needed for reading
TPM_NV_PER_WRITEDEFINE: persistent write lock by writing size 0
TPM_NV_PER_WRITEALL: write in one go

---
 include/tpm.h | 3 +++
 1 file changed, 3 insertions(+)

Applied to u-boot-dm thanks!

#6 lore raw

PATCH 5/5 tpm: add more missing va_end()

In reply to André Draszik

From: André Draszik <[email protected]>

While commit 36d35345b1f6 ("tpm: add missing va_end") added some missing calls to va_end(), it missed a few places.

---
 lib/tpm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
lib/tpm.c +4 -1
index 42a6591f81..f461e639e0 100644
@@ -92,6 +92,7 @@ int pack_byte_string(uint8_t *str, size_t size, const char *format, ...)
 			break;
 		default:
 			debug("Couldn't recognize format string\n");
+			va_end(args);
 			return -1;
 		}
 
@@ -170,8 +171,10 @@ int unpack_byte_string(const uint8_t *str, size_t size, const char *format, ...)
 			return -1;
 		}
 
-		if (offset + length > size)
+		if (offset + length > size) {
+			va_end(args);
 			return -1;
+		}
 
 		switch (*format) {
 		case 'b':
Signature · 1 line
2.14.2
#7 lore raw

PATCH v2 1/5 SPL: fix printing of image name

In reply to André Draszik

From: André Draszik <[email protected]>

The maximum length of the name of the image is
obviously not sizeof(), which is just the
length of a pointer, but IH_NMLEN.

Fixes 62cf11c0921a90c6bd62344f4bc069668e6c698c

("SPL: Limit image name print length")

---
 common/spl/spl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/spl/spl.c +1 -1
index 4afbe97fc1..7c7467ecd2 100644
@@ -154,7 +154,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 		spl_image->os = image_get_os(header);
 		spl_image->name = image_get_name(header);
 		debug("spl: payload image: %.*s load addr: 0x%lx size: %d\n",
-			(int)sizeof(spl_image->name), spl_image->name,
+			IH_NMLEN, spl_image->name,
 			spl_image->load_addr, spl_image->size);
 #else
 		/* LEGACY image not supported */
Signature · 1 line
2.14.2
#8 lore raw

PATCH v2 1/5 SPL: fix printing of image name

Hi,

Quoted 10 lines · André Draszik

From: André Draszik <[email protected]>

The maximum length of the name of the image is
obviously not sizeof(), which is just the
length of a pointer, but IH_NMLEN.

Fixes 62cf11c0921a90c6bd62344f4bc069668e6c698c

("SPL: Limit image name print length")

I see these patches haven't been merged yet, is there anything wrong?

Cheers,
Andre'

#9 lore raw

PATCH v2 1/5 SPL: fix printing of image name

Hi Andre,

Quoted 11 lines · André Draszik

Hi,

Quoted 6 lines · André Draszik

From: André Draszik <[email protected]>

The maximum length of the name of the image is
obviously not sizeof(), which is just the
length of a pointer, but IH_NMLEN.

Quoted 6 lines
Quoted 4 lines
Fixes 62cf11c0921a90c6bd62344f4bc069668e6c698c

("SPL: Limit image name print length")

I see these patches haven't been merged yet, is there anything wrong?

I just found them in my queue, but was not on cc.

#10 lore raw

PATCH v2 1/5 SPL: fix printing of image name

Hi Andre,

Quoted 11 lines · André Draszik

Hi,

Quoted 6 lines · André Draszik

From: André Draszik <[email protected]>

The maximum length of the name of the image is
obviously not sizeof(), which is just the
length of a pointer, but IH_NMLEN.

Quoted 6 lines
Quoted 4 lines
Fixes 62cf11c0921a90c6bd62344f4bc069668e6c698c

("SPL: Limit image name print length")

I see these patches haven't been merged yet, is there anything wrong?

I just found them in my queue, but was not on cc.

Applied to u-boot-dm thanks!

#11 lore raw

PATCH v2 2/5 tpm: fix reading of permanent flags

In reply to André Draszik

From: André Draszik <[email protected]>

The offset of the permanent flags structure is in a different place in the response compared to what the code is doing, which gives us a completely useless result.

Fix by replacing hand-crafted code with generic parser infrastructure.

---
 lib/tpm.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
lib/tpm.c +11 -1
index d1cf5a8a16..5659fa5e18 100644
@@ -607,14 +607,24 @@ uint32_t tpm_get_permanent_flags(struct tpm_permanent_flags *pflags)
 		0x0, 0x0, 0x0, 0x4,	/* subcap size */
 		0x0, 0x0, 0x1, 0x8,	/* subcap value */
 	};
+	const size_t data_size_offset = TPM_HEADER_SIZE;
+	const size_t data_offset = TPM_HEADER_SIZE + sizeof (uint32_t);
 	uint8_t response[COMMAND_BUFFER_SIZE];
 	size_t response_length = sizeof(response);
 	uint32_t err;
+	uint32_t data_size;
 
 	err = tpm_sendrecv_command(command, response, &response_length);
 	if (err)
 		return err;
-	memcpy(pflags, response + TPM_HEADER_SIZE, sizeof(*pflags));
+	if (unpack_byte_string(response, response_length, "d",
+			       data_size_offset, &data_size))
+		return TPM_LIB_ERROR;
+	if (data_size < sizeof(*pflags))
+		return TPM_LIB_ERROR;
+	if (unpack_byte_string(response, response_length, "s",
+			       data_offset, pflags, sizeof(*pflags)))
+		return TPM_LIB_ERROR;
 
 	return 0;
 }
Signature · 1 line
2.14.2
#12 lore raw

PATCH v2 2/5 tpm: fix reading of permanent flags

Quoted 6 lines · André Draszik

From: André Draszik <[email protected]>

The offset of the permanent flags structure is in a different place in the response compared to what the code is doing, which gives us a completely useless result.

Quoted 7 lines

Fix by replacing hand-crafted code with generic parser infrastructure.

---
 lib/tpm.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
#13 lore raw

PATCH v2 2/5 tpm: fix reading of permanent flags

Quoted 6 lines · André Draszik

From: André Draszik <[email protected]>

The offset of the permanent flags structure is in a different place in the response compared to what the code is doing, which gives us a completely useless result.

Quoted 7 lines

Fix by replacing hand-crafted code with generic parser infrastructure.

---
 lib/tpm.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Applied to u-boot-dm thanks!

#14 lore raw

PATCH v2 3/5 tpm: add tpm_get_random()

In reply to André Draszik

From: André Draszik <[email protected]>

---
 include/tpm.h | 12 ++++++++++++
 lib/tpm.c     | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
include/tpm.h +12 -0
index f88388f353..2a7528dd48 100644
@@ -651,4 +651,16 @@ uint32_t tpm_flush_specific(uint32_t key_handle, uint32_t resource_type);
 uint32_t tpm_find_key_sha1(const uint8_t auth[20], const uint8_t
 			   pubkey_digest[20], uint32_t *handle);
 #endif /* CONFIG_TPM_LOAD_KEY_BY_SHA1 */
+
+/**
+ * Read random bytes from the TPM RNG. The implementation deals with the fact
+ * that the TPM may legally return fewer bytes than requested by retrying
+ * until @p count bytes have been received.
+ *
+ * @param data		output buffer for the random bytes
+ * @param count		size of output buffer
+ * @return return code of the operation
+ */
+uint32_t tpm_get_random(void *data, uint32_t count);
+
 #endif /* __TPM_H */
lib/tpm.c +43 -0

46 lines not on this page · raw message

Signature · 1 line
2.14.2
#15 lore raw

PATCH v2 3/5 tpm: add tpm_get_random()

On 3 October 2017 at 09:55, André Draszik <[email protected]> wrote:

Please add a commit message. Also where is this used? Is it dead code?

Quoted 7 lines

From: André Draszik <[email protected]>

---
 include/tpm.h | 12 ++++++++++++
 lib/tpm.c     | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
#16 lore raw

PATCH v2 3/5 tpm: add tpm_get_random()

On 3 October 2017 at 09:55, André Draszik <[email protected]> wrote:

Please add a commit message. Also where is this used? Is it dead code?

Quoted 7 lines

From: André Draszik <[email protected]>

---
 include/tpm.h | 12 ++++++++++++
 lib/tpm.c     | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

Applied to u-boot-dm thanks!

#17 lore raw

PATCH v2 3/5 tpm: add tpm_get_random()

Hi.

Quoted 7 lines · On 17 November 2017 at 08:41, <[email protected]> wrote:

On 3 October 2017 at 09:55, André Draszik <[email protected]> wrote:

Please add a commit message. Also where is this used? Is it dead code?

I applied this with changes:
- added a commit message
- added a cast to fix a build warning on (e.g.) sandbox_noblk

#18 lore raw

PATCH v2 4/5 tpm: add more useful NV storage permission flags

In reply to André Draszik

From: André Draszik <[email protected]>

TPM_NV_PER_PPREAD: physical presence needed for reading
TPM_NV_PER_WRITEDEFINE: persistent write lock by writing size 0
TPM_NV_PER_WRITEALL: write in one go

---
 include/tpm.h | 3 +++
 1 file changed, 3 insertions(+)
include/tpm.h +3 -0
index 2a7528dd48..760d94865c 100644
@@ -84,9 +84,12 @@ enum tpm_capability_areas {
 };
 
 #define TPM_NV_PER_GLOBALLOCK		(1U << 15)
+#define TPM_NV_PER_PPREAD		(1U << 16)
 #define TPM_NV_PER_PPWRITE		(1U << 0)
 #define TPM_NV_PER_READ_STCLEAR		(1U << 31)
 #define TPM_NV_PER_WRITE_STCLEAR	(1U << 14)
+#define TPM_NV_PER_WRITEDEFINE		(1U << 13)
+#define TPM_NV_PER_WRITEALL		(1U << 12)
 
 enum {
 	TPM_PUBEK_SIZE			= 256,
Signature · 1 line
2.14.2
#19 lore raw

PATCH v2 5/5 tpm: add more missing va_end()

In reply to André Draszik

From: André Draszik <[email protected]>

While commit 36d35345b1f6 ("tpm: add missing va_end") added some missing calls to va_end(), it missed a few places.

---
 lib/tpm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
lib/tpm.c +4 -1
index 42a6591f81..f461e639e0 100644
@@ -92,6 +92,7 @@ int pack_byte_string(uint8_t *str, size_t size, const char *format, ...)
 			break;
 		default:
 			debug("Couldn't recognize format string\n");
+			va_end(args);
 			return -1;
 		}
 
@@ -170,8 +171,10 @@ int unpack_byte_string(const uint8_t *str, size_t size, const char *format, ...)
 			return -1;
 		}
 
-		if (offset + length > size)
+		if (offset + length > size) {
+			va_end(args);
 			return -1;
+		}
 
 		switch (*format) {
 		case 'b':
Signature · 1 line
2.14.2