diff mbox series

[1/3] tpm: add generic platform device

Message ID 20241210143423.101774-2-sgarzare@redhat.com (mailing list archive)
State New
Headers show
Series Enlightened vTPM support for SVSM on SEV-SNP | expand

Commit Message

Stefano Garzarella Dec. 10, 2024, 2:34 p.m. UTC
From: James Bottomley <James.Bottomley@HansenPartnership.com>

This is primarily designed to support an enlightened driver for the
AMD svsm based vTPM, but it could be used by any platform which
communicates with a TPM device.  The platform must fill in struct
tpm_platform_ops as the platform_data and set the device name to "tpm"
to have the binding by name work correctly.  The sole sendrcv
function is designed to do a single buffer request/response conforming
to the MSSIM protocol.  For the svsm vTPM case, this protocol is
transmitted directly to the SVSM, but it could be massaged for other
function type platform interfaces.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
[SG] changed references/links to TCG TPM repo
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/linux/tpm_platform.h    |  90 ++++++++++++++++++++
 drivers/char/tpm/tpm_platform.c | 141 ++++++++++++++++++++++++++++++++
 drivers/char/tpm/Kconfig        |   7 ++
 drivers/char/tpm/Makefile       |   1 +
 4 files changed, 239 insertions(+)
 create mode 100644 include/linux/tpm_platform.h
 create mode 100644 drivers/char/tpm/tpm_platform.c

Comments

