[PATCH 1/2] char/tpm: Remove duplicated lookup table

mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed

* [PATCH 1/2] char/tpm: Remove duplicated lookup table
@ 2012-11-12 22:37 Peter Huewe
  2012-11-12 22:37 ` [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning Peter Huewe
  2012-11-14 22:17 ` [PATCH 1/2] char/tpm: Remove duplicated lookup table Kent Yoder
  0 siblings, 2 replies; 9+ messages in thread
From: Peter Huewe @ 2012-11-12 22:37 UTC (permalink / raw)
  To: Kent Yoder
  Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel, Peter Huewe

The entries in tpm_protected_ordinal_duration are exactly the same as
the first 12 in tpm_ordinal_duration, so we can simply remove this one,
and save some bytes.

This does not change the behavior of the driver.

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
This patch reflects only my opinion and work, 
and is not related to my employer or anybody else.

 drivers/char/tpm/tpm.c |   18 +-----------------
 1 files changed, 1 insertions(+), 17 deletions(-)

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 93211df..9e3c529 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -65,21 +65,6 @@ static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
  * values of the SHORT, MEDIUM, and LONG durations are retrieved
  * from the chip during initialization with a call to tpm_get_timeouts.
  */
-static const u8 tpm_protected_ordinal_duration[TPM_MAX_PROTECTED_ORDINAL] = {
-	TPM_UNDEFINED,		/* 0 */
-	TPM_UNDEFINED,
-	TPM_UNDEFINED,
-	TPM_UNDEFINED,
-	TPM_UNDEFINED,
-	TPM_UNDEFINED,		/* 5 */
-	TPM_UNDEFINED,
-	TPM_UNDEFINED,
-	TPM_UNDEFINED,
-	TPM_UNDEFINED,
-	TPM_SHORT,		/* 10 */
-	TPM_SHORT,
-};
-
 static const u8 tpm_ordinal_duration[TPM_MAX_ORDINAL] = {
 	TPM_UNDEFINED,		/* 0 */
 	TPM_UNDEFINED,
@@ -357,8 +342,7 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 	else if ((ordinal & TPM_PROTECTED_ORDINAL_MASK) <
 		 TPM_MAX_PROTECTED_ORDINAL)
 		duration_idx =
-		    tpm_protected_ordinal_duration[ordinal &
-						   TPM_PROTECTED_ORDINAL_MASK];
+		    tpm_ordinal_duration[ordinal & TPM_PROTECTED_ORDINAL_MASK];
 
 	if (duration_idx != TPM_UNDEFINED)
 		duration = chip->vendor.duration[duration_idx];
-- 
1.7.8.6


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

* [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning.
  2012-11-12 22:37 [PATCH 1/2] char/tpm: Remove duplicated lookup table Peter Huewe
@ 2012-11-12 22:37 ` Peter Huewe
  2012-11-14 22:18   ` Kent Yoder
  2012-11-14 22:17 ` [PATCH 1/2] char/tpm: Remove duplicated lookup table Kent Yoder
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Huewe @ 2012-11-12 22:37 UTC (permalink / raw)
  To: Kent Yoder
  Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel, Peter Huewe

This patch changes the semantics of the duration calculation for an
ordinal, by masking out the higher bits of a tpm command, which specify
whether it's an TPM_PROTECTED_COMMAND, TPM_UNPROTECTED_COMMAND,
TPM_CONNECTION_COMMAND, TPM_CONNECTION_COMMAND, TPM_VENDOR_COMMAND.
(See TPM Main Spec Part 2 Section 17 for details).

For all TPM_PROTECTED and TPM_CONNECTION commands the results are
unchanged.
The TPM_UNPROTECTED commands are TSS commands and thus irrelevant as
they are not sent to the tpm.
For vendor commands the semantics change for ordinals 10 and 11 but
they were probably wrong anyway.

For everything else which has the ordinal set to 10 or 11 the semantics
change as it now uses TPM_UNDEFINED instead of TPM_SHORT which was
probably wrong anyway (but irrelevant as not defined by the standard).

This patch also gets rid of the (false positive) sparse warning:
 drivers/char/tpm/tpm.c:360 tpm_calc_ordinal_duration() error: buffer
 overflow 'tpm_protected_ordinal_duration' 12 <= 243

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/char/tpm/tpm.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 9e3c529..a455d11 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -40,8 +40,9 @@ enum tpm_duration {
 };
 
 #define TPM_MAX_ORDINAL 243
-#define TPM_MAX_PROTECTED_ORDINAL 12
-#define TPM_PROTECTED_ORDINAL_MASK 0xFF
+#define TCS_MAX_ORDINAL 12
+#define TPM_PROTECTED_COMMAND 0x00
+#define TPM_CONNECTION_COMMAND 0x40
 
 /*
  * Bug workaround - some TPM's don't flush the most
@@ -336,13 +337,11 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 {
 	int duration_idx = TPM_UNDEFINED;
 	int duration = 0;
+	u8 category = (ordinal >> 24) & 0xFF;
 
-	if (ordinal < TPM_MAX_ORDINAL)
+	if ((category == TPM_PROTECTED_COMMAND && ordinal < TPM_MAX_ORDINAL) ||
+	    (category == TPM_CONNECTION_COMMAND && ordinal < TCS_MAX_ORDINAL))
 		duration_idx = tpm_ordinal_duration[ordinal];
-	else if ((ordinal & TPM_PROTECTED_ORDINAL_MASK) <
-		 TPM_MAX_PROTECTED_ORDINAL)
-		duration_idx =
-		    tpm_ordinal_duration[ordinal & TPM_PROTECTED_ORDINAL_MASK];
 
 	if (duration_idx != TPM_UNDEFINED)
 		duration = chip->vendor.duration[duration_idx];
-- 
1.7.8.6


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

* Re: [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning.
  2012-11-12 22:37 ` [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning Peter Huewe
@ 2012-11-14 22:18   ` Kent Yoder
  2012-11-14 22:37     ` Peter Hüwe
  0 siblings, 1 reply; 9+ messages in thread
From: Kent Yoder @ 2012-11-14 22:18 UTC (permalink / raw)
  To: Peter Huewe; +Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel

On Mon, Nov 12, 2012 at 11:37:18PM +0100, Peter Huewe wrote:
> This patch changes the semantics of the duration calculation for an
> ordinal, by masking out the higher bits of a tpm command, which specify
> whether it's an TPM_PROTECTED_COMMAND, TPM_UNPROTECTED_COMMAND,
> TPM_CONNECTION_COMMAND, TPM_CONNECTION_COMMAND, TPM_VENDOR_COMMAND.
> (See TPM Main Spec Part 2 Section 17 for details).
> 
> For all TPM_PROTECTED and TPM_CONNECTION commands the results are
> unchanged.
> The TPM_UNPROTECTED commands are TSS commands and thus irrelevant as
> they are not sent to the tpm.
> For vendor commands the semantics change for ordinals 10 and 11 but
> they were probably wrong anyway.
> 
> For everything else which has the ordinal set to 10 or 11 the semantics
> change as it now uses TPM_UNDEFINED instead of TPM_SHORT which was
> probably wrong anyway (but irrelevant as not defined by the standard).
> 
> This patch also gets rid of the (false positive) sparse warning:
>  drivers/char/tpm/tpm.c:360 tpm_calc_ordinal_duration() error: buffer
>  overflow 'tpm_protected_ordinal_duration' 12 <= 243

  I'm not seeing this sparse warning, how did you get it?

Thanks,
Kent

> 
> Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
> ---
>  drivers/char/tpm/tpm.c |   13 ++++++-------
>  1 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index 9e3c529..a455d11 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -40,8 +40,9 @@ enum tpm_duration {
>  };
> 
>  #define TPM_MAX_ORDINAL 243
> -#define TPM_MAX_PROTECTED_ORDINAL 12
> -#define TPM_PROTECTED_ORDINAL_MASK 0xFF
> +#define TCS_MAX_ORDINAL 12
> +#define TPM_PROTECTED_COMMAND 0x00
> +#define TPM_CONNECTION_COMMAND 0x40
> 
>  /*
>   * Bug workaround - some TPM's don't flush the most
> @@ -336,13 +337,11 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
>  {
>  	int duration_idx = TPM_UNDEFINED;
>  	int duration = 0;
> +	u8 category = (ordinal >> 24) & 0xFF;
> 
> -	if (ordinal < TPM_MAX_ORDINAL)
> +	if ((category == TPM_PROTECTED_COMMAND && ordinal < TPM_MAX_ORDINAL) ||
> +	    (category == TPM_CONNECTION_COMMAND && ordinal < TCS_MAX_ORDINAL))
>  		duration_idx = tpm_ordinal_duration[ordinal];
> -	else if ((ordinal & TPM_PROTECTED_ORDINAL_MASK) <
> -		 TPM_MAX_PROTECTED_ORDINAL)
> -		duration_idx =
> -		    tpm_ordinal_duration[ordinal & TPM_PROTECTED_ORDINAL_MASK];
> 
>  	if (duration_idx != TPM_UNDEFINED)
>  		duration = chip->vendor.duration[duration_idx];
> -- 
> 1.7.8.6
> 


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

* Re: [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning.
  2012-11-14 22:18   ` Kent Yoder
@ 2012-11-14 22:37     ` Peter Hüwe
  2012-11-15 16:42       ` Kent Yoder
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Hüwe @ 2012-11-14 22:37 UTC (permalink / raw)
  To: Kent Yoder; +Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel

Am Mittwoch, 14. November 2012, 23:18:26 schrieb Kent Yoder:
> On Mon, Nov 12, 2012 at 11:37:18PM +0100, Peter Huewe wrote:> > 
> > This patch also gets rid of the (false positive) sparse warning:
> >  drivers/char/tpm/tpm.c:360 tpm_calc_ordinal_duration() error: buffer
> >  overflow 'tpm_protected_ordinal_duration' 12 <= 243
> 
>   I'm not seeing this sparse warning, how did you get it?

Oh sorry 
s/sparse/smatch/

It was a smatch warning, not sparse - sorry about that:

/data/data-old/linux-2.6/drivers/char/tpm $ make -C /data/data-old/linux-2.6/ M=`pwd`  C=1 CHECK=smatch
make: Entering directory `/data/data-old/linux-2.6'
  LD      /data/data-old/linux-2.6/drivers/char/tpm/built-in.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm.c
/data/data-old/linux-2.6/drivers/char/tpm/tpm.c:360 tpm_calc_ordinal_duration() error: buffer overflow 'tpm_protected_ordinal_duration' 12 <= 243
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_eventlog.c
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_eventlog.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_acpi.c
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_acpi.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_ppi.c
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_ppi.o
  LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_bios.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_tis.c
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_tis.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_i2c_infineon.c
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_i2c_infineon.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_nsc.c
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_nsc.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_atmel.c
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_atmel.o
  CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_infineon.c
  CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_infineon.o
  Building modules, stage 2.
  MODPOST 7 modules
  CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm.mod.o
  LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm.ko
  CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_atmel.mod.o
  LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_atmel.ko
  CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_bios.mod.o
  LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_bios.ko
  CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_i2c_infineon.mod.o
  LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_i2c_infineon.ko
  CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_infineon.mod.o
  LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_infineon.ko
  CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_nsc.mod.o
  LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_nsc.ko
  CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_tis.mod.o
  LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_tis.ko
make: Leaving directory `/data/data-old/linux-2.6'

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

* Re: [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning.
  2012-11-14 22:37     ` Peter Hüwe
@ 2012-11-15 16:42       ` Kent Yoder
       [not found]         ` <fa7287de-06dd-40b4-9d9a-56fcc8a5f670@email.android.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Kent Yoder @ 2012-11-15 16:42 UTC (permalink / raw)
  To: Peter Hüwe; +Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel

On Wed, Nov 14, 2012 at 11:37:06PM +0100, Peter Hüwe wrote:
> Am Mittwoch, 14. November 2012, 23:18:26 schrieb Kent Yoder:
> > On Mon, Nov 12, 2012 at 11:37:18PM +0100, Peter Huewe wrote:> > 
> > > This patch also gets rid of the (false positive) sparse warning:
> > >  drivers/char/tpm/tpm.c:360 tpm_calc_ordinal_duration() error: buffer
> > >  overflow 'tpm_protected_ordinal_duration' 12 <= 243
> > 
> >   I'm not seeing this sparse warning, how did you get it?
> 
> Oh sorry 
> s/sparse/smatch/
> 
> It was a smatch warning, not sparse - sorry about that:

  Ah, ok. The only other nit I had was that the spec references "TSC"
ordinals, but you had defined "TCS_MAX_ORDINAL". TCS is actually a piece
of TSS in user-space.

Kent

> /data/data-old/linux-2.6/drivers/char/tpm $ make -C /data/data-old/linux-2.6/ M=`pwd`  C=1 CHECK=smatch
> make: Entering directory `/data/data-old/linux-2.6'
>   LD      /data/data-old/linux-2.6/drivers/char/tpm/built-in.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm.c
> /data/data-old/linux-2.6/drivers/char/tpm/tpm.c:360 tpm_calc_ordinal_duration() error: buffer overflow 'tpm_protected_ordinal_duration' 12 <= 243
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_eventlog.c
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_eventlog.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_acpi.c
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_acpi.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_ppi.c
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_ppi.o
>   LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_bios.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_tis.c
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_tis.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_i2c_infineon.c
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_i2c_infineon.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_nsc.c
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_nsc.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_atmel.c
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_atmel.o
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_infineon.c
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_infineon.o
>   Building modules, stage 2.
>   MODPOST 7 modules
>   CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm.mod.o
>   LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm.ko
>   CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_atmel.mod.o
>   LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_atmel.ko
>   CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_bios.mod.o
>   LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_bios.ko
>   CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_i2c_infineon.mod.o
>   LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_i2c_infineon.ko
>   CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_infineon.mod.o
>   LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_infineon.ko
>   CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_nsc.mod.o
>   LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_nsc.ko
>   CC      /data/data-old/linux-2.6/drivers/char/tpm/tpm_tis.mod.o
>   LD [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_tis.ko
> make: Leaving directory `/data/data-old/linux-2.6'
> 


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

[parent not found: <fa7287de-06dd-40b4-9d9a-56fcc8a5f670@email.android.com>]

* Re: [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning.
       [not found]         ` <fa7287de-06dd-40b4-9d9a-56fcc8a5f670@email.android.com>
@ 2012-11-15 21:53           ` Kent Yoder
  2012-11-15 23:31             ` [PATCH v2] char/tpm: simplify duration calculation and eliminate smatch warning Peter Huewe
  0 siblings, 1 reply; 9+ messages in thread
From: Kent Yoder @ 2012-11-15 21:53 UTC (permalink / raw)
  To: Peter Huewe; +Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel

On Thu, Nov 15, 2012 at 06:25:54PM +0100, Peter Huewe wrote:
> Shall I resend a v2?

  Please do - thanks!

> Peter


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

* [PATCH v2] char/tpm: simplify duration calculation and eliminate smatch warning.
  2012-11-15 21:53           ` Kent Yoder
@ 2012-11-15 23:31             ` Peter Huewe
  2012-11-16 17:33               ` Kent Yoder
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Huewe @ 2012-11-15 23:31 UTC (permalink / raw)
  To: Kent Yoder
  Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel, Peter Huewe

This patch changes the semantics of the duration calculation for an
ordinal, by masking out the higher bits of a tpm command, which specify
whether it's an TPM_PROTECTED_COMMAND, TPM_UNPROTECTED_COMMAND,
TPM_CONNECTION_COMMAND, TPM_CONNECTION_COMMAND, TPM_VENDOR_COMMAND.
(See TPM Main Spec Part 2 Section 17 for details).

For all TPM_PROTECTED and TPM_CONNECTION commands the results are
unchanged.
The TPM_UNPROTECTED commands are TSS commands and thus irrelevant as
they are not sent to the tpm.
For vendor commands the semantics change for ordinals 10 and 11 but
they were probably wrong anyway.

For everything else which has the ordinal set to 10 or 11 the semantics
change as it now uses TPM_UNDEFINED instead of TPM_SHORT which was
probably wrong anyway (but irrelevant as not defined by the standard).

This patch also gets rid of the (false positive) smatch warning:
 drivers/char/tpm/tpm.c:360 tpm_calc_ordinal_duration() error: buffer
 overflow 'tpm_protected_ordinal_duration' 12 <= 243

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
v2: Smatch not sparse and TSC instead of TCS ;)


 drivers/char/tpm/tpm.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 9e3c529..0a08af0 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -40,8 +40,9 @@ enum tpm_duration {
 };
 
 #define TPM_MAX_ORDINAL 243
-#define TPM_MAX_PROTECTED_ORDINAL 12
-#define TPM_PROTECTED_ORDINAL_MASK 0xFF
+#define TSC_MAX_ORDINAL 12
+#define TPM_PROTECTED_COMMAND 0x00
+#define TPM_CONNECTION_COMMAND 0x40
 
 /*
  * Bug workaround - some TPM's don't flush the most
@@ -336,13 +337,11 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 {
 	int duration_idx = TPM_UNDEFINED;
 	int duration = 0;
+	u8 category = (ordinal >> 24) & 0xFF;
 
-	if (ordinal < TPM_MAX_ORDINAL)
+	if ((category == TPM_PROTECTED_COMMAND && ordinal < TPM_MAX_ORDINAL) ||
+	    (category == TPM_CONNECTION_COMMAND && ordinal < TSC_MAX_ORDINAL))
 		duration_idx = tpm_ordinal_duration[ordinal];
-	else if ((ordinal & TPM_PROTECTED_ORDINAL_MASK) <
-		 TPM_MAX_PROTECTED_ORDINAL)
-		duration_idx =
-		    tpm_ordinal_duration[ordinal & TPM_PROTECTED_ORDINAL_MASK];
 
 	if (duration_idx != TPM_UNDEFINED)
 		duration = chip->vendor.duration[duration_idx];
-- 
1.7.8.6


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

* Re: [PATCH v2] char/tpm: simplify duration calculation and eliminate smatch warning.
  2012-11-15 23:31             ` [PATCH v2] char/tpm: simplify duration calculation and eliminate smatch warning Peter Huewe
@ 2012-11-16 17:33               ` Kent Yoder
  0 siblings, 0 replies; 9+ messages in thread
From: Kent Yoder @ 2012-11-16 17:33 UTC (permalink / raw)
  To: Peter Huewe; +Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel

On Fri, Nov 16, 2012 at 12:31:29AM +0100, Peter Huewe wrote:
> This patch changes the semantics of the duration calculation for an
> ordinal, by masking out the higher bits of a tpm command, which specify
> whether it's an TPM_PROTECTED_COMMAND, TPM_UNPROTECTED_COMMAND,
> TPM_CONNECTION_COMMAND, TPM_CONNECTION_COMMAND, TPM_VENDOR_COMMAND.
> (See TPM Main Spec Part 2 Section 17 for details).
> 
> For all TPM_PROTECTED and TPM_CONNECTION commands the results are
> unchanged.
> The TPM_UNPROTECTED commands are TSS commands and thus irrelevant as
> they are not sent to the tpm.
> For vendor commands the semantics change for ordinals 10 and 11 but
> they were probably wrong anyway.
> 
> For everything else which has the ordinal set to 10 or 11 the semantics
> change as it now uses TPM_UNDEFINED instead of TPM_SHORT which was
> probably wrong anyway (but irrelevant as not defined by the standard).
> 
> This patch also gets rid of the (false positive) smatch warning:
>  drivers/char/tpm/tpm.c:360 tpm_calc_ordinal_duration() error: buffer
>  overflow 'tpm_protected_ordinal_duration' 12 <= 243
> 
> Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
> ---
> v2: Smatch not sparse and TSC instead of TCS ;)

  Applied. Thanks.  I'm queuing these up at:

  git://github.com/shpedoikal/linux.git tpmdd-11-14-12

Kent

> 
>  drivers/char/tpm/tpm.c |   13 ++++++-------
>  1 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index 9e3c529..0a08af0 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -40,8 +40,9 @@ enum tpm_duration {
>  };
> 
>  #define TPM_MAX_ORDINAL 243
> -#define TPM_MAX_PROTECTED_ORDINAL 12
> -#define TPM_PROTECTED_ORDINAL_MASK 0xFF
> +#define TSC_MAX_ORDINAL 12
> +#define TPM_PROTECTED_COMMAND 0x00
> +#define TPM_CONNECTION_COMMAND 0x40
> 
>  /*
>   * Bug workaround - some TPM's don't flush the most
> @@ -336,13 +337,11 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
>  {
>  	int duration_idx = TPM_UNDEFINED;
>  	int duration = 0;
> +	u8 category = (ordinal >> 24) & 0xFF;
> 
> -	if (ordinal < TPM_MAX_ORDINAL)
> +	if ((category == TPM_PROTECTED_COMMAND && ordinal < TPM_MAX_ORDINAL) ||
> +	    (category == TPM_CONNECTION_COMMAND && ordinal < TSC_MAX_ORDINAL))
>  		duration_idx = tpm_ordinal_duration[ordinal];
> -	else if ((ordinal & TPM_PROTECTED_ORDINAL_MASK) <
> -		 TPM_MAX_PROTECTED_ORDINAL)
> -		duration_idx =
> -		    tpm_ordinal_duration[ordinal & TPM_PROTECTED_ORDINAL_MASK];
> 
>  	if (duration_idx != TPM_UNDEFINED)
>  		duration = chip->vendor.duration[duration_idx];
> -- 
> 1.7.8.6
> 


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

* Re: [PATCH 1/2] char/tpm: Remove duplicated lookup table
  2012-11-12 22:37 [PATCH 1/2] char/tpm: Remove duplicated lookup table Peter Huewe
  2012-11-12 22:37 ` [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning Peter Huewe
@ 2012-11-14 22:17 ` Kent Yoder
  1 sibling, 0 replies; 9+ messages in thread
From: Kent Yoder @ 2012-11-14 22:17 UTC (permalink / raw)
  To: Peter Huewe; +Cc: Marcel Selhorst, Sirrix AG, tpmdd-devel, linux-kernel

On Mon, Nov 12, 2012 at 11:37:17PM +0100, Peter Huewe wrote:
> The entries in tpm_protected_ordinal_duration are exactly the same as
> the first 12 in tpm_ordinal_duration, so we can simply remove this one,
> and save some bytes.
> 
> This does not change the behavior of the driver.
> 
> Signed-off-by: Peter Huewe <peterhuewe@gmx.de>

 Applied, thanks.

Kent

> ---
> This patch reflects only my opinion and work, 
> and is not related to my employer or anybody else.
> 
>  drivers/char/tpm/tpm.c |   18 +-----------------
>  1 files changed, 1 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index 93211df..9e3c529 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -65,21 +65,6 @@ static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
>   * values of the SHORT, MEDIUM, and LONG durations are retrieved
>   * from the chip during initialization with a call to tpm_get_timeouts.
>   */
> -static const u8 tpm_protected_ordinal_duration[TPM_MAX_PROTECTED_ORDINAL] = {
> -	TPM_UNDEFINED,		/* 0 */
> -	TPM_UNDEFINED,
> -	TPM_UNDEFINED,
> -	TPM_UNDEFINED,
> -	TPM_UNDEFINED,
> -	TPM_UNDEFINED,		/* 5 */
> -	TPM_UNDEFINED,
> -	TPM_UNDEFINED,
> -	TPM_UNDEFINED,
> -	TPM_UNDEFINED,
> -	TPM_SHORT,		/* 10 */
> -	TPM_SHORT,
> -};
> -
>  static const u8 tpm_ordinal_duration[TPM_MAX_ORDINAL] = {
>  	TPM_UNDEFINED,		/* 0 */
>  	TPM_UNDEFINED,
> @@ -357,8 +342,7 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
>  	else if ((ordinal & TPM_PROTECTED_ORDINAL_MASK) <
>  		 TPM_MAX_PROTECTED_ORDINAL)
>  		duration_idx =
> -		    tpm_protected_ordinal_duration[ordinal &
> -						   TPM_PROTECTED_ORDINAL_MASK];
> +		    tpm_ordinal_duration[ordinal & TPM_PROTECTED_ORDINAL_MASK];
> 
>  	if (duration_idx != TPM_UNDEFINED)
>  		duration = chip->vendor.duration[duration_idx];
> -- 
> 1.7.8.6
> 


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

end of thread, other threads:[~2012-11-16 17:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-12 22:37 [PATCH 1/2] char/tpm: Remove duplicated lookup table Peter Huewe
2012-11-12 22:37 ` [PATCH 2/2] char/tpm: simplify duration calculation and eliminate sparse warning Peter Huewe
2012-11-14 22:18   ` Kent Yoder
2012-11-14 22:37     ` Peter Hüwe
2012-11-15 16:42       ` Kent Yoder
     [not found]         ` <fa7287de-06dd-40b4-9d9a-56fcc8a5f670@email.android.com>
2012-11-15 21:53           ` Kent Yoder
2012-11-15 23:31             ` [PATCH v2] char/tpm: simplify duration calculation and eliminate smatch warning Peter Huewe
2012-11-16 17:33               ` Kent Yoder
2012-11-14 22:17 ` [PATCH 1/2] char/tpm: Remove duplicated lookup table Kent Yoder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome