[PATCH 0/5] Migrate all TPM 2.0 commands to use struct tpm_buf

[PATCH 0/5] Migrate all TPM 2.0 commands to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, this patch set migrates all TPM 2.0 commands to use it. The next
step after this is to migrate TPM 1.2 commands in a subsequent patch set.
Finally, tpm_transmit_cmd() can take simply struct tpm_buf as its argument
and this interface can be exported to be used by the kernel keyring.

The ultimate goal of this work is to make constructing TPM commands inside
the kernel simple and robust.

Alexander Steffen (1):
  tpm: Trigger only missing TPM 2.0 self tests

Jarkko Sakkinen (4):
  tpm: migrate tpm2_shutdown() to use struct tpm_buf
  tpm: migrate tpm2_probe() to use struct tpm_buf
  tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf
  tpm: migrate tpm2_get_random() to use struct tpm_buf

 drivers/char/tpm/tpm-interface.c |  20 ++-
 drivers/char/tpm/tpm.h           |   3 +-
 drivers/char/tpm/tpm2-cmd.c      | 266 ++++++++++++++-------------------------
 3 files changed, 112 insertions(+), 177 deletions(-)

-- 
2.15.1

[PATCH 0/5] Migrate all TPM 2.0 commands to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, this patch set migrates all TPM 2.0 commands to use it. The next
step after this is to migrate TPM 1.2 commands in a subsequent patch set.
Finally, tpm_transmit_cmd() can take simply struct tpm_buf as its argument
and this interface can be exported to be used by the kernel keyring.

The ultimate goal of this work is to make constructing TPM commands inside
the kernel simple and robust.

Alexander Steffen (1):
  tpm: Trigger only missing TPM 2.0 self tests

Jarkko Sakkinen (4):
  tpm: migrate tpm2_shutdown() to use struct tpm_buf
  tpm: migrate tpm2_probe() to use struct tpm_buf
  tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf
  tpm: migrate tpm2_get_random() to use struct tpm_buf

 drivers/char/tpm/tpm-interface.c |  20 ++-
 drivers/char/tpm/tpm.h           |   3 +-
 drivers/char/tpm/tpm2-cmd.c      | 266 ++++++++++++++-------------------------
 3 files changed, 112 insertions(+), 177 deletions(-)

-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/5] Migrate all TPM 2.0 commands to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, this patch set migrates all TPM 2.0 commands to use it. The next
step after this is to migrate TPM 1.2 commands in a subsequent patch set.
Finally, tpm_transmit_cmd() can take simply struct tpm_buf as its argument
and this interface can be exported to be used by the kernel keyring.

The ultimate goal of this work is to make constructing TPM commands inside
the kernel simple and robust.

Alexander Steffen (1):
  tpm: Trigger only missing TPM 2.0 self tests

Jarkko Sakkinen (4):
  tpm: migrate tpm2_shutdown() to use struct tpm_buf
  tpm: migrate tpm2_probe() to use struct tpm_buf
  tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf
  tpm: migrate tpm2_get_random() to use struct tpm_buf

 drivers/char/tpm/tpm-interface.c |  20 ++-
 drivers/char/tpm/tpm.h           |   3 +-
 drivers/char/tpm/tpm2-cmd.c      | 266 ++++++++++++++-------------------------
 3 files changed, 112 insertions(+), 177 deletions(-)

-- 
2.15.1

[PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[Jarkko Sakkinen]

From: Alexander Steffen <Alexander.Steffen@infineon.com>

My Nuvoton 6xx in a Dell XPS-13 has been intermittently failing to work
(necessitating a reboot). The problem seems to be that the TPM gets into a
state where the partial self-test doesn't return TPM_RC_SUCCESS (meaning
all tests have run to completion), but instead returns TPM_RC_TESTING
(meaning some tests are still running in the background).  There are
various theories that resending the self-test command actually causes the
tests to restart and thus triggers more TPM_RC_TESTING returns until the
timeout is exceeded.

There are several issues here: firstly being we shouldn't slow down the
boot sequence waiting for the self test to complete once the TPM
backgrounds them.  It will actually make available all functions that have
passed and if it gets a failure return TPM_RC_FAILURE to every subsequent
command.  So the fix is to kick off self tests once and if they return
TPM_RC_TESTING log that as a backgrounded self test and continue on.  In
order to prevent other tpm users from seeing any TPM_RC_TESTING returns
(which it might if they send a command that needs a TPM subsystem which is
still under test), we loop in tpm_transmit_cmd until either a timeout or we
don't get a TPM_RC_TESTING return.

Finally, there have been observations of strange returns from a partial
test. One Nuvoton is occasionally returning TPM_RC_COMMAND_CODE, so treat
any unexpected return from a partial self test as an indication we need to
run a full self test.

Fixes: 2482b1bba5122 ("tpm: Trigger only missing TPM 2.0 self tests")
Cc: stable@vger.kernel.org
Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 20 ++++++++++++---
 drivers/char/tpm/tpm.h           |  1 +
 drivers/char/tpm/tpm2-cmd.c      | 54 ++++++++++++----------------------------
 3 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 9e80a953d693..1adb976a2e37 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -537,14 +537,26 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
 			 const char *desc)
 {
 	const struct tpm_output_header *header = buf;
+	unsigned int delay_msec = TPM2_DURATION_SHORT;
 	int err;
 	ssize_t len;
 
-	len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
-	if (len <  0)
-		return len;
+	for (;;) {
+		len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
+		if (len <  0)
+			return len;
+		err = be32_to_cpu(header->return_code);
+		if (err != TPM2_RC_TESTING)
+			break;
+
+		delay_msec *= 2;
+		if (delay_msec > TPM2_DURATION_LONG) {
+			dev_err(&chip->dev, "the self test is still running\n");
+			break;
+		}
+		tpm_msleep(delay_msec);
+	}
 
-	err = be32_to_cpu(header->return_code);
 	if (err != 0 && desc)
 		dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
 			desc);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f895fba4e20d..cccd5994a0e1 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -104,6 +104,7 @@ enum tpm2_return_codes {
 	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
 	TPM2_RC_HANDLE		= 0x008B,
 	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
+	TPM2_RC_FAILURE		= 0x0101,
 	TPM2_RC_DISABLED	= 0x0120,
 	TPM2_RC_COMMAND_CODE    = 0x0143,
 	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a700f8f9ead7..6eeff3a60003 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -31,10 +31,6 @@ struct tpm2_startup_in {
 	__be16	startup_type;
 } __packed;
 
-struct tpm2_self_test_in {
-	u8	full_test;
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -60,7 +56,6 @@ struct tpm2_get_random_out {
 
 union tpm2_cmd_params {
 	struct	tpm2_startup_in		startup_in;
-	struct	tpm2_self_test_in	selftest_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -827,16 +822,6 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 }
 EXPORT_SYMBOL_GPL(tpm2_calc_ordinal_duration);
 
-#define TPM2_SELF_TEST_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_self_test_in))
-
-static const struct tpm_input_header tpm2_selftest_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_SELF_TEST_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_SELF_TEST)
-};
-
 /**
  * tpm2_do_selftest() - ensure that all self tests have passed
  *
@@ -852,27 +837,24 @@ static const struct tpm_input_header tpm2_selftest_header = {
  */
 static int tpm2_do_selftest(struct tpm_chip *chip)
 {
+	struct tpm_buf buf;
+	int full;
 	int rc;
-	unsigned int delay_msec = 10;
-	long duration;
-	struct tpm2_cmd cmd;
 
-	duration = jiffies_to_msecs(
-		tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST));
-
-	while (1) {
-		cmd.header.in = tpm2_selftest_header;
-		cmd.params.selftest_in.full_test = 0;
-
-		rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE,
-				      0, 0, "continue selftest");
+	for (full = 0; full < 2; full++) {
+		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+		if (rc)
+			return rc;
 
-		if (rc != TPM2_RC_TESTING || delay_msec >= duration)
-			break;
+		tpm_buf_append_u8(&buf, full);
+		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+				      "attempting the self test\n");
+		tpm_buf_destroy(&buf);
 
-		/* wait longer than before */
-		delay_msec *= 2;
-		tpm_msleep(delay_msec);
+		if (rc = TPM2_RC_TESTING)
+			rc = TPM2_RC_SUCCESS;
+		if (rc = TPM2_RC_INITIALIZE || rc = TPM2_RC_SUCCESS)
+			return rc;
 	}
 
 	return rc;
@@ -1058,10 +1040,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		goto out;
 
 	rc = tpm2_do_selftest(chip);
-	if (rc != 0 && rc != TPM2_RC_INITIALIZE) {
-		dev_err(&chip->dev, "TPM self test failed\n");
+	if (rc && rc != TPM2_RC_INITIALIZE)
 		goto out;
-	}
 
 	if (rc = TPM2_RC_INITIALIZE) {
 		rc = tpm_startup(chip);
@@ -1069,10 +1049,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 			goto out;
 
 		rc = tpm2_do_selftest(chip);
-		if (rc) {
-			dev_err(&chip->dev, "TPM self test failed\n");
+		if (rc)
 			goto out;
-		}
 	}
 
 	rc = tpm2_get_pcr_allocation(chip);
-- 
2.15.1

[PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[Jarkko Sakkinen]

From: Alexander Steffen <Alexander.Steffen@infineon.com>

My Nuvoton 6xx in a Dell XPS-13 has been intermittently failing to work
(necessitating a reboot). The problem seems to be that the TPM gets into a
state where the partial self-test doesn't return TPM_RC_SUCCESS (meaning
all tests have run to completion), but instead returns TPM_RC_TESTING
(meaning some tests are still running in the background).  There are
various theories that resending the self-test command actually causes the
tests to restart and thus triggers more TPM_RC_TESTING returns until the
timeout is exceeded.

There are several issues here: firstly being we shouldn't slow down the
boot sequence waiting for the self test to complete once the TPM
backgrounds them.  It will actually make available all functions that have
passed and if it gets a failure return TPM_RC_FAILURE to every subsequent
command.  So the fix is to kick off self tests once and if they return
TPM_RC_TESTING log that as a backgrounded self test and continue on.  In
order to prevent other tpm users from seeing any TPM_RC_TESTING returns
(which it might if they send a command that needs a TPM subsystem which is
still under test), we loop in tpm_transmit_cmd until either a timeout or we
don't get a TPM_RC_TESTING return.

Finally, there have been observations of strange returns from a partial
test. One Nuvoton is occasionally returning TPM_RC_COMMAND_CODE, so treat
any unexpected return from a partial self test as an indication we need to
run a full self test.

Fixes: 2482b1bba5122 ("tpm: Trigger only missing TPM 2.0 self tests")
Cc: stable at vger.kernel.org
Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 20 ++++++++++++---
 drivers/char/tpm/tpm.h           |  1 +
 drivers/char/tpm/tpm2-cmd.c      | 54 ++++++++++++----------------------------
 3 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 9e80a953d693..1adb976a2e37 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -537,14 +537,26 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
 			 const char *desc)
 {
 	const struct tpm_output_header *header = buf;
+	unsigned int delay_msec = TPM2_DURATION_SHORT;
 	int err;
 	ssize_t len;
 
-	len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
-	if (len <  0)
-		return len;
+	for (;;) {
+		len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
+		if (len <  0)
+			return len;
+		err = be32_to_cpu(header->return_code);
+		if (err != TPM2_RC_TESTING)
+			break;
+
+		delay_msec *= 2;
+		if (delay_msec > TPM2_DURATION_LONG) {
+			dev_err(&chip->dev, "the self test is still running\n");
+			break;
+		}
+		tpm_msleep(delay_msec);
+	}
 
-	err = be32_to_cpu(header->return_code);
 	if (err != 0 && desc)
 		dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
 			desc);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f895fba4e20d..cccd5994a0e1 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -104,6 +104,7 @@ enum tpm2_return_codes {
 	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
 	TPM2_RC_HANDLE		= 0x008B,
 	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
+	TPM2_RC_FAILURE		= 0x0101,
 	TPM2_RC_DISABLED	= 0x0120,
 	TPM2_RC_COMMAND_CODE    = 0x0143,
 	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a700f8f9ead7..6eeff3a60003 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -31,10 +31,6 @@ struct tpm2_startup_in {
 	__be16	startup_type;
 } __packed;
 
-struct tpm2_self_test_in {
-	u8	full_test;
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -60,7 +56,6 @@ struct tpm2_get_random_out {
 
 union tpm2_cmd_params {
 	struct	tpm2_startup_in		startup_in;
-	struct	tpm2_self_test_in	selftest_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -827,16 +822,6 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 }
 EXPORT_SYMBOL_GPL(tpm2_calc_ordinal_duration);
 
-#define TPM2_SELF_TEST_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_self_test_in))
-
-static const struct tpm_input_header tpm2_selftest_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_SELF_TEST_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_SELF_TEST)
-};
-
 /**
  * tpm2_do_selftest() - ensure that all self tests have passed
  *
@@ -852,27 +837,24 @@ static const struct tpm_input_header tpm2_selftest_header = {
  */
 static int tpm2_do_selftest(struct tpm_chip *chip)
 {
+	struct tpm_buf buf;
+	int full;
 	int rc;
-	unsigned int delay_msec = 10;
-	long duration;
-	struct tpm2_cmd cmd;
 
-	duration = jiffies_to_msecs(
-		tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST));
-
-	while (1) {
-		cmd.header.in = tpm2_selftest_header;
-		cmd.params.selftest_in.full_test = 0;
-
-		rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE,
-				      0, 0, "continue selftest");
+	for (full = 0; full < 2; full++) {
+		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+		if (rc)
+			return rc;
 
-		if (rc != TPM2_RC_TESTING || delay_msec >= duration)
-			break;
+		tpm_buf_append_u8(&buf, full);
+		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+				      "attempting the self test\n");
+		tpm_buf_destroy(&buf);
 
-		/* wait longer than before */
-		delay_msec *= 2;
-		tpm_msleep(delay_msec);
+		if (rc == TPM2_RC_TESTING)
+			rc = TPM2_RC_SUCCESS;
+		if (rc == TPM2_RC_INITIALIZE || rc == TPM2_RC_SUCCESS)
+			return rc;
 	}
 
 	return rc;
