[tpmdd-devel,v4,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 <honclo@linux.vnet.ibm.com>
Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_eventlog.c |   78 ++++++++++++++++++++++++++++-----------
 drivers/char/tpm/tpm_eventlog.h |    6 +++
 2 files changed, 62 insertions(+), 22 deletions(-)

Looks better.  Thanks.

Reviewed-by: Ashley Lai <ashley@ahsleylai.com>

Cheers,
Ashley Lai


------------------------------------------------------------------------------

Hi Peter,

Can you please commit the patch in the next open window,
if you accept it as well?  Thanks!


Regards,
Vicky

-------- Forwarded Message --------
From: Ashley Lai <ashley@ashleylai.com>
To: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com>
Cc: tpmdd-devel@lists.sourceforge.net, Peter Huewe <PeterHuewe@gmx.de>,
Ashley Lai <ashley@ashleylai.com>, Vicky Lo <honclo2014@gmail.com>,
linux-kernel@vger.kernel.org, Joy Latten <jmlatten@linux.vnet.ibm.com>
Subject: Re: [PATCH v4 1/2] vTPM: support little endian guests
Date: Thu, 18 Jun 2015 08:23:01 -0500 (CDT)

Looks better.  Thanks.

Reviewed-by: Ashley Lai <ashley@ahsleylai.com>

Cheers,
Ashley Lai



On Wed, 2015-06-17 at 18:17 -0400, Hon Ching(Vicky) Lo wrote:
> 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 <honclo@linux.vnet.ibm.com>
> Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/tpm_eventlog.c |   78 ++++++++++++++++++++++++++++-----------
>  drivers/char/tpm/tpm_eventlog.h |    6 +++
>  2 files changed, 62 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 3a56a13..bd72fb0 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,16 @@ 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 +191,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 +199,8 @@ 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 +209,8 @@ 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 +231,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 +271,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 +280,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,



------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/

Hi Vicky,

sorry for the late reply


> 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 <honclo@linux.vnet.ibm.com>
> Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>

> 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


Why is this macro needed?
shouldn't the be32_to_cpu macro already do correct thing?
Or am I missing something here?


Thanks,
Peter

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/