2015-06-10 01:55:16

by Hon Ching(Vicky) Lo

[permalink] [raw]
Subject: [PATCH v2 1/2] vTPM: support little endian guests

This patch makes the code endianness independent. We defined a
macro do_endian_conversion to apply endianness to raw integers
in the event entries so that they will be displayed properly.
tpm_binary_bios_measurements_show() is modified for the display.

Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
Signed-off-by: Joy Latten <[email protected]>
---
drivers/char/tpm/tpm_eventlog.c | 81 ++++++++++++++++++++++++++++----------
drivers/char/tpm/tpm_eventlog.h | 6 +++
2 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 3a56a13..8689957 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -76,15 +76,25 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
void *addr = log->bios_event_log;
void *limit = log->bios_event_log_end;
struct tcpa_event *event;
+ u32 converted_event_size;
+ u32 converted_event_type;
+

/* read over *pos measurements */
for (i = 0; i < *pos; i++) {
event = addr;

+ converted_event_size =
+ do_endian_conversion(event->event_size);
+ converted_event_type =
+ do_endian_conversion(event->event_type);
+
if ((addr + sizeof(struct tcpa_event)) < limit) {
- if (event->event_type == 0 && event->event_size == 0)
+ if ((converted_event_type == 0) &&
+ (converted_event_size == 0))
return NULL;
- addr += sizeof(struct tcpa_event) + event->event_size;
+ addr += (sizeof(struct tcpa_event) +
+ converted_event_size);
}
}

@@ -94,8 +104,12 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)

event = addr;

- if ((event->event_type == 0 && event->event_size == 0) ||
- ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit))
+ converted_event_size = do_endian_conversion(event->event_size);
+ converted_event_type = do_endian_conversion(event->event_type);
+
+ if (((converted_event_type == 0) && (converted_event_size == 0))
+ || ((addr + sizeof(struct tcpa_event) + converted_event_size)
+ >= limit))
return NULL;

return addr;
@@ -107,8 +121,12 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
struct tcpa_event *event = v;
struct tpm_bios_log *log = m->private;
void *limit = log->bios_event_log_end;
+ u32 converted_event_size;
+ u32 converted_event_type;

- v += sizeof(struct tcpa_event) + event->event_size;
+ converted_event_size = do_endian_conversion(event->event_size);
+
+ v += sizeof(struct tcpa_event) + converted_event_size;

/* now check if current entry is valid */
if ((v + sizeof(struct tcpa_event)) >= limit)
@@ -116,11 +134,11 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,

event = v;

- if (event->event_type == 0 && event->event_size == 0)
- return NULL;
+ converted_event_size = do_endian_conversion(event->event_size);
+ converted_event_type = do_endian_conversion(event->event_type);

- if ((event->event_type == 0 && event->event_size == 0) ||
- ((v + sizeof(struct tcpa_event) + event->event_size) >= limit))
+ if (((converted_event_type == 0) && (converted_event_size == 0)) ||
+ ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit))
return NULL;

(*pos)++;
@@ -140,7 +158,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,
int i, n_len = 0, d_len = 0;
struct tcpa_pc_event *pc_event;