Stefano Garzarella Dec. 12, 2024, 9:51 a.m. UTC | #1
On Tue, Dec 10, 2024 at 03:34:21PM +0100, Stefano Garzarella wrote:
>From: James Bottomley <James.Bottomley@HansenPartnership.com>
>
>This is primarily designed to support an enlightened driver for the
>AMD svsm based vTPM, but it could be used by any platform which
>communicates with a TPM device.  The platform must fill in struct
>tpm_platform_ops as the platform_data and set the device name to "tpm"
>to have the binding by name work correctly.  The sole sendrcv
>function is designed to do a single buffer request/response conforming
>to the MSSIM protocol.  For the svsm vTPM case, this protocol is
>transmitted directly to the SVSM, but it could be massaged for other
>function type platform interfaces.
>
>Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>[SG] changed references/links to TCG TPM repo
>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>---
> include/linux/tpm_platform.h    |  90 ++++++++++++++++++++
> drivers/char/tpm/tpm_platform.c | 141 ++++++++++++++++++++++++++++++++
> drivers/char/tpm/Kconfig        |   7 ++
> drivers/char/tpm/Makefile       |   1 +
> 4 files changed, 239 insertions(+)
> create mode 100644 include/linux/tpm_platform.h
> create mode 100644 drivers/char/tpm/tpm_platform.c
>
>diff --git a/include/linux/tpm_platform.h b/include/linux/tpm_platform.h
>new file mode 100644
>index 000000000000..95c17a75d59d
>--- /dev/null
>+++ b/include/linux/tpm_platform.h
>@@ -0,0 +1,90 @@
>+/* SPDX-License-Identifier: GPL-2.0-only */
>+/*
>+ * Copyright (C) 2023 James.Bottomley@HansenPartnership.com
>+ *
>+ * Interface specification for platforms wishing to activate the
>+ * platform tpm device.  The device must be a platform device created
>+ * with the name "tpm" and it must populate platform_data with struct
>+ * tpm_platform_ops
>+ */
>+
>+/*
>+ * The current MSSIM TPM commands we support.  The complete list is
>+ * in the TcpTpmProtocol header:
>+ *
>+ * https://github.com/TrustedComputingGroup/TPM/blob/main/TPMCmd/Simulator/include/TpmTcpProtocol.h
>+ */
>+
>+#define TPM_SEND_COMMAND		8
>+#define TPM_SIGNAL_CANCEL_ON		9
>+#define TPM_SIGNAL_CANCEL_OFF		10
>+/*
>+ * Any platform specific commands should be placed here and should start
>+ * at 0x8000 to avoid clashes with the MSSIM protocol.  They should follow
>+ * the same self describing buffer format below
>+ */
>+
>+#define TPM_PLATFORM_MAX_BUFFER		4096 /* max req/resp buffer size */
>+
>+/**
>+ * struct tpm_platform_ops - the share platform operations
>+ *
>+ * @sendrcv:	Send a TPM command using the MSSIM protocol.
>+ *
>+ * The MSSIM protocol is designed for a network, so the buffers are
>+ * self describing.  The minimum buffer size is sizeof(u32).  Every
>+ * MSSIM command defines its own transport buffer and the command is
>+ * sent in the first u32 array.  The only modification we make is that
>+ * the MSSIM uses network order and we use the endianness of the
>+ * architecture.  The response to every command (in the same buffer)
>+ * is a u32 size preceded array.  Most of the MSSIM commands simply
>+ * return zero here because they have no defined response.
>+ *
>+ * The only command with a defined request/response size is TPM_SEND_COMMAND
>+ * The definition is in the structures below
>+ */
>+struct tpm_platform_ops {
>+	int (*sendrcv)(u8 *buffer);
>+};
>+
>+/**
>+ * struct tpm_send_cmd_req - Structure for a TPM_SEND_COMMAND
>+ *
>+ * @cmd:	The command (must be TPM_SEND_COMMAND)
>+ * @locality:	The locality
>+ * @inbuf_size:	The size of the input buffer following
>+ * @inbuf:	A buffer of size inbuf_size
>+ *
>+ * Note that MSSIM expects @inbuf_size to be equal to the size of the
>+ * specific TPM command, otherwise an TPM_RC_COMMAND_SIZE error is
>+ * returned.
>+ */
>+struct tpm_send_cmd_req {
>+	u32	cmd;
>+	u8	locality;
>+	u32	inbuf_size;
>+	u8	inbuf[];
>+} __packed;
>+
>+/**
>+ * struct tpm_req - generic request header for single word command
>+ *
>+ * @cmd:	The command to send
>+ */
>+struct tpm_req {
>+	u32	cmd;
>+} __packed;
>+
>+/**
>+ * struct tpm_resp - generic response header
>+ *
>+ * @size:	The response size (zero if nothing follows)
>+ *
>+ * Note: most MSSIM commands simply return zero here with no indication
>+ * of success or failure.
>+ */
>+
>+struct tpm_resp {
>+	s32	size;
>+} __packed;
>+
>diff --git a/drivers/char/tpm/tpm_platform.c b/drivers/char/tpm/tpm_platform.c
>new file mode 100644
>index 000000000000..b53d74344d61
>--- /dev/null
>+++ b/drivers/char/tpm/tpm_platform.c
>@@ -0,0 +1,141 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/*
>+ * Platform based TPM emulator
>+ *
>+ * Copyright (C) 2023 James.Bottomley@HansenPartnership.com
>+ *
>+ * Designed to handle a simple function request/response single buffer
>+ * TPM or vTPM rooted in the platform.  This device driver uses the
>+ * MSSIM protocol from the official TCG reference implementation
>+ *
>+ * https://github.com/TrustedComputingGroup/TPM
>+ *
>+ * to communicate between the driver and the platform.  This is rich
>+ * enough to allow platform operations like cancellation The platform
>+ * should not act on platform commands like power on/off and reset
>+ * which can disrupt the TPM guarantees.
>+ *
>+ * This driver is designed to be single threaded (one call in to the
>+ * platform TPM at any one time).  The threading guarantees are
>+ * provided by the chip mutex.
>+ */
>+
>+#include <linux/module.h>
>+#include <linux/kernel.h>
>+#include <linux/platform_device.h>
>+#include <linux/tpm_platform.h>
>+
>+#include "tpm.h"
>+
>+static struct tpm_platform_ops *pops;
>+
>+static u8 *buffer;
>+/*
>+ * FIXME: before implementing locality we need to agree what it means
>+ * to the platform
>+ */
>+static u8 locality;
>+
>+static int tpm_platform_send(struct tpm_chip *chip, u8 *buf, size_t len)
>+{
>+	int ret;
>+	struct tpm_send_cmd_req *req = (struct tpm_send_cmd_req *)buffer;
>+
>+	if (len > TPM_PLATFORM_MAX_BUFFER - sizeof(*req))
>+		return -EINVAL;
>+	req->cmd = TPM_SEND_COMMAND;
>+	req->locality = locality;
>+	req->inbuf_size = len;
>+	memcpy(req->inbuf, buf, len);
>+
>+	ret = pops->sendrcv(buffer);
>+	if (ret)
>+		return ret;
>+
>+	return 0;
>+}
>+
>+static int tpm_platform_recv(struct tpm_chip *chip, u8 *buf, size_t len)
>+{
>+	struct tpm_resp *resp = (struct tpm_resp *)buffer;
>+
>+	if (resp->size < 0)
>+		return resp->size;

While reviewing Oliver's work for the driver in edk2[1], I noticed that
there wasn't this check and asked to add it, but talking to him and
looking in the code/spec, we realized that it's strange that
tpm_resp.size field is signed.

 From SVSM spec it looks like it can't be negative:

     Table 17: TPM_SEND_COMMAND Response Structure

     Byte     Size        Meaning
     Offset   (Bytes)
     0x000    4           Response size (in bytes)
     0x004    Variable    Variable Response

And also Coconut SVSM remap it to the `responseSize` of the TCG TPM
implementation which is unsigned:

     LIB_EXPORT void _plat__RunCommand(
         uint32_t        requestSize,   // IN: command buffer size
         unsigned char*  request,       // IN: command buffer
         uint32_t*       responseSize,  // IN/OUT: response buffer size
         unsigned char** response       // IN/OUT: response buffer
     )

@James, @Claudio, @Tom, should we use u32 for tpm_resp.size?

Thanks,
Stefano

[1] https://github.com/tianocore/edk2/pull/6527#discussion_r1880204144

>+
>+	if (len < resp->size)
>+		return -E2BIG;
>+
>+	if (resp->size > TPM_PLATFORM_MAX_BUFFER - sizeof(*resp))
>+		return -EINVAL;  // Invalid response from the platform TPM
>+
>+	memcpy(buf, buffer + sizeof(*resp), resp->size);
>+
>+	return resp->size;
>+}
>+
>+static struct tpm_class_ops tpm_chip_ops = {
>+	.flags = TPM_OPS_AUTO_STARTUP,
>+	.send = tpm_platform_send,
>+	.recv = tpm_platform_recv,
>+};
>+
>+static struct platform_driver tpm_platform_driver = {
>+	.driver = {
>+		.name = "tpm",
>+	},
>+};
>+
>+static int __init tpm_platform_probe(struct platform_device *pdev)
>+{
>+	struct device *dev = &pdev->dev;
>+	struct tpm_chip *chip;
>+	int err;
>+
>+	if (!dev->platform_data)
>+		return -ENODEV;
>+
>+	/*
>+	 * in theory platform matching should mean this is always
>+	 * true, but just in case anyone tries force binding
>+	 */
>+	if (strcmp(pdev->name, tpm_platform_driver.driver.name) != 0)
>+		return -ENODEV;
>+
>+	if (!buffer)
>+		buffer = kmalloc(TPM_PLATFORM_MAX_BUFFER, GFP_KERNEL);
>+
>+	if (!buffer)
>+		return -ENOMEM;
>+
>+	pops = dev->platform_data;
>+
>+	chip = tpmm_chip_alloc(dev, &tpm_chip_ops);
>+	if (IS_ERR(chip))
>+		return PTR_ERR(chip);
>+
>+	/*
>+	 * Setting TPM_CHIP_FLAG_IRQ guarantees that ->recv will be
>+	 * called straight after ->send and means we don't need to
>+	 * implement any other chip ops.
>+	 */
>+	chip->flags |= TPM_CHIP_FLAG_IRQ;
>+	err = tpm2_probe(chip);
>+	if (err)
>+		return err;
>+
>+	err = tpm_chip_register(chip);
>+	if (err)
>+		return err;
>+
>+	dev_info(dev, "TPM %s platform device\n",
>+		 (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2");
>+
>+	return 0;
>+}
>+
>+module_platform_driver_probe(tpm_platform_driver, tpm_platform_probe);
>+
>+MODULE_AUTHOR("James Bottomley <James.Bottomley@HansenPartnership.com>");
>+MODULE_LICENSE("GPL");
>+MODULE_DESCRIPTION("Platform TPM Driver");
>+MODULE_ALIAS("platform:tpm");
>diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
>index 0fc9a510e059..b162f59305ef 100644
>--- a/drivers/char/tpm/Kconfig
>+++ b/drivers/char/tpm/Kconfig
>@@ -225,5 +225,12 @@ config TCG_FTPM_TEE
> 	help
> 	  This driver proxies for firmware TPM running in TEE.
>
>+config TCG_PLATFORM
>+	tristate "Platform TPM Device"
>+	help
>+	  This driver requires a platform implementation to provide the
>+	  TPM function.  It will not bind if the implementation is not
>+	  present.
>+
> source "drivers/char/tpm/st33zp24/Kconfig"
> endif # TCG_TPM
>diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
>index 9bb142c75243..4b2c04e23bd3 100644
>--- a/drivers/char/tpm/Makefile
>+++ b/drivers/char/tpm/Makefile
>@@ -44,3 +44,4 @@ obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
> obj-$(CONFIG_TCG_CRB) += tpm_crb.o
> obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
> obj-$(CONFIG_TCG_FTPM_TEE) += tpm_ftpm_tee.o
>+obj-$(CONFIG_TCG_PLATFORM) += tpm_platform.o
>-- 
>2.47.1
>
James Bottomley Dec. 12, 2024, 2:35 p.m. UTC | #2
On Thu, 2024-12-12 at 10:51 +0100, Stefano Garzarella wrote:
> On Tue, Dec 10, 2024 at 03:34:21PM +0100, Stefano Garzarella wrote:
[...]
> > +static int tpm_platform_recv(struct tpm_chip *chip, u8 *buf,
> > size_t len)
> > +{
> > +       struct tpm_resp *resp = (struct tpm_resp *)buffer;
> > +
> > +       if (resp->size < 0)
> > +               return resp->size;
> 
> While reviewing Oliver's work for the driver in edk2[1], I noticed
> that
> there wasn't this check and asked to add it, but talking to him and
> looking in the code/spec, we realized that it's strange that
> tpm_resp.size field is signed.
> 
>  From SVSM spec it looks like it can't be negative:
> 
>      Table 17: TPM_SEND_COMMAND Response Structure
> 
>      Byte     Size        Meaning
>      Offset   (Bytes)
>      0x000    4           Response size (in bytes)
>      0x004    Variable    Variable Response
> 
> And also Coconut SVSM remap it to the `responseSize` of the TCG TPM
> implementation which is unsigned:
> 
>      LIB_EXPORT void _plat__RunCommand(
>          uint32_t        requestSize,   // IN: command buffer size
>          unsigned char*  request,       // IN: command buffer
>          uint32_t*       responseSize,  // IN/OUT: response buffer
> size
>          unsigned char** response       // IN/OUT: response buffer
>      )
> 
> @James, @Claudio, @Tom, should we use u32 for tpm_resp.size?

The original idea was to allow the protocol to return an error (like
out of memory or something) before the command ever got to the TPM
rather than having to wrap it up in a TPM error.  However, that's done
in the actual return from the SVSM call, which the sendrecv routine
checks, so I agree this can be removed and a u32 done for the length. 
Dov did recommend we should check the returned length against the
maximum allowable:

https://lore.kernel.org/linux-coco/f7d0bd07-ba1b-894e-5e39-15fb1817bc8b@linux.ibm.com/

Regards,

James
Stefano Garzarella Dec. 12, 2024, 3:30 p.m. UTC | #3
On Thu, Dec 12, 2024 at 09:35:46AM -0500, James Bottomley wrote:
>On Thu, 2024-12-12 at 10:51 +0100, Stefano Garzarella wrote:
>> On Tue, Dec 10, 2024 at 03:34:21PM +0100, Stefano Garzarella wrote:
>[...]
>> > +static int tpm_platform_recv(struct tpm_chip *chip, u8 *buf,
>> > size_t len)
>> > +{
>> > +       struct tpm_resp *resp = (struct tpm_resp *)buffer;
>> > +
>> > +       if (resp->size < 0)
>> > +               return resp->size;
>>
>> While reviewing Oliver's work for the driver in edk2[1], I noticed
>> that
>> there wasn't this check and asked to add it, but talking to him and
>> looking in the code/spec, we realized that it's strange that
>> tpm_resp.size field is signed.
>>
>>  From SVSM spec it looks like it can't be negative:
>>
>>      Table 17: TPM_SEND_COMMAND Response Structure
>>
>>      Byte     Size        Meaning
>>      Offset   (Bytes)
>>      0x000    4           Response size (in bytes)
>>      0x004    Variable    Variable Response
>>
>> And also Coconut SVSM remap it to the `responseSize` of the TCG TPM
>> implementation which is unsigned:
>>
>>      LIB_EXPORT void _plat__RunCommand(
>>          uint32_t        requestSize,   // IN: command buffer size
>>          unsigned char*  request,       // IN: command buffer
>>          uint32_t*       responseSize,  // IN/OUT: response buffer
>> size
>>          unsigned char** response       // IN/OUT: response buffer
>>      )
>>
>> @James, @Claudio, @Tom, should we use u32 for tpm_resp.size?
>
>The original idea was to allow the protocol to return an error (like
>out of memory or something) before the command ever got to the TPM
>rather than having to wrap it up in a TPM error.  However, that's done
>in the actual return from the SVSM call, which the sendrecv routine
>checks, so I agree this can be removed and a u32 done for the length.

Thanks for the details!
I'll fix it in v2 and put a comment also in the edk2 PR.

>Dov did recommend we should check the returned length against the
>maximum allowable:
>
>https://lore.kernel.org/linux-coco/f7d0bd07-ba1b-894e-5e39-15fb1817bc8b@linux.ibm.com/

I added in this version the check he suggested:

	if (resp->size > TPM_PLATFORM_MAX_BUFFER - sizeof(*resp))
		return -EINVAL;  // Invalid response from the platform TPM

Were you referring to that?

Thanks,
Stefano
James Bottomley Dec. 12, 2024, 3:41 p.m. UTC | #4
On Thu, 2024-12-12 at 16:30 +0100, Stefano Garzarella wrote:
> On Thu, Dec 12, 2024 at 09:35:46AM -0500, James Bottomley wrote:
> > On Thu, 2024-12-12 at 10:51 +0100, Stefano Garzarella wrote:
> > > On Tue, Dec 10, 2024 at 03:34:21PM +0100, Stefano Garzarella
> > > wrote:
> > [...]
> > > > +static int tpm_platform_recv(struct tpm_chip *chip, u8 *buf,
> > > > size_t len)
> > > > +{
> > > > +       struct tpm_resp *resp = (struct tpm_resp *)buffer;
> > > > +
> > > > +       if (resp->size < 0)
> > > > +               return resp->size;
> > > 
> > > While reviewing Oliver's work for the driver in edk2[1], I
> > > noticed that there wasn't this check and asked to add it, but
> > > talking to him and looking in the code/spec, we realized that
> > > it's strange that tpm_resp.size field is signed.
> > > 
> > >  From SVSM spec it looks like it can't be negative:
> > > 
> > >      Table 17: TPM_SEND_COMMAND Response Structure
> > > 
> > >      Byte     Size        Meaning
> > >      Offset   (Bytes)
> > >      0x000    4           Response size (in bytes)
> > >      0x004    Variable    Variable Response
> > > 
> > > And also Coconut SVSM remap it to the `responseSize` of the TCG
> > > TPM implementation which is unsigned:
> > > 
> > >      LIB_EXPORT void _plat__RunCommand(
> > >          uint32_t        requestSize,   // IN: command buffer size
> > >          unsigned char*  request,       // IN: command buffer
> > >          uint32_t*       responseSize,  // IN/OUT: response buffer
> > > size
> > >          unsigned char** response       // IN/OUT: response buffer
> > >      )
> > > 
> > > @James, @Claudio, @Tom, should we use u32 for tpm_resp.size?
> > 
> > The original idea was to allow the protocol to return an error
> > (like out of memory or something) before the command ever got to
> > the TPM rather than having to wrap it up in a TPM error.  However,
> > that's done in the actual return from the SVSM call, which the
> > sendrecv routine checks, so I agree this can be removed and a u32
> > done for the length.
> 
> Thanks for the details!
> I'll fix it in v2 and put a comment also in the edk2 PR.
> 
> > Dov did recommend we should check the returned length against the
> > maximum allowable:
> > 
> > https://lore.kernel.org/linux-coco/f7d0bd07-ba1b-894e-5e39-15fb1817bc8b@linux.ibm.com/
> 
> I added in this version the check he suggested:
> 
>         if (resp->size > TPM_PLATFORM_MAX_BUFFER - sizeof(*resp))
>                 return -EINVAL;  // Invalid response from the
> platform TPM
> 
> Were you referring to that?

Yes, the theory being that we're required to provide a buffer of this
length for the response, but if someone can inject a bogus response
they could induce us to copy beyond the end of the buffer we provided.

Regards,

James
Stefano Garzarella Dec. 12, 2024, 4:12 p.m. UTC | #5
On Thu, Dec 12, 2024 at 10:41:40AM -0500, James Bottomley wrote:
>On Thu, 2024-12-12 at 16:30 +0100, Stefano Garzarella wrote:
>> On Thu, Dec 12, 2024 at 09:35:46AM -0500, James Bottomley wrote:
>> > On Thu, 2024-12-12 at 10:51 +0100, Stefano Garzarella wrote:
>> > > On Tue, Dec 10, 2024 at 03:34:21PM +0100, Stefano Garzarella
>> > > wrote:
>> > [...]
>> > > > +static int tpm_platform_recv(struct tpm_chip *chip, u8 *buf,
>> > > > size_t len)
>> > > > +{
>> > > > +       struct tpm_resp *resp = (struct tpm_resp *)buffer;
>> > > > +
>> > > > +       if (resp->size < 0)
>> > > > +               return resp->size;
>> > >
>> > > While reviewing Oliver's work for the driver in edk2[1], I
>> > > noticed that there wasn't this check and asked to add it, but
>> > > talking to him and looking in the code/spec, we realized that
>> > > it's strange that tpm_resp.size field is signed.
>> > >
>> > >  From SVSM spec it looks like it can't be negative:
>> > >
>> > >      Table 17: TPM_SEND_COMMAND Response Structure
>> > >
>> > >      Byte     Size        Meaning
>> > >      Offset   (Bytes)
>> > >      0x000    4           Response size (in bytes)
>> > >      0x004    Variable    Variable Response
>> > >
>> > > And also Coconut SVSM remap it to the `responseSize` of the TCG
>> > > TPM implementation which is unsigned:
>> > >
>> > >      LIB_EXPORT void _plat__RunCommand(
>> > >          uint32_t        requestSize,   // IN: command buffer size
>> > >          unsigned char*  request,       // IN: command buffer
>> > >          uint32_t*       responseSize,  // IN/OUT: response buffer
>> > > size
>> > >          unsigned char** response       // IN/OUT: response buffer
>> > >      )
>> > >
>> > > @James, @Claudio, @Tom, should we use u32 for tpm_resp.size?
>> >
>> > The original idea was to allow the protocol to return an error
>> > (like out of memory or something) before the command ever got to
>> > the TPM rather than having to wrap it up in a TPM error.  However,
>> > that's done in the actual return from the SVSM call, which the
>> > sendrecv routine checks, so I agree this can be removed and a u32
>> > done for the length.
>>
>> Thanks for the details!
>> I'll fix it in v2 and put a comment also in the edk2 PR.
>>
>> > Dov did recommend we should check the returned length against the
>> > maximum allowable:
>> >
>> > https://lore.kernel.org/linux-coco/f7d0bd07-ba1b-894e-5e39-15fb1817bc8b@linux.ibm.com/
>>
>> I added in this version the check he suggested:
>>
>>         if (resp->size > TPM_PLATFORM_MAX_BUFFER - sizeof(*resp))
>>                 return -EINVAL;  // Invalid response from the
>> platform TPM
>>
>> Were you referring to that?
>
>Yes, the theory being that we're required to provide a buffer of this
>length for the response, but if someone can inject a bogus response
>they could induce us to copy beyond the end of the buffer we provided.

I see, but we alread check that `len < resp->size` in
tpm_platform_recv(), so on second glance, for the current
implementation, maybe it's a duplicate check.

This because in tpm_platform_send() we return an error if `len >
TPM_PLATFORM_MAX_BUFFER - sizeof(*req)` and here, in
tpm_platform_recv(), we already return an error if `len < resp->size`.

IIUC buf/len are the same for send() and recv(), so the `resp->size >
TPM_PLATFORM_MAX_BUFFER - sizeof(*resp)` case would already be covered,
right?

Anyway this code will change a bit in v2 if we implement the send_recv() 
op for tpm_class_ops, so I'll be sure to take care of this case.

Thanks,
Stefano
diff mbox series

Patch

diff --git a/include/linux/tpm_platform.h b/include/linux/tpm_platform.h
new file mode 100644
index 000000000000..95c17a75d59d
--- /dev/null
+++ b/include/linux/tpm_platform.h
@@ -0,0 +1,90 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023 James.Bottomley@HansenPartnership.com
+ *
+ * Interface specification for platforms wishing to activate the
+ * platform tpm device.  The device must be a platform device created
+ * with the name "tpm" and it must populate platform_data with struct
+ * tpm_platform_ops
+ */
+
+/*
+ * The current MSSIM TPM commands we support.  The complete list is
+ * in the TcpTpmProtocol header:
+ *
+ * https://github.com/TrustedComputingGroup/TPM/blob/main/TPMCmd/Simulator/include/TpmTcpProtocol.h
+ */
+
+#define TPM_SEND_COMMAND		8
+#define TPM_SIGNAL_CANCEL_ON		9
+#define TPM_SIGNAL_CANCEL_OFF		10
+/*
+ * Any platform specific commands should be placed here and should start
+ * at 0x8000 to avoid clashes with the MSSIM protocol.  They should follow
+ * the same self describing buffer format below
+ */
+
+#define TPM_PLATFORM_MAX_BUFFER		4096 /* max req/resp buffer size */
+
+/**
+ * struct tpm_platform_ops - the share platform operations
+ *
+ * @sendrcv:	Send a TPM command using the MSSIM protocol.
+ *
+ * The MSSIM protocol is designed for a network, so the buffers are
+ * self describing.  The minimum buffer size is sizeof(u32).  Every
+ * MSSIM command defines its own transport buffer and the command is
+ * sent in the first u32 array.  The only modification we make is that
+ * the MSSIM uses network order and we use the endianness of the
+ * architecture.  The response to every command (in the same buffer)
+ * is a u32 size preceded array.  Most of the MSSIM commands simply
+ * return zero here because they have no defined response.
+ *
+ * The only command with a defined request/response size is TPM_SEND_COMMAND
+ * The definition is in the structures below
+ */
+struct tpm_platform_ops {
+	int (*sendrcv)(u8 *buffer);
+};
+
+/**
+ * struct tpm_send_cmd_req - Structure for a TPM_SEND_COMMAND
+ *
+ * @cmd:	The command (must be TPM_SEND_COMMAND)
+ * @locality:	The locality
+ * @inbuf_size:	The size of the input buffer following
+ * @inbuf:	A buffer of size inbuf_size
+ *
+ * Note that MSSIM expects @inbuf_size to be equal to the size of the
+ * specific TPM command, otherwise an TPM_RC_COMMAND_SIZE error is
+ * returned.
+ */
+struct tpm_send_cmd_req {
+	u32	cmd;
+	u8	locality;
+	u32	inbuf_size;
+	u8	inbuf[];
+} __packed;
+
+/**
+ * struct tpm_req - generic request header for single word command
+ *
+ * @cmd:	The command to send
+ */
+struct tpm_req {
+	u32	cmd;
+} __packed;
+
+/**
+ * struct tpm_resp - generic response header
+ *
+ * @size:	The response size (zero if nothing follows)
+ *
+ * Note: most MSSIM commands simply return zero here with no indication
+ * of success or failure.
+ */
+
+struct tpm_resp {
+	s32	size;
+} __packed;
+
diff --git a/drivers/char/tpm/tpm_platform.c b/drivers/char/tpm/tpm_platform.c
new file mode 100644
index 000000000000..b53d74344d61
--- /dev/null
+++ b/drivers/char/tpm/tpm_platform.c
@@ -0,0 +1,141 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Platform based TPM emulator
+ *
+ * Copyright (C) 2023 James.Bottomley@HansenPartnership.com
+ *
+ * Designed to handle a simple function request/response single buffer
+ * TPM or vTPM rooted in the platform.  This device driver uses the
+ * MSSIM protocol from the official TCG reference implementation
+ *
+ * https://github.com/TrustedComputingGroup/TPM
+ *
+ * to communicate between the driver and the platform.  This is rich
+ * enough to allow platform operations like cancellation The platform
+ * should not act on platform commands like power on/off and reset
+ * which can disrupt the TPM guarantees.
+ *
+ * This driver is designed to be single threaded (one call in to the
+ * platform TPM at any one time).  The threading guarantees are
+ * provided by the chip mutex.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/tpm_platform.h>
+
+#include "tpm.h"
+
+static struct tpm_platform_ops *pops;
+
+static u8 *buffer;
+/*
+ * FIXME: before implementing locality we need to agree what it means
+ * to the platform
+ */
+static u8 locality;
+
+static int tpm_platform_send(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	int ret;
+	struct tpm_send_cmd_req *req = (struct tpm_send_cmd_req *)buffer;
+
+	if (len > TPM_PLATFORM_MAX_BUFFER - sizeof(*req))
+		return -EINVAL;
+	req->cmd = TPM_SEND_COMMAND;
+	req->locality = locality;
+	req->inbuf_size = len;
+	memcpy(req->inbuf, buf, len);
+
+	ret = pops->sendrcv(buffer);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int tpm_platform_recv(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	struct tpm_resp *resp = (struct tpm_resp *)buffer;
+
+	if (resp->size < 0)
+		return resp->size;
+
+	if (len < resp->size)
+		return -E2BIG;
+
+	if (resp->size > TPM_PLATFORM_MAX_BUFFER - sizeof(*resp))
+		return -EINVAL;  // Invalid response from the platform TPM
+
+	memcpy(buf, buffer + sizeof(*resp), resp->size);
+
+	return resp->size;
+}
+
+static struct tpm_class_ops tpm_chip_ops = {
+	.flags = TPM_OPS_AUTO_STARTUP,
+	.send = tpm_platform_send,
+	.recv = tpm_platform_recv,
+};
+
+static struct platform_driver tpm_platform_driver = {
+	.driver = {
+		.name = "tpm",
+	},
+};
+
+static int __init tpm_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct tpm_chip *chip;
+	int err;
+
+	if (!dev->platform_data)
+		return -ENODEV;
+
+	/*
+	 * in theory platform matching should mean this is always
+	 * true, but just in case anyone tries force binding
+	 */
+	if (strcmp(pdev->name, tpm_platform_driver.driver.name) != 0)
+		return -ENODEV;
+
+	if (!buffer)
+		buffer = kmalloc(TPM_PLATFORM_MAX_BUFFER, GFP_KERNEL);
+
+	if (!buffer)
+		return -ENOMEM;
+
+	pops = dev->platform_data;
+
+	chip = tpmm_chip_alloc(dev, &tpm_chip_ops);
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	/*
+	 * Setting TPM_CHIP_FLAG_IRQ guarantees that ->recv will be
+	 * called straight after ->send and means we don't need to
+	 * implement any other chip ops.
+	 */
+	chip->flags |= TPM_CHIP_FLAG_IRQ;
+	err = tpm2_probe(chip);
+	if (err)
+		return err;
+
+	err = tpm_chip_register(chip);
+	if (err)
+		return err;
+
+	dev_info(dev, "TPM %s platform device\n",
+		 (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2");
+
+	return 0;
+}
+
+module_platform_driver_probe(tpm_platform_driver, tpm_platform_probe);
+
+MODULE_AUTHOR("James Bottomley <James.Bottomley@HansenPartnership.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Platform TPM Driver");
+MODULE_ALIAS("platform:tpm");
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 0fc9a510e059..b162f59305ef 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -225,5 +225,12 @@  config TCG_FTPM_TEE
 	help
 	  This driver proxies for firmware TPM running in TEE.
 
+config TCG_PLATFORM
+	tristate "Platform TPM Device"
+	help
+	  This driver requires a platform implementation to provide the
+	  TPM function.  It will not bind if the implementation is not
+	  present.
+
 source "drivers/char/tpm/st33zp24/Kconfig"
 endif # TCG_TPM
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 9bb142c75243..4b2c04e23bd3 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -44,3 +44,4 @@  obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
 obj-$(CONFIG_TCG_CRB) += tpm_crb.o
 obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
 obj-$(CONFIG_TCG_FTPM_TEE) += tpm_ftpm_tee.o
+obj-$(CONFIG_TCG_PLATFORM) += tpm_platform.o