tpm: Move Linux RNG connection to hwrng

The tpm-rng.c approach is completely inconsistent with how the kernel handles hotplug. Instead manage a hwrng device for each TPM. This will cause the kernel to read entropy from the TPM when it is plugged in, and allow access to the TPM rng via /dev/hwrng.

Signed-off-by PrasannaKumar Muralidharan <[email protected]>

Signed-off-by Jason Gunthorpe <[email protected]>

---
 drivers/char/hw_random/Kconfig   | 13 -----------
 drivers/char/hw_random/Makefile  |  1 -
 drivers/char/hw_random/tpm-rng.c | 50 ----------------------------------------
 drivers/char/tpm/Kconfig         | 11 +++++++++
 drivers/char/tpm/tpm-chip.c      | 41 ++++++++++++++++++++++++++++----
 drivers/char/tpm/tpm.h           |  4 ++++
 6 files changed, 52 insertions(+), 68 deletions(-)
 delete mode 100644 drivers/char/hw_random/tpm-rng.c

v2 applies against Jarkko's patch
"tpm: use struct tpm_chip for tpm_chip_find_get()"

drivers/char/hw_random/Kconfig +0 -13

index 95a031e9eced07..a20fed182cbcce 100644
@@ -306,19 +306,6 @@ config HW_RANDOM_POWERNV
 
 	  If unsure, say Y.
 
-config HW_RANDOM_TPM
-	tristate "TPM HW Random Number Generator support"
-	depends on TCG_TPM
-	default HW_RANDOM
-	---help---
-	  This driver provides kernel-side support for the Random Number
-	  Generator in the Trusted Platform Module
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called tpm-rng.
-
-	  If unsure, say Y.
-
 config HW_RANDOM_HISI
 	tristate "Hisilicon Random Number Generator support"
 	depends on HW_RANDOM && ARCH_HISI

drivers/char/hw_random/Makefile +0 -1

index 39a67defac67cb..91cb8e8213e7c1 100644
@@ -26,7 +26,6 @@ obj-$(CONFIG_HW_RANDOM_NOMADIK) += nomadik-rng.o
 obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
 obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o
 obj-$(CONFIG_HW_RANDOM_HISI)	+= hisi-rng.o
-obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
 obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
 obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o
 obj-$(CONFIG_HW_RANDOM_MSM) += msm-rng.o

drivers/char/hw_random/tpm-rng.c deleted +0 -50

deleted file mode 100644
index c5e363825af008..00000000000000
@@ -1,50 +0,0 @@
-/*
- * Copyright (C) 2012 Kent Yoder IBM Corporation
- *
- * HWRNG interfaces to pull RNG data from a TPM
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
- */
-
-#include <linux/module.h>
-#include <linux/hw_random.h>
-#include <linux/tpm.h>
-
-#define MODULE_NAME "tpm-rng"
-
-static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
-{
-	return tpm_get_random(NULL, data, max);
-}
-
-static struct hwrng tpm_rng = {
-	.name = MODULE_NAME,
-	.read = tpm_rng_read,
-};
-
-static int __init rng_init(void)
-{
-	return hwrng_register(&tpm_rng);
-}
-module_init(rng_init);
-
-static void __exit rng_exit(void)
-{
-	hwrng_unregister(&tpm_rng);
-}
-module_exit(rng_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Kent Yoder <[email protected]>");
-MODULE_DESCRIPTION("RNG driver for TPM devices");

drivers/char/tpm/Kconfig +11 -0

index a30352202f1fdc..18c81cbe4704ca 100644
@@ -26,6 +26,17 @@ menuconfig TCG_TPM
 
 if TCG_TPM
 
+config HW_RANDOM_TPM
+	bool "TPM HW Random Number Generator support"
+	depends on TCG_TPM && HW_RANDOM && !(TCG_TPM=y && HW_RANDOM=m)
+	default y
+	---help---
+	  This setting exposes the TPM's Random Number Generator as a hwrng
+	  device. This allows the kernel to collect randomness from the TPM at
+	  boot, and provides the TPM randomines in /dev/hwrng.
+
+	  If unsure, say Y.
+
 config TCG_TIS_CORE
 	tristate
 	---help---

drivers/char/tpm/tpm-chip.c +37 -4

index 6faf4c3fa6dbbd..797ab962291333 100644
@@ -26,6 +26,7 @@
 #include <linux/spinlock.h>
 #include <linux/freezer.h>
 #include <linux/major.h>
+#include <linux/hw_random.h>
 #include "tpm.h"
 #include "tpm_eventlog.h"
 

@@ -391,6 +392,26 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
 
 	return 0;
 }