@@ -1058,10 +1040,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		goto out;
 
 	rc = tpm2_do_selftest(chip);
-	if (rc != 0 && rc != TPM2_RC_INITIALIZE) {
-		dev_err(&chip->dev, "TPM self test failed\n");
+	if (rc && rc != TPM2_RC_INITIALIZE)
 		goto out;
-	}
 
 	if (rc == TPM2_RC_INITIALIZE) {
 		rc = tpm_startup(chip);
@@ -1069,10 +1049,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 			goto out;
 
 		rc = tpm2_do_selftest(chip);
-		if (rc) {
-			dev_err(&chip->dev, "TPM self test failed\n");
+		if (rc)
 			goto out;
-		}
 	}
 
 	rc = tpm2_get_pcr_allocation(chip);
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[Jarkko Sakkinen]

From: Alexander Steffen <Alexander.Steffen@infineon.com>

My Nuvoton 6xx in a Dell XPS-13 has been intermittently failing to work
(necessitating a reboot). The problem seems to be that the TPM gets into a
state where the partial self-test doesn't return TPM_RC_SUCCESS (meaning
all tests have run to completion), but instead returns TPM_RC_TESTING
(meaning some tests are still running in the background).  There are
various theories that resending the self-test command actually causes the
tests to restart and thus triggers more TPM_RC_TESTING returns until the
timeout is exceeded.

There are several issues here: firstly being we shouldn't slow down the
boot sequence waiting for the self test to complete once the TPM
backgrounds them.  It will actually make available all functions that have
passed and if it gets a failure return TPM_RC_FAILURE to every subsequent
command.  So the fix is to kick off self tests once and if they return
TPM_RC_TESTING log that as a backgrounded self test and continue on.  In
order to prevent other tpm users from seeing any TPM_RC_TESTING returns
(which it might if they send a command that needs a TPM subsystem which is
still under test), we loop in tpm_transmit_cmd until either a timeout or we
don't get a TPM_RC_TESTING return.

Finally, there have been observations of strange returns from a partial
test. One Nuvoton is occasionally returning TPM_RC_COMMAND_CODE, so treat
any unexpected return from a partial self test as an indication we need to
run a full self test.

Fixes: 2482b1bba5122 ("tpm: Trigger only missing TPM 2.0 self tests")
Cc: stable@vger.kernel.org
Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 20 ++++++++++++---
 drivers/char/tpm/tpm.h           |  1 +
 drivers/char/tpm/tpm2-cmd.c      | 54 ++++++++++++----------------------------
 3 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 9e80a953d693..1adb976a2e37 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -537,14 +537,26 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
 			 const char *desc)
 {
 	const struct tpm_output_header *header = buf;
+	unsigned int delay_msec = TPM2_DURATION_SHORT;
 	int err;
 	ssize_t len;
 
-	len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
-	if (len <  0)
-		return len;
+	for (;;) {
+		len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
+		if (len <  0)
+			return len;
+		err = be32_to_cpu(header->return_code);
+		if (err != TPM2_RC_TESTING)
+			break;
+
+		delay_msec *= 2;
+		if (delay_msec > TPM2_DURATION_LONG) {
+			dev_err(&chip->dev, "the self test is still running\n");
+			break;
+		}
+		tpm_msleep(delay_msec);
+	}
 
-	err = be32_to_cpu(header->return_code);
 	if (err != 0 && desc)
 		dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
 			desc);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f895fba4e20d..cccd5994a0e1 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -104,6 +104,7 @@ enum tpm2_return_codes {
 	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
 	TPM2_RC_HANDLE		= 0x008B,
 	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
+	TPM2_RC_FAILURE		= 0x0101,
 	TPM2_RC_DISABLED	= 0x0120,
 	TPM2_RC_COMMAND_CODE    = 0x0143,
 	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a700f8f9ead7..6eeff3a60003 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -31,10 +31,6 @@ struct tpm2_startup_in {
 	__be16	startup_type;
 } __packed;
 
-struct tpm2_self_test_in {
-	u8	full_test;
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -60,7 +56,6 @@ struct tpm2_get_random_out {
 
 union tpm2_cmd_params {
 	struct	tpm2_startup_in		startup_in;
-	struct	tpm2_self_test_in	selftest_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -827,16 +822,6 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 }
 EXPORT_SYMBOL_GPL(tpm2_calc_ordinal_duration);
 
-#define TPM2_SELF_TEST_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_self_test_in))
-
-static const struct tpm_input_header tpm2_selftest_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_SELF_TEST_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_SELF_TEST)
-};
-
 /**
  * tpm2_do_selftest() - ensure that all self tests have passed
  *
@@ -852,27 +837,24 @@ static const struct tpm_input_header tpm2_selftest_header = {
  */
 static int tpm2_do_selftest(struct tpm_chip *chip)
 {
+	struct tpm_buf buf;
+	int full;
 	int rc;
-	unsigned int delay_msec = 10;
-	long duration;
-	struct tpm2_cmd cmd;
 
-	duration = jiffies_to_msecs(
-		tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST));
-
-	while (1) {
-		cmd.header.in = tpm2_selftest_header;
-		cmd.params.selftest_in.full_test = 0;
-
-		rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE,
-				      0, 0, "continue selftest");
+	for (full = 0; full < 2; full++) {
+		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+		if (rc)
+			return rc;
 
-		if (rc != TPM2_RC_TESTING || delay_msec >= duration)
-			break;
+		tpm_buf_append_u8(&buf, full);
+		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+				      "attempting the self test\n");
+		tpm_buf_destroy(&buf);
 
-		/* wait longer than before */
-		delay_msec *= 2;
-		tpm_msleep(delay_msec);
+		if (rc == TPM2_RC_TESTING)
+			rc = TPM2_RC_SUCCESS;
+		if (rc == TPM2_RC_INITIALIZE || rc == TPM2_RC_SUCCESS)
+			return rc;
 	}
 
 	return rc;
@@ -1058,10 +1040,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		goto out;
 
 	rc = tpm2_do_selftest(chip);
-	if (rc != 0 && rc != TPM2_RC_INITIALIZE) {
-		dev_err(&chip->dev, "TPM self test failed\n");
+	if (rc && rc != TPM2_RC_INITIALIZE)
 		goto out;
-	}
 
 	if (rc == TPM2_RC_INITIALIZE) {
 		rc = tpm_startup(chip);
@@ -1069,10 +1049,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 			goto out;
 
 		rc = tpm2_do_selftest(chip);
-		if (rc) {
-			dev_err(&chip->dev, "TPM self test failed\n");
+		if (rc)
 			goto out;
-		}
 	}
 
 	rc = tpm2_get_pcr_allocation(chip);
-- 
2.15.1

