linux-kernel - RE: [PATCH resend] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <74A44E99E3274B4CB570415926B37D440F7675@MUCSE501.eu.infineon.com>
Date:	Wed, 21 Nov 2012 08:59:24 +0000
From:	<Peter.Huewe@...ineon.com>
To:	<jgunthorpe@...idianresearch.com>, <key@...ux.vnet.ibm.com>
CC:	<linux-kernel@...r.kernel.org>, <tpmdd-devel@...ts.sourceforge.net>
Subject: RE: [PATCH resend] TPM: Issue TPM_STARTUP at driver load if the TPM
 has not been started

Hi Jason, Hi Kent,
>
>Discussion on this got a bit sidetracked talking about
>suspend/resume.. To be clear, this fixes a real, serious, problem on
>normal embedded cases where the kernel refuses to attach the TPM driver
>at all.
>
>Key, are you happy with this as-is for the next merge window?
>
>This version is rebased and retested to 3.7-rc6. Tested on Atmel
>and Nuvoton LPC TPMs on PPC32.

Sorry I was tied up at work with other stuff, so I couldn't try out your initial patch.
Thanks for resubmitting!

I just gave the new version a run on my beagleboard with our Infineon SLB9635 TT 1.2 Soft I2C TPM
and it seems to work as expected. (Tested with and without previous startup).

Tested-by: Peter Huewe <peter.huewe@...ineon.com>

I just have some minor comments - please see below.


>+++ b/drivers/char/tpm/tpm.c
>+#define TPM_ORD_STARTUP cpu_to_be32(153)
>+#define TPM_ST_CLEAR cpu_to_be16(1)
>+#define TPM_ST_STATE cpu_to_be16(2)
>+#define TPM_ST_DEACTIVATED cpu_to_be16(3)
>+static const struct tpm_input_header tpm_startup_header = {
>+	.tag = TPM_TAG_RQU_COMMAND,
>+	.length = cpu_to_be32(12),
>+	.ordinal = TPM_ORD_STARTUP
>+};
>+
> ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> 		   const char *desc)
> {


Purely cosmetic question, but why did you define this before the tpm_getcap and not tpm_startup?
All the other definitions are made before they are used - so this should perhaps better be moved directly before tpm_startup.
(Maybe we should move out these definitions to a common location? Header?)




>@@ -541,11 +560,27 @@ int tpm_get_timeouts(struct tpm_chip *chip)
> 	tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
> 	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
> 	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
>+	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);

Please use NULL instead of 0, otherwise sparse complains - so 
-	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
+	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);


>+	if (rc == TPM_ERR_INVALID_POSTINIT) {
>	...
>+		rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);

Same here 
please use NULL instead of 0, otherwise sparse complains - so 
-		rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
+		rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);


>diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> extern ssize_t tpm_show_pubek(struct device *, struct device_attribute *attr,
>@@ -291,6 +292,10 @@ struct tpm_getrandom_in {
> 	__be32 num_bytes;
> }__attribute__((packed));
> 
>+struct tpm_startup_in {
>+	__be16	startup_type;
>+} __packed;


All the other user
__attribute__((packed));
Care to change to be consistent?


Apart from these three minor topics also a 
Reviewed-by: Peter Huewe <peter.huewe@...ineon.com>

Thanks,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/