+
+static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
+{
+	struct tpm_chip *chip = container_of(rng, struct tpm_chip, hwrng);
+
+	return tpm_get_random(chip, data, max);
+}
+
+static int tpm_add_hwrng(struct tpm_chip *chip)
+{
+	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
+		return 0;
+
+	snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
+		 "tpm-rng-%d", chip->dev_num);
+	chip->hwrng.name = chip->hwrng_name;
+	chip->hwrng.read = tpm_hwrng_read;
+	return hwrng_register(&chip->hwrng);
+}
+
 /*
  * tpm_chip_register() - create a character device for the TPM chip
  * @chip: TPM chip to use.

@@ -423,11 +444,13 @@ int tpm_chip_register(struct tpm_chip *chip)
 
 	tpm_add_ppi(chip);
 
+	rc = tpm_add_hwrng(chip);
+	if (rc)
+		goto out_ppi;
+
 	rc = tpm_add_char_device(chip);
-	if (rc) {
-		tpm_bios_log_teardown(chip);
-		return rc;
-	}
+	if (rc)
+		goto out_hwrng;
 
 	rc = tpm_add_legacy_sysfs(chip);
 	if (rc) {

@@ -436,6 +459,14 @@ int tpm_chip_register(struct tpm_chip *chip)
 	}
 
 	return 0;
+
+out_hwrng:
+	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM))
+		hwrng_unregister(&chip->hwrng);
+out_ppi:
+	tpm_bios_log_teardown(chip);
+
+	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_chip_register);
 

@@ -455,6 +486,8 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
 void tpm_chip_unregister(struct tpm_chip *chip)
 {
 	tpm_del_legacy_sysfs(chip);
+	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM))
+		hwrng_unregister(&chip->hwrng);
 	tpm_bios_log_teardown(chip);
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		cdev_device_del(&chip->cdevs, &chip->devs);

drivers/char/tpm/tpm.h +4 -0

index 7424511241031a..efe8ad0d137f74 100644
@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/fs.h>
+#include <linux/hw_random.h>
 #include <linux/mutex.h>
 #include <linux/sched.h>
 #include <linux/platform_device.h>

@@ -210,6 +211,9 @@ struct tpm_chip {
 	int dev_num;		/* /dev/tpm# */
 	unsigned long is_open;	/* only one allowed */
 
+	char hwrng_name[64];
+	struct hwrng hwrng;
+
 	struct mutex tpm_mutex;	/* tpm is processing */
 
 	unsigned long timeout_a; /* jiffies */

Signature · 1 line

2.7.4

I asked to create a series for a reason. Now this doesn't apply because I don't have an ancestor in my git history.

Please resend as series together with my patch. I can apply neither yet because they have zero tested-by's.

Quoted 2 lines · Jarkko Sakkinen

I asked to create a series for a reason. Now this doesn't apply because I don't have an ancestor in my git history.

It would be unusual for me to put your patch into a series unless I am also adopting it. eg what happens if there are more comments on it?

Also, I wasn't sure what branch your patch was against since my tree didn't have history for it either..

Sometimes the maintainer has to sort stuff like this out... :)

Quoted 2 lines