Re: [PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[James Bottomley]

On Wed, 2018-02-28 at 21:58 +0200, Jarkko Sakkinen wrote:
> From: Alexander Steffen <Alexander.Steffen@infineon.com>
> 
> My Nuvoton 6xx in a Dell XPS-13 has been intermittently failing to
> work (necessitating a reboot). The problem seems to be that the TPM
> gets into a state where the partial self-test doesn't return
> TPM_RC_SUCCESS (meaning all tests have run to completion), but
> instead returns TPM_RC_TESTING (meaning some tests are still running
> in the background).  There are various theories that resending the
> self-test command actually causes the tests to restart and thus
> triggers more TPM_RC_TESTING returns until the timeout is exceeded.
> 
> There are several issues here: firstly being we shouldn't slow down
> the boot sequence waiting for the self test to complete once the TPM
> backgrounds them.  It will actually make available all functions that
> have passed and if it gets a failure return TPM_RC_FAILURE to every
> subsequent command.  So the fix is to kick off self tests once and if
> they return TPM_RC_TESTING log that as a backgrounded self test

I still think removing all logging traces is a mistake for something
that can consume a significant amount of time in the boot sequence.
 It's going to cause lost of people doing boot timings to waste lots of
effort.

However, removing the log messages makes the above statement a lie, so
one of the two needs fixing.

[...]
> @@ -852,27 +837,24 @@ static const struct tpm_input_header
> tpm2_selftest_header = {
>   */
>  static int tpm2_do_selftest(struct tpm_chip *chip)
>  {
> +	struct tpm_buf buf;
> +	int full;
>  	int rc;
> -	unsigned int delay_msec = 10;
> -	long duration;
> -	struct tpm2_cmd cmd;
>  
> -	duration = jiffies_to_msecs(
> -		tpm2_calc_ordinal_duration(chip,
> TPM2_CC_SELF_TEST));
> -
> -	while (1) {
> -		cmd.header.in = tpm2_selftest_header;
> -		cmd.params.selftest_in.full_test = 0;
> -
> -		rc = tpm_transmit_cmd(chip, NULL, &cmd,
> TPM2_SELF_TEST_IN_SIZE,
> -				      0, 0, "continue selftest");
> +	for (full = 0; full < 2; full++) {
> +		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> TPM2_CC_SELF_TEST);
> +		if (rc)
> +			return rc;
>  
> -		if (rc != TPM2_RC_TESTING || delay_msec >= duration)
> -			break;
> +		tpm_buf_append_u8(&buf, full);
> +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> PAGE_SIZE, 0, 0,
> +				      "attempting the self test\n");

There shouldn't be a \n in the string: the failure message already
appends one.

James

Re: [PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[James Bottomley]

On Wed, 2018-02-28 at 21:58 +0200, Jarkko Sakkinen wrote:
> From: Alexander Steffen <Alexander.Steffen@infineon.com>
> 
> My Nuvoton 6xx in a Dell XPS-13 has been intermittently failing to
> work (necessitating a reboot). The problem seems to be that the TPM
> gets into a state where the partial self-test doesn't return
> TPM_RC_SUCCESS (meaning all tests have run to completion), but
> instead returns TPM_RC_TESTING (meaning some tests are still running
> in the background).  There are various theories that resending the
> self-test command actually causes the tests to restart and thus
> triggers more TPM_RC_TESTING returns until the timeout is exceeded.
> 
> There are several issues here: firstly being we shouldn't slow down
> the boot sequence waiting for the self test to complete once the TPM
> backgrounds them.  It will actually make available all functions that
> have passed and if it gets a failure return TPM_RC_FAILURE to every
> subsequent command.  So the fix is to kick off self tests once and if
> they return TPM_RC_TESTING log that as a backgrounded self test

I still think removing all logging traces is a mistake for something
that can consume a significant amount of time in the boot sequence.
 It's going to cause lost of people doing boot timings to waste lots of
effort.

However, removing the log messages makes the above statement a lie, so
one of the two needs fixing.

[...]
> @@ -852,27 +837,24 @@ static const struct tpm_input_header
> tpm2_selftest_header = {
>   */
>  static int tpm2_do_selftest(struct tpm_chip *chip)
>  {
> +	struct tpm_buf buf;
> +	int full;
>  	int rc;
> -	unsigned int delay_msec = 10;
> -	long duration;
> -	struct tpm2_cmd cmd;
>  
> -	duration = jiffies_to_msecs(
> -		tpm2_calc_ordinal_duration(chip,
> TPM2_CC_SELF_TEST));
> -
> -	while (1) {
> -		cmd.header.in = tpm2_selftest_header;
> -		cmd.params.selftest_in.full_test = 0;
> -
> -		rc = tpm_transmit_cmd(chip, NULL, &cmd,
> TPM2_SELF_TEST_IN_SIZE,
> -				      0, 0, "continue selftest");
> +	for (full = 0; full < 2; full++) {
> +		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> TPM2_CC_SELF_TEST);
> +		if (rc)
> +			return rc;
>  
> -		if (rc != TPM2_RC_TESTING || delay_msec >= duration)
> -			break;
> +		tpm_buf_append_u8(&buf, full);
> +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> PAGE_SIZE, 0, 0,
> +				      "attempting the self test\n");

There shouldn't be a \n in the string: the failure message already
appends one.

James

[PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[James Bottomley]

On Wed, 2018-02-28 at 21:58 +0200, Jarkko Sakkinen wrote:
> From: Alexander Steffen <Alexander.Steffen@infineon.com>
> 
> My Nuvoton 6xx in a Dell XPS-13 has been intermittently failing to
> work (necessitating a reboot). The problem seems to be that the TPM
> gets into a state where the partial self-test doesn't return
> TPM_RC_SUCCESS (meaning all tests have run to completion), but
> instead returns TPM_RC_TESTING (meaning some tests are still running
> in the background).??There are various theories that resending the
> self-test command actually causes the tests to restart and thus
> triggers more TPM_RC_TESTING returns until the timeout is exceeded.
> 
> There are several issues here: firstly being we shouldn't slow down
> the boot sequence waiting for the self test to complete once the TPM
> backgrounds them.??It will actually make available all functions that
> have passed and if it gets a failure return TPM_RC_FAILURE to every
> subsequent command.??So the fix is to kick off self tests once and if
> they return TPM_RC_TESTING log that as a backgrounded self test

I still think removing all logging traces is a mistake for something
that can consume a significant amount of time in the boot sequence.
?It's going to cause lost of people doing boot timings to waste lots of
effort.

However, removing the log messages makes the above statement a lie, so
one of the two needs fixing.

[...]
> @@ -852,27 +837,24 @@ static const struct tpm_input_header
> tpm2_selftest_header = {
> ? */
> ?static int tpm2_do_selftest(struct tpm_chip *chip)
> ?{
> +	struct tpm_buf buf;
> +	int full;
> ?	int rc;
> -	unsigned int delay_msec = 10;
> -	long duration;
> -	struct tpm2_cmd cmd;
> ?
> -	duration = jiffies_to_msecs(
> -		tpm2_calc_ordinal_duration(chip,
> TPM2_CC_SELF_TEST));
> -
> -	while (1) {
> -		cmd.header.in = tpm2_selftest_header;
> -		cmd.params.selftest_in.full_test = 0;
> -
> -		rc = tpm_transmit_cmd(chip, NULL, &cmd,
> TPM2_SELF_TEST_IN_SIZE,
> -				??????0, 0, "continue selftest");
> +	for (full = 0; full < 2; full++) {
> +		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> TPM2_CC_SELF_TEST);
> +		if (rc)
> +			return rc;
> ?
> -		if (rc != TPM2_RC_TESTING || delay_msec >= duration)
> -			break;
> +		tpm_buf_append_u8(&buf, full);
> +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> PAGE_SIZE, 0, 0,
> +				??????"attempting the self test\n");

There shouldn't be a \n in the string: the failure message already
appends one.

James

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[James Bottomley]

On Wed, 2018-02-28 at 21:58 +0200, Jarkko Sakkinen wrote:
> From: Alexander Steffen <Alexander.Steffen@infineon.com>
> 
> My Nuvoton 6xx in a Dell XPS-13 has been intermittently failing to
> work (necessitating a reboot). The problem seems to be that the TPM
> gets into a state where the partial self-test doesn't return
> TPM_RC_SUCCESS (meaning all tests have run to completion), but
> instead returns TPM_RC_TESTING (meaning some tests are still running
> in the background).  There are various theories that resending the
> self-test command actually causes the tests to restart and thus
> triggers more TPM_RC_TESTING returns until the timeout is exceeded.
> 
> There are several issues here: firstly being we shouldn't slow down
> the boot sequence waiting for the self test to complete once the TPM
> backgrounds them.  It will actually make available all functions that
> have passed and if it gets a failure return TPM_RC_FAILURE to every
> subsequent command.  So the fix is to kick off self tests once and if
> they return TPM_RC_TESTING log that as a backgrounded self test

I still think removing all logging traces is a mistake for something
that can consume a significant amount of time in the boot sequence.
 It's going to cause lost of people doing boot timings to waste lots of
effort.

However, removing the log messages makes the above statement a lie, so
one of the two needs fixing.

[...]
> @@ -852,27 +837,24 @@ static const struct tpm_input_header
> tpm2_selftest_header = {
>   */
>  static int tpm2_do_selftest(struct tpm_chip *chip)
>  {
> +	struct tpm_buf buf;
> +	int full;
>  	int rc;
> -	unsigned int delay_msec = 10;
> -	long duration;
> -	struct tpm2_cmd cmd;
>  
> -	duration = jiffies_to_msecs(
> -		tpm2_calc_ordinal_duration(chip,
> TPM2_CC_SELF_TEST));
> -
> -	while (1) {
> -		cmd.header.in = tpm2_selftest_header;
> -		cmd.params.selftest_in.full_test = 0;
> -
> -		rc = tpm_transmit_cmd(chip, NULL, &cmd,
> TPM2_SELF_TEST_IN_SIZE,
> -				      0, 0, "continue selftest");
> +	for (full = 0; full < 2; full++) {
> +		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> TPM2_CC_SELF_TEST);
> +		if (rc)
> +			return rc;
>  
> -		if (rc != TPM2_RC_TESTING || delay_msec >= duration)
> -			break;
> +		tpm_buf_append_u8(&buf, full);
> +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> PAGE_SIZE, 0, 0,
> +				      "attempting the self test\n");

There shouldn't be a \n in the string: the failure message already
appends one.

James

Re: [PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[Jarkko Sakkinen]

On Sat, Mar 03, 2018 at 12:11:59PM -0800, James Bottomley wrote:
> I still think removing all logging traces is a mistake for something
> that can consume a significant amount of time in the boot sequence.
>  It's going to cause lost of people doing boot timings to waste lots of
> effort.
> 
> However, removing the log messages makes the above statement a lie, so
> one of the two needs fixing.

The commit itself makes sense but the implementation was sloppy to say
the least in v3. It was like prototype/PoC version of something that
could be merged to mainline, not something that can be merged to
mainline.

For example:

* You could have implemented it more cleanly without that new
  'tpm_transmit_check' helper function as can be seen.
* Many log messages contained a redundant "TPM:" prefix.
* There duplicate logs to tpm_transmit_cmd(). Pass a NULL as
  desc tpm_transmit_cmd() if you want to take care of logging
  yourself.
* The commit has the same short summary as the commit it fixes.
  This issue still persists. Open for suggestions.

Please state if v3 contains a log message that has been removed and is
still mandatory and I can add it if it makes sense.

> > +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> > PAGE_SIZE, 0, 0,
> > +				      "attempting the self test\n");
> 
> There shouldn't be a \n in the string: the failure message already
> appends one.

Oops, my bad, will fix it.

/Jarkko

Re: [PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[Jarkko Sakkinen]

On Sat, Mar 03, 2018 at 12:11:59PM -0800, James Bottomley wrote:
> I still think removing all logging traces is a mistake for something
> that can consume a significant amount of time in the boot sequence.
>  It's going to cause lost of people doing boot timings to waste lots of
> effort.
> 
> However, removing the log messages makes the above statement a lie, so
> one of the two needs fixing.

The commit itself makes sense but the implementation was sloppy to say
the least in v3. It was like prototype/PoC version of something that
could be merged to mainline, not something that can be merged to
mainline.

For example:

* You could have implemented it more cleanly without that new
  'tpm_transmit_check' helper function as can be seen.
* Many log messages contained a redundant "TPM:" prefix.
* There duplicate logs to tpm_transmit_cmd(). Pass a NULL as
  desc tpm_transmit_cmd() if you want to take care of logging
  yourself.
* The commit has the same short summary as the commit it fixes.
  This issue still persists. Open for suggestions.

Please state if v3 contains a log message that has been removed and is
still mandatory and I can add it if it makes sense.

> > +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> > PAGE_SIZE, 0, 0,
> > +				      "attempting the self test\n");
> 
> There shouldn't be a \n in the string: the failure message already
> appends one.

Oops, my bad, will fix it.

/Jarkko

[PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[Jarkko Sakkinen]

On Sat, Mar 03, 2018 at 12:11:59PM -0800, James Bottomley wrote:
> I still think removing all logging traces is a mistake for something
> that can consume a significant amount of time in the boot sequence.
> ?It's going to cause lost of people doing boot timings to waste lots of
> effort.
> 
> However, removing the log messages makes the above statement a lie, so
> one of the two needs fixing.

The commit itself makes sense but the implementation was sloppy to say
the least in v3. It was like prototype/PoC version of something that
could be merged to mainline, not something that can be merged to
mainline.

For example:

* You could have implemented it more cleanly without that new
  'tpm_transmit_check' helper function as can be seen.
* Many log messages contained a redundant "TPM:" prefix.
* There duplicate logs to tpm_transmit_cmd(). Pass a NULL as
  desc tpm_transmit_cmd() if you want to take care of logging
  yourself.
* The commit has the same short summary as the commit it fixes.
  This issue still persists. Open for suggestions.

Please state if v3 contains a log message that has been removed and is
still mandatory and I can add it if it makes sense.

> > +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> > PAGE_SIZE, 0, 0,
> > +				??????"attempting the self test\n");
> 
> There shouldn't be a \n in the string: the failure message already
> appends one.

Oops, my bad, will fix it.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[Jarkko Sakkinen]

On Sat, Mar 03, 2018 at 12:11:59PM -0800, James Bottomley wrote:
> I still think removing all logging traces is a mistake for something
> that can consume a significant amount of time in the boot sequence.
> �It's going to cause lost of people doing boot timings to waste lots of
> effort.
> 
> However, removing the log messages makes the above statement a lie, so
> one of the two needs fixing.

The commit itself makes sense but the implementation was sloppy to say
the least in v3. It was like prototype/PoC version of something that
could be merged to mainline, not something that can be merged to
mainline.

For example:

* You could have implemented it more cleanly without that new
  'tpm_transmit_check' helper function as can be seen.
* Many log messages contained a redundant "TPM:" prefix.
* There duplicate logs to tpm_transmit_cmd(). Pass a NULL as
  desc tpm_transmit_cmd() if you want to take care of logging
  yourself.
* The commit has the same short summary as the commit it fixes.
  This issue still persists. Open for suggestions.

Please state if v3 contains a log message that has been removed and is
still mandatory and I can add it if it makes sense.

> > +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> > PAGE_SIZE, 0, 0,
> > +				������"attempting the self test\n");
> 
> There shouldn't be a \n in the string: the failure message already
> appends one.

Oops, my bad, will fix it.

/Jarkko

Re: [PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests

[Jarkko Sakkinen]

On Sat, Mar 03, 2018 at 12:11:59PM -0800, James Bottomley wrote:
> I still think removing all logging traces is a mistake for something
> that can consume a significant amount of time in the boot sequence.
>  It's going to cause lost of people doing boot timings to waste lots of
> effort.
> 
> However, removing the log messages makes the above statement a lie, so
> one of the two needs fixing.

The commit itself makes sense but the implementation was sloppy to say
the least in v3. It was like prototype/PoC version of something that
could be merged to mainline, not something that can be merged to
mainline.

For example:

* You could have implemented it more cleanly without that new
  'tpm_transmit_check' helper function as can be seen.
* Many log messages contained a redundant "TPM:" prefix.
* There duplicate logs to tpm_transmit_cmd(). Pass a NULL as
  desc tpm_transmit_cmd() if you want to take care of logging
  yourself.
* The commit has the same short summary as the commit it fixes.
  This issue still persists. Open for suggestions.

Please state if v3 contains a log message that has been removed and is
still mandatory and I can add it if it makes sense.

> > +		rc = tpm_transmit_cmd(chip, NULL, buf.data,
> > PAGE_SIZE, 0, 0,
> > +				      "attempting the self test\n");
> 
> There shouldn't be a \n in the string: the failure message already
> appends one.

Oops, my bad, will fix it.

/Jarkko

[PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
prints an error message.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eeff3a60003..5f92141b1e16 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,10 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_startup_in {
-	__be16	startup_type;
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -55,7 +51,6 @@ struct tpm2_get_random_out {
 } __packed;
 
 union tpm2_cmd_params {
-	struct	tpm2_startup_in		startup_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
 
-#define TPM2_SHUTDOWN_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_startup_in))
-
-static const struct tpm_input_header tpm2_shutdown_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
-};
-
 /**
  * tpm2_shutdown() - send shutdown command to the TPM chip
  *
+ * In places where shutdown command is sent there's no much we can do except
+ * print the error code on a system failure.
+ *
  * @chip:		TPM chip to use.
  * @shutdown_type:	shutdown type. The value is either
  *			TPM_SU_CLEAR or TPM_SU_STATE.
  */
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_shutdown_header;
-	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
-			      "stopping the TPM");
-
-	/* In places where shutdown command is sent there's no much we can do
-	 * except print the error code on a system failure.
-	 */
-	if (rc < 0 && rc != -EPIPE)
-		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
-			 rc);
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+	if (rc) {
+		dev_err(&chip->dev, "%s: out of memory", __func__);
+		return;
+	}
+	tpm_buf_append_u16(&buf, shutdown_type);
+	tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+			 "stopping the TPM");
+	tpm_buf_destroy(&buf);
 }
 
 /*
-- 
2.15.1

[PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
prints an error message.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eeff3a60003..5f92141b1e16 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,10 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_startup_in {
-	__be16	startup_type;
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -55,7 +51,6 @@ struct tpm2_get_random_out {
 } __packed;
 
 union tpm2_cmd_params {
-	struct	tpm2_startup_in		startup_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
 
-#define TPM2_SHUTDOWN_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_startup_in))
-
-static const struct tpm_input_header tpm2_shutdown_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
-};
-
 /**
  * tpm2_shutdown() - send shutdown command to the TPM chip
  *
+ * In places where shutdown command is sent there's no much we can do except
+ * print the error code on a system failure.
+ *
  * @chip:		TPM chip to use.
  * @shutdown_type:	shutdown type. The value is either
  *			TPM_SU_CLEAR or TPM_SU_STATE.
  */
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_shutdown_header;
-	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
-			      "stopping the TPM");
-
-	/* In places where shutdown command is sent there's no much we can do
-	 * except print the error code on a system failure.
-	 */
-	if (rc < 0 && rc != -EPIPE)
-		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
-			 rc);
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+	if (rc) {
+		dev_err(&chip->dev, "%s: out of memory", __func__);
+		return;
+	}
+	tpm_buf_append_u16(&buf, shutdown_type);
+	tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+			 "stopping the TPM");
+	tpm_buf_destroy(&buf);
 }
 
 /*
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
prints an error message.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eeff3a60003..5f92141b1e16 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,10 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_startup_in {
-	__be16	startup_type;
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -55,7 +51,6 @@ struct tpm2_get_random_out {
 } __packed;
 
 union tpm2_cmd_params {
-	struct	tpm2_startup_in		startup_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
 
-#define TPM2_SHUTDOWN_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_startup_in))
-
-static const struct tpm_input_header tpm2_shutdown_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
-};
-
 /**
  * tpm2_shutdown() - send shutdown command to the TPM chip
  *
+ * In places where shutdown command is sent there's no much we can do except
+ * print the error code on a system failure.
+ *
  * @chip:		TPM chip to use.
  * @shutdown_type:	shutdown type. The value is either
  *			TPM_SU_CLEAR or TPM_SU_STATE.
  */
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_shutdown_header;
-	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
-			      "stopping the TPM");
-
-	/* In places where shutdown command is sent there's no much we can do
-	 * except print the error code on a system failure.
-	 */
-	if (rc < 0 && rc != -EPIPE)
-		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
-			 rc);
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+	if (rc) {
+		dev_err(&chip->dev, "%s: out of memory", __func__);
+		return;
+	}
+	tpm_buf_append_u16(&buf, shutdown_type);
+	tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+			 "stopping the TPM");
+	tpm_buf_destroy(&buf);
 }
 
 /*
-- 
2.15.1

Re: [PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jason Gunthorpe]

On Wed, Feb 28, 2018 at 09:58:15PM +0200, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing TPM
> commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> prints an error message.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>  drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
>  1 file changed, 13 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 6eeff3a60003..5f92141b1e16 100644
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,10 +27,6 @@ enum tpm2_session_attributes {
>  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>  };
>  
> -struct tpm2_startup_in {
> -	__be16	startup_type;
> -} __packed;
> -
>  struct tpm2_get_tpm_pt_in {
>  	__be32	cap_id;
>  	__be32	property_id;
> @@ -55,7 +51,6 @@ struct tpm2_get_random_out {
>  } __packed;
>  
>  union tpm2_cmd_params {
> -	struct	tpm2_startup_in		startup_in;
>  	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
>  	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
>  	struct	tpm2_get_random_in	getrandom_in;
> @@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
>  }
>  EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
>  
> -#define TPM2_SHUTDOWN_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_startup_in))
> -
> -static const struct tpm_input_header tpm2_shutdown_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
> -};
> -
>  /**
>   * tpm2_shutdown() - send shutdown command to the TPM chip
>   *
> + * In places where shutdown command is sent there's no much we can do except
> + * print the error code on a system failure.
> + *
>   * @chip:		TPM chip to use.
>   * @shutdown_type:	shutdown type. The value is either
>   *			TPM_SU_CLEAR or TPM_SU_STATE.
>   */
>  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
>  {
> -	struct tpm2_cmd cmd;
> +	struct tpm_buf buf;
>  	int rc;
>  
> -	cmd.header.in = tpm2_shutdown_header;
> -	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
> -
> -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> -			      "stopping the TPM");
> -
> -	/* In places where shutdown command is sent there's no much we can do
> -	 * except print the error code on a system failure.
> -	 */
> -	if (rc < 0 && rc != -EPIPE)
> -		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
> -			 rc);
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
> +	if (rc) {
> +		dev_err(&chip->dev, "%s: out of memory", __func__);

Don't print for out of memory. The kernel already makes lots of
prints for you. Same comment everywhere.

Jason

[PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jason Gunthorpe]

On Wed, Feb 28, 2018 at 09:58:15PM +0200, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing TPM
> commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> prints an error message.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>  drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
>  1 file changed, 13 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 6eeff3a60003..5f92141b1e16 100644
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,10 +27,6 @@ enum tpm2_session_attributes {
>  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>  };
>  
> -struct tpm2_startup_in {
> -	__be16	startup_type;
> -} __packed;
> -
>  struct tpm2_get_tpm_pt_in {
>  	__be32	cap_id;
>  	__be32	property_id;
> @@ -55,7 +51,6 @@ struct tpm2_get_random_out {
>  } __packed;
>  
>  union tpm2_cmd_params {
> -	struct	tpm2_startup_in		startup_in;
>  	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
>  	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
>  	struct	tpm2_get_random_in	getrandom_in;
> @@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
>  }
>  EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
>  
> -#define TPM2_SHUTDOWN_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_startup_in))
> -
> -static const struct tpm_input_header tpm2_shutdown_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
> -};
> -
>  /**
>   * tpm2_shutdown() - send shutdown command to the TPM chip
>   *
> + * In places where shutdown command is sent there's no much we can do except
> + * print the error code on a system failure.
> + *
>   * @chip:		TPM chip to use.
>   * @shutdown_type:	shutdown type. The value is either
>   *			TPM_SU_CLEAR or TPM_SU_STATE.
>   */
>  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
>  {
> -	struct tpm2_cmd cmd;
> +	struct tpm_buf buf;
>  	int rc;
>  
> -	cmd.header.in = tpm2_shutdown_header;
> -	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
> -
> -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> -			      "stopping the TPM");
> -
> -	/* In places where shutdown command is sent there's no much we can do
> -	 * except print the error code on a system failure.
> -	 */
> -	if (rc < 0 && rc != -EPIPE)
> -		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
> -			 rc);
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
> +	if (rc) {
> +		dev_err(&chip->dev, "%s: out of memory", __func__);

Don't print for out of memory. The kernel already makes lots of
prints for you. Same comment everywhere.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jason Gunthorpe]

On Wed, Feb 28, 2018 at 09:58:15PM +0200, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing TPM
> commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> prints an error message.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>  drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
>  1 file changed, 13 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 6eeff3a60003..5f92141b1e16 100644
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,10 +27,6 @@ enum tpm2_session_attributes {
>  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>  };
>  
> -struct tpm2_startup_in {
> -	__be16	startup_type;
> -} __packed;
> -
>  struct tpm2_get_tpm_pt_in {
>  	__be32	cap_id;
>  	__be32	property_id;
> @@ -55,7 +51,6 @@ struct tpm2_get_random_out {
>  } __packed;
>  
>  union tpm2_cmd_params {
> -	struct	tpm2_startup_in		startup_in;
>  	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
>  	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
>  	struct	tpm2_get_random_in	getrandom_in;
> @@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
>  }
>  EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
>  
> -#define TPM2_SHUTDOWN_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_startup_in))
> -
> -static const struct tpm_input_header tpm2_shutdown_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
> -};
> -
>  /**
>   * tpm2_shutdown() - send shutdown command to the TPM chip
>   *
> + * In places where shutdown command is sent there's no much we can do except
> + * print the error code on a system failure.
> + *
>   * @chip:		TPM chip to use.
>   * @shutdown_type:	shutdown type. The value is either
>   *			TPM_SU_CLEAR or TPM_SU_STATE.
>   */
>  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
>  {
> -	struct tpm2_cmd cmd;
> +	struct tpm_buf buf;
>  	int rc;
>  
> -	cmd.header.in = tpm2_shutdown_header;
> -	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
> -
> -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> -			      "stopping the TPM");
> -
> -	/* In places where shutdown command is sent there's no much we can do
> -	 * except print the error code on a system failure.
> -	 */
> -	if (rc < 0 && rc != -EPIPE)
> -		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
> -			 rc);
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
> +	if (rc) {
> +		dev_err(&chip->dev, "%s: out of memory", __func__);

Don't print for out of memory. The kernel already makes lots of
prints for you. Same comment everywhere.

Jason

Re: [PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

On Wed, Feb 28, 2018 at 01:25:40PM -0700, Jason Gunthorpe wrote:
> On Wed, Feb 28, 2018 at 09:58:15PM +0200, Jarkko Sakkinen wrote:
> > In order to make struct tpm_buf the first class object for constructing TPM
> > commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> > entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> > prints an error message.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >  drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
> >  1 file changed, 13 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index 6eeff3a60003..5f92141b1e16 100644
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -27,10 +27,6 @@ enum tpm2_session_attributes {
> >  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
> >  };
> >  
> > -struct tpm2_startup_in {
> > -	__be16	startup_type;
> > -} __packed;
> > -
> >  struct tpm2_get_tpm_pt_in {
> >  	__be32	cap_id;
> >  	__be32	property_id;
> > @@ -55,7 +51,6 @@ struct tpm2_get_random_out {
> >  } __packed;
> >  
> >  union tpm2_cmd_params {
> > -	struct	tpm2_startup_in		startup_in;
> >  	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
> >  	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
> >  	struct	tpm2_get_random_in	getrandom_in;
> > @@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
> >  }
> >  EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
> >  
> > -#define TPM2_SHUTDOWN_IN_SIZE \
> > -	(sizeof(struct tpm_input_header) + \
> > -	 sizeof(struct tpm2_startup_in))
> > -
> > -static const struct tpm_input_header tpm2_shutdown_header = {
> > -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > -	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
> > -	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
> > -};
> > -
> >  /**
> >   * tpm2_shutdown() - send shutdown command to the TPM chip
> >   *
> > + * In places where shutdown command is sent there's no much we can do except
> > + * print the error code on a system failure.
> > + *
> >   * @chip:		TPM chip to use.
> >   * @shutdown_type:	shutdown type. The value is either
> >   *			TPM_SU_CLEAR or TPM_SU_STATE.
> >   */
> >  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
> >  {
> > -	struct tpm2_cmd cmd;
> > +	struct tpm_buf buf;
> >  	int rc;
> >  
> > -	cmd.header.in = tpm2_shutdown_header;
> > -	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
> > -
> > -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> > -			      "stopping the TPM");
> > -
> > -	/* In places where shutdown command is sent there's no much we can do
> > -	 * except print the error code on a system failure.
> > -	 */
> > -	if (rc < 0 && rc != -EPIPE)
> > -		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
> > -			 rc);
> > +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
> > +	if (rc) {
> > +		dev_err(&chip->dev, "%s: out of memory", __func__);
> 
> Don't print for out of memory. The kernel already makes lots of
> prints for you. Same comment everywhere.
> 
> Jason

I'll remove that. Thanks!

/Jarkko

[PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

On Wed, Feb 28, 2018 at 01:25:40PM -0700, Jason Gunthorpe wrote:
> On Wed, Feb 28, 2018 at 09:58:15PM +0200, Jarkko Sakkinen wrote:
> > In order to make struct tpm_buf the first class object for constructing TPM
> > commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> > entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> > prints an error message.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >  drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
> >  1 file changed, 13 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index 6eeff3a60003..5f92141b1e16 100644
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -27,10 +27,6 @@ enum tpm2_session_attributes {
> >  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
> >  };
> >  
> > -struct tpm2_startup_in {
> > -	__be16	startup_type;
> > -} __packed;
> > -
> >  struct tpm2_get_tpm_pt_in {
> >  	__be32	cap_id;
> >  	__be32	property_id;
> > @@ -55,7 +51,6 @@ struct tpm2_get_random_out {
> >  } __packed;
> >  
> >  union tpm2_cmd_params {
> > -	struct	tpm2_startup_in		startup_in;
> >  	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
> >  	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
> >  	struct	tpm2_get_random_in	getrandom_in;
> > @@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
> >  }
> >  EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
> >  
> > -#define TPM2_SHUTDOWN_IN_SIZE \
> > -	(sizeof(struct tpm_input_header) + \
> > -	 sizeof(struct tpm2_startup_in))
> > -
> > -static const struct tpm_input_header tpm2_shutdown_header = {
> > -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > -	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
> > -	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
> > -};
> > -
> >  /**
> >   * tpm2_shutdown() - send shutdown command to the TPM chip
> >   *
> > + * In places where shutdown command is sent there's no much we can do except
> > + * print the error code on a system failure.
> > + *
> >   * @chip:		TPM chip to use.
> >   * @shutdown_type:	shutdown type. The value is either
> >   *			TPM_SU_CLEAR or TPM_SU_STATE.
> >   */
> >  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
> >  {
> > -	struct tpm2_cmd cmd;
> > +	struct tpm_buf buf;
> >  	int rc;
> >  
> > -	cmd.header.in = tpm2_shutdown_header;
> > -	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
> > -
> > -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> > -			      "stopping the TPM");
> > -
> > -	/* In places where shutdown command is sent there's no much we can do
> > -	 * except print the error code on a system failure.
> > -	 */
> > -	if (rc < 0 && rc != -EPIPE)
> > -		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
> > -			 rc);
> > +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
> > +	if (rc) {
> > +		dev_err(&chip->dev, "%s: out of memory", __func__);
> 
> Don't print for out of memory. The kernel already makes lots of
> prints for you. Same comment everywhere.
> 
> Jason

I'll remove that. Thanks!

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

On Wed, Feb 28, 2018 at 01:25:40PM -0700, Jason Gunthorpe wrote:
> On Wed, Feb 28, 2018 at 09:58:15PM +0200, Jarkko Sakkinen wrote:
> > In order to make struct tpm_buf the first class object for constructing TPM
> > commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> > entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> > prints an error message.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >  drivers/char/tpm/tpm2-cmd.c | 41 +++++++++++++----------------------------
> >  1 file changed, 13 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index 6eeff3a60003..5f92141b1e16 100644
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -27,10 +27,6 @@ enum tpm2_session_attributes {
> >  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
> >  };
> >  
> > -struct tpm2_startup_in {
> > -	__be16	startup_type;
> > -} __packed;
> > -
> >  struct tpm2_get_tpm_pt_in {
> >  	__be32	cap_id;
> >  	__be32	property_id;
> > @@ -55,7 +51,6 @@ struct tpm2_get_random_out {
> >  } __packed;
> >  
> >  union tpm2_cmd_params {
> > -	struct	tpm2_startup_in		startup_in;
> >  	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
> >  	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
> >  	struct	tpm2_get_random_in	getrandom_in;
> > @@ -760,40 +755,30 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
> >  }
> >  EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
> >  
> > -#define TPM2_SHUTDOWN_IN_SIZE \
> > -	(sizeof(struct tpm_input_header) + \
> > -	 sizeof(struct tpm2_startup_in))
> > -
> > -static const struct tpm_input_header tpm2_shutdown_header = {
> > -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > -	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
> > -	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
> > -};
> > -
> >  /**
> >   * tpm2_shutdown() - send shutdown command to the TPM chip
> >   *
> > + * In places where shutdown command is sent there's no much we can do except
> > + * print the error code on a system failure.
> > + *
> >   * @chip:		TPM chip to use.
> >   * @shutdown_type:	shutdown type. The value is either
> >   *			TPM_SU_CLEAR or TPM_SU_STATE.
> >   */
> >  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
> >  {
> > -	struct tpm2_cmd cmd;
> > +	struct tpm_buf buf;
> >  	int rc;
> >  
> > -	cmd.header.in = tpm2_shutdown_header;
> > -	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
> > -
> > -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> > -			      "stopping the TPM");
> > -
> > -	/* In places where shutdown command is sent there's no much we can do
> > -	 * except print the error code on a system failure.
> > -	 */
> > -	if (rc < 0 && rc != -EPIPE)
> > -		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
> > -			 rc);
> > +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
> > +	if (rc) {
> > +		dev_err(&chip->dev, "%s: out of memory", __func__);
> 
> Don't print for out of memory. The kernel already makes lots of
> prints for you. Same comment everywhere.
> 
> Jason

I'll remove that. Thanks!

/Jarkko

[PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_probe() to use it.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 5f92141b1e16..36f6b0ac412b 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -856,19 +856,22 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
  */
 int tpm2_probe(struct tpm_chip *chip)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_output_header *out;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_get_tpm_pt_header;
-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
+	tpm_buf_append_u32(&buf, 1);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	tpm_buf_destroy(&buf);
 	if (rc <  0)
 		return rc;
-
-	if (be16_to_cpu(cmd.header.out.tag) = TPM2_ST_NO_SESSIONS)
+	out = (struct tpm_output_header *)buf.data;
+	if (be16_to_cpu(out->tag) = TPM2_ST_NO_SESSIONS)
 		chip->flags |= TPM_CHIP_FLAG_TPM2;
 
 	return 0;
-- 
2.15.1

[PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_probe() to use it.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 5f92141b1e16..36f6b0ac412b 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -856,19 +856,22 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
  */
 int tpm2_probe(struct tpm_chip *chip)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_output_header *out;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_get_tpm_pt_header;
-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
+	tpm_buf_append_u32(&buf, 1);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	tpm_buf_destroy(&buf);
 	if (rc <  0)
 		return rc;
-
-	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
+	out = (struct tpm_output_header *)buf.data;
+	if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
 		chip->flags |= TPM_CHIP_FLAG_TPM2;
 
 	return 0;
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_probe() to use it.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 5f92141b1e16..36f6b0ac412b 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -856,19 +856,22 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
  */
 int tpm2_probe(struct tpm_chip *chip)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_output_header *out;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_get_tpm_pt_header;
-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
+	tpm_buf_append_u32(&buf, 1);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	tpm_buf_destroy(&buf);
 	if (rc <  0)
 		return rc;
-
-	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
+	out = (struct tpm_output_header *)buf.data;
+	if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
 		chip->flags |= TPM_CHIP_FLAG_TPM2;
 
 	return 0;
-- 
2.15.1

Re: [PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[J Freyensee]

.
.
.
I'm new to this area of the kernel, but I'm not getting these lines:

> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> +	tpm_buf_destroy(&buf);
>   	if (rc <  0)
Why is this if() check not directly after the tpm_transmit_cmd() call 
that sets rc?  Is it correct you want to destroy buf regardless of the 
tpm_transmit_cmd() outcome?
>   		return rc;
> -
> -	if (be16_to_cpu(cmd.header.out.tag) = TPM2_ST_NO_SESSIONS)
> +	out = (struct tpm_output_header *)buf.data;

So buf has been destroyed, buf.data sill has something valid to assign 
to out?
> +	if (be16_to_cpu(out->tag) = TPM2_ST_NO_SESSIONS)
>   		chip->flags |= TPM_CHIP_FLAG_TPM2;
>   
>   	return 0;
Thanks,
Jay

Re: [PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[J Freyensee]

.
.
.
I'm new to this area of the kernel, but I'm not getting these lines:

> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> +	tpm_buf_destroy(&buf);
>   	if (rc <  0)
Why is this if() check not directly after the tpm_transmit_cmd() call 
that sets rc?  Is it correct you want to destroy buf regardless of the 
tpm_transmit_cmd() outcome?
>   		return rc;
> -
> -	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
> +	out = (struct tpm_output_header *)buf.data;

So buf has been destroyed, buf.data sill has something valid to assign 
to out?
> +	if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
>   		chip->flags |= TPM_CHIP_FLAG_TPM2;
>   
>   	return 0;
Thanks,
Jay

[PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[J Freyensee]

.
.
.
I'm new to this area of the kernel, but I'm not getting these lines:

> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> +	tpm_buf_destroy(&buf);
>   	if (rc <  0)
Why is this if() check not directly after the tpm_transmit_cmd() call 
that sets rc?? Is it correct you want to destroy buf regardless of the 
tpm_transmit_cmd() outcome?
>   		return rc;
> -
> -	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
> +	out = (struct tpm_output_header *)buf.data;

So buf has been destroyed, buf.data sill has something valid to assign 
to out?
> +	if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
>   		chip->flags |= TPM_CHIP_FLAG_TPM2;
>   
>   	return 0;
Thanks,
Jay
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[J Freyensee]

.
.
.
I'm new to this area of the kernel, but I'm not getting these lines:

> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> +	tpm_buf_destroy(&buf);
>   	if (rc <  0)
Why is this if() check not directly after the tpm_transmit_cmd() call 
that sets rc?  Is it correct you want to destroy buf regardless of the 
tpm_transmit_cmd() outcome?
>   		return rc;
> -
> -	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
> +	out = (struct tpm_output_header *)buf.data;

So buf has been destroyed, buf.data sill has something valid to assign 
to out?
> +	if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
>   		chip->flags |= TPM_CHIP_FLAG_TPM2;
>   
>   	return 0;
Thanks,
Jay

Re: [PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jarkko Sakkinen]

On Thu, Mar 01, 2018 at 02:10:17PM -0800, J Freyensee wrote:
> .
> .
> .
> I'm new to this area of the kernel, but I'm not getting these lines:
> 
> > +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> > +	tpm_buf_destroy(&buf);
> >   	if (rc <  0)
> Why is this if() check not directly after the tpm_transmit_cmd() call that
> sets rc?  Is it correct you want to destroy buf regardless of the
> tpm_transmit_cmd() outcome?
> >   		return rc;
> > -
> > -	if (be16_to_cpu(cmd.header.out.tag) = TPM2_ST_NO_SESSIONS)
> > +	out = (struct tpm_output_header *)buf.data;
> 
> So buf has been destroyed, buf.data sill has something valid to assign to
> out?
> > +	if (be16_to_cpu(out->tag) = TPM2_ST_NO_SESSIONS)
> >   		chip->flags |= TPM_CHIP_FLAG_TPM2;
> >   	return 0;
> Thanks,
> Jay

Nope it is a regression in the patch. Thank you :-) tpm_buf_destroy()
can be called if the response data is not needed other than everything
went OK (tpm_transmit_cmd() already digs this info).

/Jarkko

Re: [PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jarkko Sakkinen]

On Thu, Mar 01, 2018 at 02:10:17PM -0800, J Freyensee wrote:
> .
> .
> .
> I'm new to this area of the kernel, but I'm not getting these lines:
> 
> > +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> > +	tpm_buf_destroy(&buf);
> >   	if (rc <  0)
> Why is this if() check not directly after the tpm_transmit_cmd() call that
> sets rc?  Is it correct you want to destroy buf regardless of the
> tpm_transmit_cmd() outcome?
> >   		return rc;
> > -
> > -	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
> > +	out = (struct tpm_output_header *)buf.data;
> 
> So buf has been destroyed, buf.data sill has something valid to assign to
> out?
> > +	if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
> >   		chip->flags |= TPM_CHIP_FLAG_TPM2;
> >   	return 0;
> Thanks,
> Jay

Nope it is a regression in the patch. Thank you :-) tpm_buf_destroy()
can be called if the response data is not needed other than everything
went OK (tpm_transmit_cmd() already digs this info).

/Jarkko

[PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jarkko Sakkinen]

On Thu, Mar 01, 2018 at 02:10:17PM -0800, J Freyensee wrote:
> .
> .
> .
> I'm new to this area of the kernel, but I'm not getting these lines:
> 
> > +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> > +	tpm_buf_destroy(&buf);
> >   	if (rc <  0)
> Why is this if() check not directly after the tpm_transmit_cmd() call that
> sets rc?? Is it correct you want to destroy buf regardless of the
> tpm_transmit_cmd() outcome?
> >   		return rc;
> > -
> > -	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
> > +	out = (struct tpm_output_header *)buf.data;
> 
> So buf has been destroyed, buf.data sill has something valid to assign to
> out?
> > +	if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
> >   		chip->flags |= TPM_CHIP_FLAG_TPM2;
> >   	return 0;
> Thanks,
> Jay

Nope it is a regression in the patch. Thank you :-) tpm_buf_destroy()
can be called if the response data is not needed other than everything
went OK (tpm_transmit_cmd() already digs this info).

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jarkko Sakkinen]

On Thu, Mar 01, 2018 at 02:10:17PM -0800, J Freyensee wrote:
> .
> .
> .
> I'm new to this area of the kernel, but I'm not getting these lines:
> 
> > +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> > +	tpm_buf_destroy(&buf);
> >   	if (rc <  0)
> Why is this if() check not directly after the tpm_transmit_cmd() call that
> sets rc?  Is it correct you want to destroy buf regardless of the
> tpm_transmit_cmd() outcome?
> >   		return rc;
> > -
> > -	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
> > +	out = (struct tpm_output_header *)buf.data;
> 
> So buf has been destroyed, buf.data sill has something valid to assign to
> out?
> > +	if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
> >   		chip->flags |= TPM_CHIP_FLAG_TPM2;
> >   	return 0;
> Thanks,
> Jay

Nope it is a regression in the patch. Thank you :-) tpm_buf_destroy()
can be called if the response data is not needed other than everything
went OK (tpm_transmit_cmd() already digs this info).

/Jarkko

[PATCH 4/5] tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_get_tpm_pt() to use it.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 63 +++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 36f6b0ac412b..43791947b8e1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,20 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_get_tpm_pt_in {
-	__be32	cap_id;
-	__be32	property_id;
-	__be32	property_cnt;
-} __packed;
-
-struct tpm2_get_tpm_pt_out {
-	u8	more_data;
-	__be32	subcap_id;
-	__be32	property_cnt;
-	__be32	property_id;
-	__be32	value;
-} __packed;
-
 struct tpm2_get_random_in {
 	__be16	size;
 } __packed;
@@ -51,8 +37,6 @@ struct tpm2_get_random_out {
 } __packed;
 
 union tpm2_cmd_params {
-	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
-	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
 	struct	tpm2_get_random_out	getrandom_out;
 };
@@ -377,19 +361,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 	return total ? total : -EIO;
 }
 
-#define TPM2_GET_TPM_PT_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_tpm_pt_in))
-
-#define TPM2_GET_TPM_PT_OUT_BODY_SIZE \
-	 sizeof(struct tpm2_get_tpm_pt_out)
-
-static const struct tpm_input_header tpm2_get_tpm_pt_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
-};
-
 /**
  * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
  * @chip: TPM chip to use
@@ -726,6 +697,14 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	return rc;
 }
 
+struct tpm2_get_cap_out {
+	u8 more_data;
+	__be32 subcap_id;
+	__be32 property_cnt;
+	__be32 property_id;
+	__be32 value;
+} __packed;
+
 /**
  * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
  * @chip:		TPM chip to use.
@@ -738,19 +717,23 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 			const char *desc)
 {
-	struct tpm2_cmd cmd;
+	struct tpm2_get_cap_out *out;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_get_tpm_pt_header;
-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
-			      TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
-	if (!rc)
-		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
-
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+	tpm_buf_append_u32(&buf, property_id);
+	tpm_buf_append_u32(&buf, 1);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	if (!rc) {
+		out = (struct tpm2_get_cap_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		*value = be32_to_cpu(out->value);
+	}
+	tpm_buf_destroy(&buf);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
-- 
2.15.1

[PATCH 4/5] tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_get_tpm_pt() to use it.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 63 +++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 36f6b0ac412b..43791947b8e1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,20 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_get_tpm_pt_in {
-	__be32	cap_id;
-	__be32	property_id;
-	__be32	property_cnt;
-} __packed;
-
-struct tpm2_get_tpm_pt_out {
-	u8	more_data;
-	__be32	subcap_id;
-	__be32	property_cnt;
-	__be32	property_id;
-	__be32	value;
-} __packed;
-
 struct tpm2_get_random_in {
 	__be16	size;
 } __packed;
@@ -51,8 +37,6 @@ struct tpm2_get_random_out {
 } __packed;
 
 union tpm2_cmd_params {
-	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
-	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
 	struct	tpm2_get_random_out	getrandom_out;
 };
@@ -377,19 +361,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 	return total ? total : -EIO;
 }
 
-#define TPM2_GET_TPM_PT_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_tpm_pt_in))
-
-#define TPM2_GET_TPM_PT_OUT_BODY_SIZE \
-	 sizeof(struct tpm2_get_tpm_pt_out)
-
-static const struct tpm_input_header tpm2_get_tpm_pt_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
-};
-
 /**
  * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
  * @chip: TPM chip to use
@@ -726,6 +697,14 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	return rc;
 }
 
+struct tpm2_get_cap_out {
+	u8 more_data;
+	__be32 subcap_id;
+	__be32 property_cnt;
+	__be32 property_id;
+	__be32 value;
+} __packed;
+
 /**
  * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
  * @chip:		TPM chip to use.
@@ -738,19 +717,23 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 			const char *desc)
 {
-	struct tpm2_cmd cmd;
+	struct tpm2_get_cap_out *out;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_get_tpm_pt_header;
-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
-			      TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
-	if (!rc)
-		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
-
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+	tpm_buf_append_u32(&buf, property_id);
+	tpm_buf_append_u32(&buf, 1);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	if (!rc) {
+		out = (struct tpm2_get_cap_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		*value = be32_to_cpu(out->value);
+	}
+	tpm_buf_destroy(&buf);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_get_tpm_pt() to use it.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 63 +++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 36f6b0ac412b..43791947b8e1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,20 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_get_tpm_pt_in {
-	__be32	cap_id;
-	__be32	property_id;
-	__be32	property_cnt;
-} __packed;
-
-struct tpm2_get_tpm_pt_out {
-	u8	more_data;
-	__be32	subcap_id;
-	__be32	property_cnt;
-	__be32	property_id;
-	__be32	value;
-} __packed;
-
 struct tpm2_get_random_in {
 	__be16	size;
 } __packed;
@@ -51,8 +37,6 @@ struct tpm2_get_random_out {
 } __packed;
 
 union tpm2_cmd_params {
-	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
-	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
 	struct	tpm2_get_random_out	getrandom_out;
 };
@@ -377,19 +361,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 	return total ? total : -EIO;
 }
 
-#define TPM2_GET_TPM_PT_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_tpm_pt_in))
-
-#define TPM2_GET_TPM_PT_OUT_BODY_SIZE \
-	 sizeof(struct tpm2_get_tpm_pt_out)
-
-static const struct tpm_input_header tpm2_get_tpm_pt_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
-};
-
 /**
  * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
  * @chip: TPM chip to use
@@ -726,6 +697,14 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	return rc;
 }
 
+struct tpm2_get_cap_out {
+	u8 more_data;
+	__be32 subcap_id;
+	__be32 property_cnt;
+	__be32 property_id;
+	__be32 value;
+} __packed;
+
 /**
  * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
  * @chip:		TPM chip to use.
@@ -738,19 +717,23 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 			const char *desc)
 {
-	struct tpm2_cmd cmd;
+	struct tpm2_get_cap_out *out;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_get_tpm_pt_header;
-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
-			      TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
-	if (!rc)
-		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
-
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+	tpm_buf_append_u32(&buf, property_id);
+	tpm_buf_append_u32(&buf, 1);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	if (!rc) {
+		out = (struct tpm2_get_cap_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		*value = be32_to_cpu(out->value);
+	}
+	tpm_buf_destroy(&buf);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
-- 
2.15.1

[PATCH 5/5] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing
TPM commands, migrate tpm2_get_random() to use it. In addition, removed
remaining references to struct tpm2_cmd. All of them use it to acquire
the length of the response, which can be achieved by using
tpm_buf_length().

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h      |  2 +-
 drivers/char/tpm/tpm2-cmd.c | 87 ++++++++++++++++-----------------------------
 2 files changed, 31 insertions(+), 58 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index cccd5994a0e1..cffb71ec0289 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -557,7 +557,7 @@ static inline u32 tpm2_rc_value(u32 rc)
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		    struct tpm2_digest *digests);
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
 void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 			    unsigned int flags);
 int tpm2_seal_trusted(struct tpm_chip *chip,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 43791947b8e1..574caa33aab3 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,25 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_get_random_in {
-	__be16	size;
-} __packed;
-
-struct tpm2_get_random_out {
-	__be16	size;
-	u8	buffer[TPM_MAX_RNG_DATA];
-} __packed;
-
-union tpm2_cmd_params {
-	struct	tpm2_get_random_in	getrandom_in;
-	struct	tpm2_get_random_out	getrandom_out;
-};
-
-struct tpm2_cmd {
-	tpm_cmd_header		header;
-	union tpm2_cmd_params	params;
-} __packed;
-
 struct tpm2_hash {
 	unsigned int crypto_id;
 	unsigned int tpm_id;
@@ -298,62 +279,58 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 }
 
 
-#define TPM2_GETRANDOM_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_random_in))
-
-static const struct tpm_input_header tpm2_getrandom_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
-};
+struct tpm2_get_random_out {
+	__be16 size;
+	u8 buffer[TPM_MAX_RNG_DATA];
+} __packed;
 
 /**
  * tpm2_get_random() - get random bytes from the TPM RNG
  *
  * @chip: TPM chip to use
- * @out: destination buffer for the random bytes
+ * @dest: destination buffer for the random bytes
  * @max: the max number of bytes to write to @out
  *
  * Return:
- *    Size of the output buffer, or -EIO on error.
+ * size of the output buffer when the operation is successful.
+ * A negative number for system errors (errno).
  */
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 {
-	struct tpm2_cmd cmd;
-	u32 recd, rlength;
-	u32 num_bytes;
+	struct tpm2_get_random_out *out;
+	struct tpm_buf buf;
+	u32 recd;
+	u32 num_bytes = max;
 	int err;
 	int total = 0;
 	int retries = 5;
-	u8 *dest = out;
+	u8 *dest_ptr = dest;
 
-	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
-
-	if (!out || !num_bytes ||
-	    max > sizeof(cmd.params.getrandom_out.buffer))
+	if (!num_bytes || max > TPM_MAX_RNG_DATA)
 		return -EINVAL;
 
 	do {
-		cmd.header.in = tpm2_getrandom_header;
-		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
-
-		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
+		err = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
+				   TPM2_CC_GET_RANDOM);
+		if (err)
+			return err;
+		tpm_buf_append_u16(&buf, num_bytes);
+		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
 				       offsetof(struct tpm2_get_random_out,
 						buffer),
 				       0, "attempting get random");
 		if (err)
 			break;
 
-		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
-			     num_bytes);
-		rlength = be32_to_cpu(cmd.header.out.length);
-		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
-			      recd)
+		out = (struct tpm2_get_random_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
+		if (tpm_buf_length(&buf) <
+		    offsetof(struct tpm2_get_random_out, buffer) + recd)
 			return -EFAULT;
-		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
+		memcpy(dest_ptr, out->buffer, recd);
 
-		dest += recd;
+		dest_ptr += recd;
 		total += recd;
 		num_bytes -= recd;
 	} while (retries-- && total < max);
@@ -435,7 +412,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
-	u32 hash, rlength;
+	u32 hash;
 	int i;
 	int rc;
 
@@ -510,8 +487,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		rc = -E2BIG;
 		goto out;
 	}
-	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
-	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
+	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
 		rc = -EFAULT;
 		goto out;
 	}