- switch(event->event_type) {
+ switch (do_endian_conversion(event->event_type)) {
case PREBOOT:
case POST_CODE:
case UNUSED:
@@ -156,14 +174,17 @@ static int get_event_name(char *dest, struct tcpa_event *event,
case NONHOST_CODE:
case NONHOST_CONFIG:
case NONHOST_INFO:
- name = tcpa_event_type_strings[event->event_type];
+ name =
+ tcpa_event_type_strings[do_endian_conversion
+ (event->event_type)];
n_len = strlen(name);
break;
case SEPARATOR:
case ACTION:
- if (MAX_TEXT_EVENT > event->event_size) {
+ if (MAX_TEXT_EVENT >
+ do_endian_conversion(event->event_size)) {
name = event_entry;
- n_len = event->event_size;
+ n_len = do_endian_conversion(event->event_size);
}
break;
case EVENT_TAG:
@@ -171,7 +192,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,

/* ToDo Row data -> Base64 */

- switch (pc_event->event_id) {
+ switch (do_endian_conversion(pc_event->event_id)) {
case SMBIOS:
case BIS_CERT:
case CMOS:
@@ -179,7 +200,9 @@ static int get_event_name(char *dest, struct tcpa_event *event,
case OPTION_ROM_EXEC:
case OPTION_ROM_CONFIG:
case S_CRTM_VERSION:
- name = tcpa_pc_event_id_strings[pc_event->event_id];
+ name =
+ tcpa_pc_event_id_strings[do_endian_conversion
+ (pc_event->event_id)];
n_len = strlen(name);
break;
/* hash data */
@@ -188,7 +211,9 @@ static int get_event_name(char *dest, struct tcpa_event *event,
case OPTION_ROM_MICROCODE:
case S_CRTM_CONTENTS:
case POST_CONTENTS:
- name = tcpa_pc_event_id_strings[pc_event->event_id];
+ name =
+ tcpa_pc_event_id_strings[do_endian_conversion
+ (pc_event->event_id)];
n_len = strlen(name);
for (i = 0; i < 20; i++)
d_len += sprintf(&data[2*i], "%02x",
@@ -209,13 +234,24 @@ static int get_event_name(char *dest, struct tcpa_event *event,
static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
{
struct tcpa_event *event = v;
- char *data = v;
+ struct tcpa_event temp_event;
+ char *tempPtr;
int i;

- for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
- seq_putc(m, data[i]);
+ memcpy(&temp_event, event, sizeof(struct tcpa_event));
+
+ /* convert raw integers for endianness */
+ temp_event.pcr_index = do_endian_conversion(event->pcr_index);
+ temp_event.event_type = do_endian_conversion(event->event_type);
+ temp_event.event_size = do_endian_conversion(event->event_size);
+
+ tempPtr = (char *)&temp_event;
+
+ for (i = 0; i < sizeof(struct tcpa_event) + temp_event.event_size; i++)
+ seq_putc(m, tempPtr[i]);

return 0;
+
}

static int tpm_bios_measurements_release(struct inode *inode,
@@ -238,7 +274,7 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
char *eventname;
struct tcpa_event *event = v;
unsigned char *event_entry =
- (unsigned char *) (v + sizeof(struct tcpa_event));
+ (unsigned char *)(v + sizeof(struct tcpa_event));

eventname = kmalloc(MAX_TEXT_EVENT, GFP_KERNEL);
if (!eventname) {
@@ -247,13 +283,14 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
return -EFAULT;
}

- seq_printf(m, "%2d ", event->pcr_index);
+ /* 1st: PCR */
+ seq_printf(m, "%2d ", do_endian_conversion(event->pcr_index));

/* 2nd: SHA1 */
seq_printf(m, "%20phN", event->pcr_value);

/* 3rd: event type identifier */
- seq_printf(m, " %02x", event->event_type);
+ seq_printf(m, " %02x", do_endian_conversion(event->event_type));

len += get_event_name(eventname, event, event_entry);

diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index e7da086..267bfbd 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -6,6 +6,12 @@
#define MAX_TEXT_EVENT 1000 /* Max event string length */
#define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */

+#ifdef CONFIG_PPC64
+#define do_endian_conversion(x) be32_to_cpu(x)
+#else
+#define do_endian_conversion(x) x
+#endif
+
enum bios_platform_class {
BIOS_CLIENT = 0x00,
BIOS_SERVER = 0x01,
--
1.7.1


2015-06-10 01:55:27

by Hon Ching(Vicky) Lo

[permalink] [raw]
Subject: [PATCH v2 2/2] TPM: remove unnecessary little endian conversion

The base pointer for the event log is allocated in the local
kernel (in prom_instantiate_sml()), therefore it is already in
the host's endian byte order and requires no conversion.

The content of the 'basep' pointer in read_log() stores the
base address of the log. This patch ensures that it is correctly
implemented.

Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
Signed-off-by: Joy Latten <[email protected]>
---
drivers/char/tpm/tpm_of.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index c002d1b..62a22ce 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -24,7 +24,7 @@ int read_log(struct tpm_bios_log *log)
{
struct device_node *np;
const u32 *sizep;
- const __be64 *basep;
+ const u64 *basep;

if (log->bios_event_log != NULL) {
pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
@@ -63,7 +63,7 @@ int read_log(struct tpm_bios_log *log)

log->bios_event_log_end = log->bios_event_log + *sizep;

- memcpy(log->bios_event_log, __va(be64_to_cpup(basep)), *sizep);
+ memcpy(log->bios_event_log, __va(*basep), *sizep);

return 0;

--
1.7.1

2015-06-17 01:18:06

by Ashley Lai

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] vTPM: support little endian guests

Just a small comment otherwise it looks good.

On Tue, 9 Jun 2015, Hon Ching(Vicky) Lo wrote:
> case NONHOST_INFO:
> - name = tcpa_event_type_strings[event->event_type];
> + name =
> + tcpa_event_type_strings[do_endian_conversion
> + (event->event_type)];
Not being picky but if it does not exceed 80 characters it looks better
to join the line above.
name = tcpa_event_type_strings[do_endian_conversion
(event->event_type)];

> case POST_CONTENTS:
> - name = tcpa_pc_event_id_strings[pc_event->event_id];
> + name =
> + tcpa_pc_event_id_strings[do_endian_conversion
> + (pc_event->event_id)];
Same as above.

Thanks,
--Ashley Lai

2015-06-17 01:21:06

by Ashley Lai

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] TPM: remove unnecessary little endian conversion

Looks good.

Reviewed-by: Ashley Lai <[email protected]>

Thanks,
--Ashley

On Tue, 9 Jun 2015, Hon Ching(Vicky) Lo wrote:

> The base pointer for the event log is allocated in the local
> kernel (in prom_instantiate_sml()), therefore it is already in
> the host's endian byte order and requires no conversion.
>
> The content of the 'basep' pointer in read_log() stores the
> base address of the log. This patch ensures that it is correctly
> implemented.
>
> Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
> Signed-off-by: Joy Latten <[email protected]>
> ---
> drivers/char/tpm/tpm_of.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index c002d1b..62a22ce 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -24,7 +24,7 @@ int read_log(struct tpm_bios_log *log)
> {
> struct device_node *np;
> const u32 *sizep;
> - const __be64 *basep;
> + const u64 *basep;
>
> if (log->bios_event_log != NULL) {
> pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
> @@ -63,7 +63,7 @@ int read_log(struct tpm_bios_log *log)
>
> log->bios_event_log_end = log->bios_event_log + *sizep;
>
> - memcpy(log->bios_event_log, __va(be64_to_cpup(basep)), *sizep);
> + memcpy(log->bios_event_log, __va(*basep), *sizep);
>
> return 0;
>
> --
> 1.7.1
>
>

2015-06-17 21:19:16

by Hon Ching(Vicky) Lo

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] vTPM: support little endian guests

Hi Ashley,

Ah, good catch. I think I can only join the first two lines (where the
assignments are) and will have to leave the rest splitted. I'll resubmit
this one soon.

Thanks for the review!


Vicky
On Tue, 2015-06-16 at 20:17 -0500, Ashley Lai wrote:
> Just a small comment otherwise it looks good.
>
> On Tue, 9 Jun 2015, Hon Ching(Vicky) Lo wrote:
> > case NONHOST_INFO:
> > - name = tcpa_event_type_strings[event->event_type];
> > + name =
> > + tcpa_event_type_strings[do_endian_conversion
> > + (event->event_type)];
> Not being picky but if it does not exceed 80 characters it looks better
> to join the line above.
> name = tcpa_event_type_strings[do_endian_conversion
> (event->event_type)];
>
> > case POST_CONTENTS:
> > - name = tcpa_pc_event_id_strings[pc_event->event_id];
> > + name =
> > + tcpa_pc_event_id_strings[do_endian_conversion
> > + (pc_event->event_id)];
> Same as above.
>
> Thanks,
> --Ashley Lai
>