Please resend as series together with my patch. I can apply neither yet because they have zero tested-by's.

Hopefully PrasannaKumar can test both patches.

I am assuming you are talking about the following patches - using struct tpm_chip instead of chip number and this patch.

I won't be able to test if struct tpm_chip usage as I don't have multiple tpm hw in one machine. In case of tpm rng changes I can test only the lifecycle of tpm rng device. Is that enough? I feel my test will be limited. Please provide your thoughts on this.

Quoted 2 lines · PrasannaKumar Muralidharan

I am assuming you are talking about the following patches - using struct tpm_chip instead of chip number and this patch.

yes

Quoted 4 lines

I won't be able to test if struct tpm_chip usage as I don't have multiple tpm hw in one machine. In case of tpm rng changes I can test only the lifecycle of tpm rng device. Is that enough? I feel my test will be limited. Please provide your thoughts on this.

That is certainly better than no testing.

Hi Jason,

Quoted 5 lines · Jason Gunthorpe

Quoted 2 lines · PrasannaKumar Muralidharan

I am assuming you are talking about the following patches - using struct tpm_chip instead of chip number and this patch.

Quoted 8 lines

yes

Quoted 4 lines

I won't be able to test if struct tpm_chip usage as I don't have multiple tpm hw in one machine. In case of tpm rng changes I can test only the lifecycle of tpm rng device. Is that enough? I feel my test will be limited. Please provide your thoughts on this.

That is certainly better than no testing.

The struct tpm_chip patch partially applied on linux next. I had to manually change the code. In qemu tpm rng device did not show up on loading tpm module. My laptop has tpm hw but Linux next did not work properly in that. All my console were getting spammed with some USB log message and I could not do anything. X did not start either. I could not debug the issue as the logs were printing infinitely. Will get little more time this weekend. Will do a proper test and provide you the result.

Quoted 17 lines · PrasannaKumar Muralidharan

Hi Jason,

Quoted 5 lines · Jason Gunthorpe

Quoted 2 lines · PrasannaKumar Muralidharan

I am assuming you are talking about the following patches - using struct tpm_chip instead of chip number and this patch.

Quoted 8 lines

yes

Quoted 4 lines

I won't be able to test if struct tpm_chip usage as I don't have multiple tpm hw in one machine. In case of tpm rng changes I can test only the lifecycle of tpm rng device. Is that enough? I feel my test will be limited. Please provide your thoughts on this.

That is certainly better than no testing.

Quoted 8 lines

The struct tpm_chip patch partially applied on linux next. I had to manually change the code. In qemu tpm rng device did not show up on loading tpm module. My laptop has tpm hw but Linux next did not work properly in that. All my console were getting spammed with some USB log message and I could not do anything. X did not start either. I could not debug the issue as the logs were printing infinitely. Will get little more time this weekend. Will do a proper test and provide you the result.

Test against 4.15-rc, here are the two patches

https://github.com/jgunthorpe/linux/tree/tpm

Pull from here and merge the latest rc and you will probably have a bootable system.

Jason

Quoted 2 lines · PrasannaKumar Muralidharan

I am assuming you are talking about the following patches - using struct tpm_chip instead of chip number and this patch.

yes

Quoted 4 lines

I won't be able to test if struct tpm_chip usage as I don't have multiple tpm hw in one machine. In case of tpm rng changes I can test only the lifecycle of tpm rng device. Is that enough? I feel my test will be limited. Please provide your thoughts on this.

That is certainly better than no testing.

Jason

Quoted 14 lines · Jason Gunthorpe

Quoted 2 lines · PrasannaKumar Muralidharan

I am assuming you are talking about the following patches - using struct tpm_chip instead of chip number and this patch.

yes

Quoted 4 lines

I won't be able to test if struct tpm_chip usage as I don't have multiple tpm hw in one machine. In case of tpm rng changes I can test only the lifecycle of tpm rng device. Is that enough? I feel my test will be limited. Please provide your thoughts on this.

That is certainly better than no testing.

Quoted 1 line

Jason

WFM too.

Tested-by Jarkko Sakkinen <[email protected]>

Reviewed-by Jarkko Sakkinen <[email protected]>

/Jarkko

Quoted 22 lines · Jarkko Sakkinen

Quoted 7 lines · Jason Gunthorpe

Quoted 2 lines · PrasannaKumar Muralidharan

I am assuming you are talking about the following patches - using struct tpm_chip instead of chip number and this patch.

yes

Quoted 8 lines

Quoted 4 lines

I won't be able to test if struct tpm_chip usage as I don't have multiple tpm hw in one machine. In case of tpm rng changes I can test only the lifecycle of tpm rng device. Is that enough? I feel my test will be limited. Please provide your thoughts on this.

That is certainly better than no testing.

Jason

WFM too.

Tested-by Jarkko Sakkinen <[email protected]>

Reviewed-by Jarkko Sakkinen <[email protected]>

Quoted 1 line

/Jarkko

I applied these commits. Please check that everything is correct as I had to do manual work with the 2nd commit.

/Jarkko

On Fri, Nov 17, 2017 at 03:28:53PM +0200, Jarkko Sakkinen wrote:

At least signed-off-by from PrassanaKumar is missing from the 2nd commit. I'll add it.

Hi Jarkko,

Quoted 4 lines · Jarkko Sakkinen

On Fri, Nov 17, 2017 at 03:28:53PM +0200, Jarkko Sakkinen wrote:

At least signed-off-by from PrassanaKumar is missing from the 2nd commit. I'll add it.

I had the impression that my signed-off-by will be present in this change. But it is missing in [1]. Is it supposed to be that way?

1. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?idn592a065d51d26f9d62b8b7501a5114076af8b4

Thanks,
PrasannaKumar

Quoted 12 lines · PrasannaKumar Muralidharan

Hi Jarkko,

Quoted 4 lines · Jarkko Sakkinen

On Fri, Nov 17, 2017 at 03:28:53PM +0200, Jarkko Sakkinen wrote:

At least signed-off-by from PrassanaKumar is missing from the 2nd commit. I'll add it.

I had the impression that my signed-off-by will be present in this change. But it is missing in [1]. Is it supposed to be that way?

Quoted 4 lines

1. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?idn592a065d51d26f9d62b8b7501a5114076af8b4

Thanks,
PrasannaKumar

Yes, it would be senseful.

James, would it still be possible to amend this tag to security tree?

Quoted 15 lines · Jarkko Sakkinen

Quoted 14 lines · PrasannaKumar Muralidharan

Hi Jarkko,

Quoted 4 lines · Jarkko Sakkinen

On Fri, Nov 17, 2017 at 03:28:53PM +0200, Jarkko Sakkinen wrote:

At least signed-off-by from PrassanaKumar is missing from the 2nd commit. I'll add it.

I had the impression that my signed-off-by will be present in this change. But it is missing in [1]. Is it supposed to be that way?

1. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?idn592a065d51d26f9d62b8b7501a5114076af8b4

Quoted 6 lines

Quoted 2 lines

Thanks,
PrasannaKumar

Yes, it would be senseful.

James, would it still be possible to amend this tag to security tree?

Nope, it's been pushed to Linus.

Quoted 2 lines · Jarkko Sakkinen

I asked to create a series for a reason. Now this doesn't apply because I don't have an ancestor in my git history.

It would be unusual for me to put your patch into a series unless I am also adopting it. eg what happens if there are more comments on it?

Also, I wasn't sure what branch your patch was against since my tree didn't have history for it either..

Sometimes the maintainer has to sort stuff like this out... :)

Quoted 2 lines

Please resend as series together with my patch. I can apply neither yet because they have zero tested-by's.

Hopefully PrasannaKumar can test both patches.

Jason