@@ -621,7 +597,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	u16 data_len;
 	u8 *data;
 	int rc;
-	u32 rlength;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
 	if (rc)
@@ -649,9 +624,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			goto out;
 		}
 
-		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
-					->header.out.length);
-		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
+		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
 			rc = -EFAULT;
 			goto out;
 		}
-- 
2.15.1

[PATCH 5/5] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing
TPM commands, migrate tpm2_get_random() to use it. In addition, removed
remaining references to struct tpm2_cmd. All of them use it to acquire
the length of the response, which can be achieved by using
tpm_buf_length().

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h      |  2 +-
 drivers/char/tpm/tpm2-cmd.c | 87 ++++++++++++++++-----------------------------
 2 files changed, 31 insertions(+), 58 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index cccd5994a0e1..cffb71ec0289 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -557,7 +557,7 @@ static inline u32 tpm2_rc_value(u32 rc)
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		    struct tpm2_digest *digests);
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
 void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 			    unsigned int flags);
 int tpm2_seal_trusted(struct tpm_chip *chip,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 43791947b8e1..574caa33aab3 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,25 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_get_random_in {
-	__be16	size;
-} __packed;
-
-struct tpm2_get_random_out {
-	__be16	size;
-	u8	buffer[TPM_MAX_RNG_DATA];
-} __packed;
-
-union tpm2_cmd_params {
-	struct	tpm2_get_random_in	getrandom_in;
-	struct	tpm2_get_random_out	getrandom_out;
-};
-
-struct tpm2_cmd {
-	tpm_cmd_header		header;
-	union tpm2_cmd_params	params;
-} __packed;
-
 struct tpm2_hash {
 	unsigned int crypto_id;
 	unsigned int tpm_id;
@@ -298,62 +279,58 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 }
 
 
-#define TPM2_GETRANDOM_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_random_in))
-
-static const struct tpm_input_header tpm2_getrandom_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
-};
+struct tpm2_get_random_out {
+	__be16 size;
+	u8 buffer[TPM_MAX_RNG_DATA];
+} __packed;
 
 /**
  * tpm2_get_random() - get random bytes from the TPM RNG
  *
  * @chip: TPM chip to use
- * @out: destination buffer for the random bytes
+ * @dest: destination buffer for the random bytes
  * @max: the max number of bytes to write to @out
  *
  * Return:
- *    Size of the output buffer, or -EIO on error.
+ * size of the output buffer when the operation is successful.
+ * A negative number for system errors (errno).
  */
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 {
-	struct tpm2_cmd cmd;
-	u32 recd, rlength;
-	u32 num_bytes;
+	struct tpm2_get_random_out *out;
+	struct tpm_buf buf;
+	u32 recd;
+	u32 num_bytes = max;
 	int err;
 	int total = 0;
 	int retries = 5;
-	u8 *dest = out;
+	u8 *dest_ptr = dest;
 
-	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
-
-	if (!out || !num_bytes ||
-	    max > sizeof(cmd.params.getrandom_out.buffer))
+	if (!num_bytes || max > TPM_MAX_RNG_DATA)
 		return -EINVAL;
 
 	do {
-		cmd.header.in = tpm2_getrandom_header;
-		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
-
-		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
+		err = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
+				   TPM2_CC_GET_RANDOM);
+		if (err)
+			return err;
+		tpm_buf_append_u16(&buf, num_bytes);
+		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
 				       offsetof(struct tpm2_get_random_out,
 						buffer),
 				       0, "attempting get random");
 		if (err)
 			break;
 
-		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
-			     num_bytes);
-		rlength = be32_to_cpu(cmd.header.out.length);
-		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
-			      recd)
+		out = (struct tpm2_get_random_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
+		if (tpm_buf_length(&buf) <
+		    offsetof(struct tpm2_get_random_out, buffer) + recd)
 			return -EFAULT;
-		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
+		memcpy(dest_ptr, out->buffer, recd);
 
-		dest += recd;
+		dest_ptr += recd;
 		total += recd;
 		num_bytes -= recd;
 	} while (retries-- && total < max);
@@ -435,7 +412,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
-	u32 hash, rlength;
+	u32 hash;
 	int i;
 	int rc;
 
@@ -510,8 +487,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		rc = -E2BIG;
 		goto out;
 	}
-	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
-	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
+	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
 		rc = -EFAULT;
 		goto out;
 	}
@@ -621,7 +597,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	u16 data_len;
 	u8 *data;
 	int rc;
-	u32 rlength;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
 	if (rc)
@@ -649,9 +624,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			goto out;
 		}
 
-		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
-					->header.out.length);
-		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
+		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
 			rc = -EFAULT;
 			goto out;
 		}
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info@ http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing
TPM commands, migrate tpm2_get_random() to use it. In addition, removed
remaining references to struct tpm2_cmd. All of them use it to acquire
the length of the response, which can be achieved by using
tpm_buf_length().

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h      |  2 +-
 drivers/char/tpm/tpm2-cmd.c | 87 ++++++++++++++++-----------------------------
 2 files changed, 31 insertions(+), 58 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index cccd5994a0e1..cffb71ec0289 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -557,7 +557,7 @@ static inline u32 tpm2_rc_value(u32 rc)
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		    struct tpm2_digest *digests);
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
 void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 			    unsigned int flags);
 int tpm2_seal_trusted(struct tpm_chip *chip,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 43791947b8e1..574caa33aab3 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,25 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_get_random_in {
-	__be16	size;
-} __packed;
-
-struct tpm2_get_random_out {
-	__be16	size;
-	u8	buffer[TPM_MAX_RNG_DATA];
-} __packed;
-
-union tpm2_cmd_params {
-	struct	tpm2_get_random_in	getrandom_in;
-	struct	tpm2_get_random_out	getrandom_out;
-};
-
-struct tpm2_cmd {
-	tpm_cmd_header		header;
-	union tpm2_cmd_params	params;
-} __packed;
-
 struct tpm2_hash {
 	unsigned int crypto_id;
 	unsigned int tpm_id;
@@ -298,62 +279,58 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 }
 
 
-#define TPM2_GETRANDOM_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_random_in))
-
-static const struct tpm_input_header tpm2_getrandom_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
-};
+struct tpm2_get_random_out {
+	__be16 size;
+	u8 buffer[TPM_MAX_RNG_DATA];
+} __packed;
 
 /**
  * tpm2_get_random() - get random bytes from the TPM RNG
  *
  * @chip: TPM chip to use
- * @out: destination buffer for the random bytes
+ * @dest: destination buffer for the random bytes
  * @max: the max number of bytes to write to @out
  *
  * Return:
- *    Size of the output buffer, or -EIO on error.
+ * size of the output buffer when the operation is successful.
+ * A negative number for system errors (errno).
  */
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 {
-	struct tpm2_cmd cmd;
-	u32 recd, rlength;
-	u32 num_bytes;
+	struct tpm2_get_random_out *out;
+	struct tpm_buf buf;
+	u32 recd;
+	u32 num_bytes = max;
 	int err;
 	int total = 0;
 	int retries = 5;
-	u8 *dest = out;
+	u8 *dest_ptr = dest;
 
-	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
-
-	if (!out || !num_bytes ||
-	    max > sizeof(cmd.params.getrandom_out.buffer))
+	if (!num_bytes || max > TPM_MAX_RNG_DATA)
 		return -EINVAL;
 
 	do {
-		cmd.header.in = tpm2_getrandom_header;
-		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
-
-		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
+		err = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
+				   TPM2_CC_GET_RANDOM);
+		if (err)
+			return err;
+		tpm_buf_append_u16(&buf, num_bytes);
+		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
 				       offsetof(struct tpm2_get_random_out,
 						buffer),
 				       0, "attempting get random");
 		if (err)
 			break;
 
-		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
-			     num_bytes);
-		rlength = be32_to_cpu(cmd.header.out.length);
-		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
-			      recd)
+		out = (struct tpm2_get_random_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
+		if (tpm_buf_length(&buf) <
+		    offsetof(struct tpm2_get_random_out, buffer) + recd)
 			return -EFAULT;
-		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
+		memcpy(dest_ptr, out->buffer, recd);
 
-		dest += recd;
+		dest_ptr += recd;
 		total += recd;
 		num_bytes -= recd;
 	} while (retries-- && total < max);
@@ -435,7 +412,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
-	u32 hash, rlength;
+	u32 hash;
 	int i;
 	int rc;
 
@@ -510,8 +487,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		rc = -E2BIG;
 		goto out;
 	}
-	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
-	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
+	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
 		rc = -EFAULT;
 		goto out;
 	}
@@ -621,7 +597,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	u16 data_len;
 	u8 *data;
 	int rc;
-	u32 rlength;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
 	if (rc)
@@ -649,9 +624,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			goto out;
 		}
 
-		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
-					->header.out.length);
-		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
+		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
 			rc = -EFAULT;
 			goto out;
 		}
-- 
2.15.1

Re: [PATCH 5/5] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Jarkko Sakkinen]

On Wed, Feb 28, 2018 at 09:58:18PM +0200, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing
> TPM commands, migrate tpm2_get_random() to use it. In addition, removed
> remaining references to struct tpm2_cmd. All of them use it to acquire
> the length of the response, which can be achieved by using
> tpm_buf_length().
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/char/tpm/tpm.h      |  2 +-
>  drivers/char/tpm/tpm2-cmd.c | 87 ++++++++++++++++-----------------------------
>  2 files changed, 31 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index cccd5994a0e1..cffb71ec0289 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -557,7 +557,7 @@ static inline u32 tpm2_rc_value(u32 rc)
>  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>  int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  		    struct tpm2_digest *digests);
> -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
>  void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>  			    unsigned int flags);
>  int tpm2_seal_trusted(struct tpm_chip *chip,
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 43791947b8e1..574caa33aab3 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,25 +27,6 @@ enum tpm2_session_attributes {
>  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>  };
>  
> -struct tpm2_get_random_in {
> -	__be16	size;
> -} __packed;
> -
> -struct tpm2_get_random_out {
> -	__be16	size;
> -	u8	buffer[TPM_MAX_RNG_DATA];
> -} __packed;
> -
> -union tpm2_cmd_params {
> -	struct	tpm2_get_random_in	getrandom_in;
> -	struct	tpm2_get_random_out	getrandom_out;
> -};
> -
> -struct tpm2_cmd {
> -	tpm_cmd_header		header;
> -	union tpm2_cmd_params	params;
> -} __packed;
> -
>  struct tpm2_hash {
>  	unsigned int crypto_id;
>  	unsigned int tpm_id;
> @@ -298,62 +279,58 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  }
>  
>  
> -#define TPM2_GETRANDOM_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_get_random_in))
> -
> -static const struct tpm_input_header tpm2_getrandom_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
> -};
> +struct tpm2_get_random_out {
> +	__be16 size;
> +	u8 buffer[TPM_MAX_RNG_DATA];
> +} __packed;
>  
>  /**
>   * tpm2_get_random() - get random bytes from the TPM RNG
>   *
>   * @chip: TPM chip to use
> - * @out: destination buffer for the random bytes
> + * @dest: destination buffer for the random bytes
>   * @max: the max number of bytes to write to @out
>   *
>   * Return:
> - *    Size of the output buffer, or -EIO on error.
> + * size of the output buffer when the operation is successful.
> + * A negative number for system errors (errno).
>   */
> -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>  {
> -	struct tpm2_cmd cmd;
> -	u32 recd, rlength;
> -	u32 num_bytes;
> +	struct tpm2_get_random_out *out;
> +	struct tpm_buf buf;
> +	u32 recd;
> +	u32 num_bytes = max;
>  	int err;
>  	int total = 0;
>  	int retries = 5;
> -	u8 *dest = out;
> +	u8 *dest_ptr = dest;
>  
> -	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
> -
> -	if (!out || !num_bytes ||
> -	    max > sizeof(cmd.params.getrandom_out.buffer))
> +	if (!num_bytes || max > TPM_MAX_RNG_DATA)
>  		return -EINVAL;
>  
>  	do {
> -		cmd.header.in = tpm2_getrandom_header;
> -		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
> -
> -		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
> +		err = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> +				   TPM2_CC_GET_RANDOM);
> +		if (err)
> +			return err;
> +		tpm_buf_append_u16(&buf, num_bytes);
> +		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
>  				       offsetof(struct tpm2_get_random_out,
>  						buffer),
>  				       0, "attempting get random");
>  		if (err)
>  			break;
>  
> -		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
> -			     num_bytes);
> -		rlength = be32_to_cpu(cmd.header.out.length);
> -		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
> -			      recd)
> +		out = (struct tpm2_get_random_out *)
> +			&buf.data[TPM_HEADER_SIZE];
> +		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
> +		if (tpm_buf_length(&buf) <
> +		    offsetof(struct tpm2_get_random_out, buffer) + recd)
>  			return -EFAULT;
> -		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
> +		memcpy(dest_ptr, out->buffer, recd);
>  
> -		dest += recd;
> +		dest_ptr += recd;
>  		total += recd;
>  		num_bytes -= recd;
>  	} while (retries-- && total < max);
> @@ -435,7 +412,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  {
>  	unsigned int blob_len;
>  	struct tpm_buf buf;
> -	u32 hash, rlength;
> +	u32 hash;
>  	int i;
>  	int rc;
>  
> @@ -510,8 +487,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  		rc = -E2BIG;
>  		goto out;
>  	}
> -	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
> -	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
> +	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
>  		rc = -EFAULT;
>  		goto out;
>  	}
> @@ -621,7 +597,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  	u16 data_len;
>  	u8 *data;
>  	int rc;
> -	u32 rlength;
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
>  	if (rc)
> @@ -649,9 +624,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  			goto out;
>  		}
>  
> -		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
> -					->header.out.length);
> -		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
> +		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
>  			rc = -EFAULT;
>  			goto out;
>  		}
> -- 
> 2.15.1
> 

tpm_buf_destroy() missing

/Jarkko

[PATCH 5/5] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Jarkko Sakkinen]

On Wed, Feb 28, 2018 at 09:58:18PM +0200, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing
> TPM commands, migrate tpm2_get_random() to use it. In addition, removed
> remaining references to struct tpm2_cmd. All of them use it to acquire
> the length of the response, which can be achieved by using
> tpm_buf_length().
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/char/tpm/tpm.h      |  2 +-
>  drivers/char/tpm/tpm2-cmd.c | 87 ++++++++++++++++-----------------------------
>  2 files changed, 31 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index cccd5994a0e1..cffb71ec0289 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -557,7 +557,7 @@ static inline u32 tpm2_rc_value(u32 rc)
>  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>  int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  		    struct tpm2_digest *digests);
> -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
>  void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>  			    unsigned int flags);
>  int tpm2_seal_trusted(struct tpm_chip *chip,
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 43791947b8e1..574caa33aab3 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,25 +27,6 @@ enum tpm2_session_attributes {
>  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>  };
>  
> -struct tpm2_get_random_in {
> -	__be16	size;
> -} __packed;
> -
> -struct tpm2_get_random_out {
> -	__be16	size;
> -	u8	buffer[TPM_MAX_RNG_DATA];
> -} __packed;
> -
> -union tpm2_cmd_params {
> -	struct	tpm2_get_random_in	getrandom_in;
> -	struct	tpm2_get_random_out	getrandom_out;
> -};
> -
> -struct tpm2_cmd {
> -	tpm_cmd_header		header;
> -	union tpm2_cmd_params	params;
> -} __packed;
> -
>  struct tpm2_hash {
>  	unsigned int crypto_id;
>  	unsigned int tpm_id;
> @@ -298,62 +279,58 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  }
>  
>  
> -#define TPM2_GETRANDOM_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_get_random_in))
> -
> -static const struct tpm_input_header tpm2_getrandom_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
> -};
> +struct tpm2_get_random_out {
> +	__be16 size;
> +	u8 buffer[TPM_MAX_RNG_DATA];
> +} __packed;
>  
>  /**
>   * tpm2_get_random() - get random bytes from the TPM RNG
>   *
>   * @chip: TPM chip to use
> - * @out: destination buffer for the random bytes
> + * @dest: destination buffer for the random bytes
>   * @max: the max number of bytes to write to @out
>   *
>   * Return:
> - *    Size of the output buffer, or -EIO on error.
> + * size of the output buffer when the operation is successful.
> + * A negative number for system errors (errno).
>   */
> -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>  {
> -	struct tpm2_cmd cmd;
> -	u32 recd, rlength;
> -	u32 num_bytes;
> +	struct tpm2_get_random_out *out;
> +	struct tpm_buf buf;
> +	u32 recd;
> +	u32 num_bytes = max;
>  	int err;
>  	int total = 0;
>  	int retries = 5;
> -	u8 *dest = out;
> +	u8 *dest_ptr = dest;
>  
> -	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
> -
> -	if (!out || !num_bytes ||
> -	    max > sizeof(cmd.params.getrandom_out.buffer))
> +	if (!num_bytes || max > TPM_MAX_RNG_DATA)
>  		return -EINVAL;
>  
>  	do {
> -		cmd.header.in = tpm2_getrandom_header;
> -		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
> -
> -		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
> +		err = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> +				   TPM2_CC_GET_RANDOM);
> +		if (err)
> +			return err;
> +		tpm_buf_append_u16(&buf, num_bytes);
> +		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
>  				       offsetof(struct tpm2_get_random_out,
>  						buffer),
>  				       0, "attempting get random");
>  		if (err)
>  			break;
>  
> -		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
> -			     num_bytes);
> -		rlength = be32_to_cpu(cmd.header.out.length);
> -		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
> -			      recd)
> +		out = (struct tpm2_get_random_out *)
> +			&buf.data[TPM_HEADER_SIZE];
> +		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
> +		if (tpm_buf_length(&buf) <
> +		    offsetof(struct tpm2_get_random_out, buffer) + recd)
>  			return -EFAULT;
> -		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
> +		memcpy(dest_ptr, out->buffer, recd);
>  
> -		dest += recd;
> +		dest_ptr += recd;
>  		total += recd;
>  		num_bytes -= recd;
>  	} while (retries-- && total < max);
> @@ -435,7 +412,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  {
>  	unsigned int blob_len;
>  	struct tpm_buf buf;
> -	u32 hash, rlength;
> +	u32 hash;
>  	int i;
>  	int rc;
>  
> @@ -510,8 +487,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  		rc = -E2BIG;
>  		goto out;
>  	}
> -	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
> -	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
> +	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
>  		rc = -EFAULT;
>  		goto out;
>  	}
> @@ -621,7 +597,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  	u16 data_len;
>  	u8 *data;
>  	int rc;
> -	u32 rlength;
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
>  	if (rc)
> @@ -649,9 +624,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  			goto out;
>  		}
>  
> -		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
> -					->header.out.length);
> -		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
> +		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
>  			rc = -EFAULT;
>  			goto out;
>  		}
> -- 
> 2.15.1
> 

tpm_buf_destroy() missing

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Jarkko Sakkinen]

On Wed, Feb 28, 2018 at 09:58:18PM +0200, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing
> TPM commands, migrate tpm2_get_random() to use it. In addition, removed
> remaining references to struct tpm2_cmd. All of them use it to acquire
> the length of the response, which can be achieved by using
> tpm_buf_length().
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/char/tpm/tpm.h      |  2 +-
>  drivers/char/tpm/tpm2-cmd.c | 87 ++++++++++++++++-----------------------------
>  2 files changed, 31 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index cccd5994a0e1..cffb71ec0289 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -557,7 +557,7 @@ static inline u32 tpm2_rc_value(u32 rc)
>  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>  int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  		    struct tpm2_digest *digests);
> -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
>  void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>  			    unsigned int flags);
>  int tpm2_seal_trusted(struct tpm_chip *chip,
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 43791947b8e1..574caa33aab3 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,25 +27,6 @@ enum tpm2_session_attributes {
>  	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>  };
>  
> -struct tpm2_get_random_in {
> -	__be16	size;
> -} __packed;
> -
> -struct tpm2_get_random_out {
> -	__be16	size;
> -	u8	buffer[TPM_MAX_RNG_DATA];
> -} __packed;
> -
> -union tpm2_cmd_params {
> -	struct	tpm2_get_random_in	getrandom_in;
> -	struct	tpm2_get_random_out	getrandom_out;
> -};
> -
> -struct tpm2_cmd {
> -	tpm_cmd_header		header;
> -	union tpm2_cmd_params	params;
> -} __packed;
> -
>  struct tpm2_hash {
>  	unsigned int crypto_id;
>  	unsigned int tpm_id;
> @@ -298,62 +279,58 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  }
>  
>  
> -#define TPM2_GETRANDOM_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_get_random_in))
> -
> -static const struct tpm_input_header tpm2_getrandom_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
> -};
> +struct tpm2_get_random_out {
> +	__be16 size;
> +	u8 buffer[TPM_MAX_RNG_DATA];
> +} __packed;
>  
>  /**
>   * tpm2_get_random() - get random bytes from the TPM RNG
>   *
>   * @chip: TPM chip to use
> - * @out: destination buffer for the random bytes
> + * @dest: destination buffer for the random bytes
>   * @max: the max number of bytes to write to @out
>   *
>   * Return:
> - *    Size of the output buffer, or -EIO on error.
> + * size of the output buffer when the operation is successful.
> + * A negative number for system errors (errno).
>   */
> -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>  {
> -	struct tpm2_cmd cmd;
> -	u32 recd, rlength;
> -	u32 num_bytes;
> +	struct tpm2_get_random_out *out;
> +	struct tpm_buf buf;
> +	u32 recd;
> +	u32 num_bytes = max;
>  	int err;
>  	int total = 0;
>  	int retries = 5;
> -	u8 *dest = out;
> +	u8 *dest_ptr = dest;
>  
> -	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
> -
> -	if (!out || !num_bytes ||
> -	    max > sizeof(cmd.params.getrandom_out.buffer))
> +	if (!num_bytes || max > TPM_MAX_RNG_DATA)
>  		return -EINVAL;
>  
>  	do {
> -		cmd.header.in = tpm2_getrandom_header;
> -		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
> -
> -		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
> +		err = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> +				   TPM2_CC_GET_RANDOM);
> +		if (err)
> +			return err;
> +		tpm_buf_append_u16(&buf, num_bytes);
> +		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
>  				       offsetof(struct tpm2_get_random_out,
>  						buffer),
>  				       0, "attempting get random");
>  		if (err)
>  			break;
>  
> -		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
> -			     num_bytes);
> -		rlength = be32_to_cpu(cmd.header.out.length);
> -		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
> -			      recd)
> +		out = (struct tpm2_get_random_out *)
> +			&buf.data[TPM_HEADER_SIZE];
> +		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
> +		if (tpm_buf_length(&buf) <
> +		    offsetof(struct tpm2_get_random_out, buffer) + recd)
>  			return -EFAULT;
> -		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
> +		memcpy(dest_ptr, out->buffer, recd);
>  
> -		dest += recd;
> +		dest_ptr += recd;
>  		total += recd;
>  		num_bytes -= recd;
>  	} while (retries-- && total < max);
> @@ -435,7 +412,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  {
>  	unsigned int blob_len;
>  	struct tpm_buf buf;
> -	u32 hash, rlength;
> +	u32 hash;
>  	int i;
>  	int rc;
>  
> @@ -510,8 +487,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  		rc = -E2BIG;
>  		goto out;
>  	}
> -	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
> -	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
> +	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
>  		rc = -EFAULT;
>  		goto out;
>  	}
> @@ -621,7 +597,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  	u16 data_len;
>  	u8 *data;
>  	int rc;
> -	u32 rlength;
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
>  	if (rc)
> @@ -649,9 +624,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  			goto out;
>  		}
>  
> -		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
> -					->header.out.length);
> -		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
> +		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
>  			rc = -EFAULT;
>  			goto out;
>  		}
> -- 
> 2.15.1
> 

tpm_buf_destroy() missing

/Jarkko