Re: [PATCH V2 1/3] platform/x86/intel/sdsi: Add ioctl SPDM transport

Ilpo Järvinen
2024-05-08T08:23:59Z

On Tue, 7 May 2024, David E. Box wrote:

> Intel On Demand adds attestation and firmware measurement retrieval
> services through use of the protocols defined the Security Protocols and
> Data Measurement (SPDM) specification. SPDM messages exchanges are used to
> authenticate On Demand hardware and to retrieve signed measurements of the
> NVRAM state used to track feature provisioning and the NVRAM state used for
> metering services. These allow software to verify the authenticity of the
> On Demand hardware as well as the integrity of the reported silicon
> configuration.
>
> Add an ioctl interface for sending SPDM messages through the On Demand
> mailbox. Provides commands to get a list of SPDM enabled devices, get the
> message size limits for SPDM Requesters and Responders, and perform an SPDM
> message exchange.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.0.1.pdf [1]
> ---
> V2
> - Move size < 4 check into sdsi_spdm_exchange() and add comment
> clarifying return values of that function.
> - Use SZ_4K and add helpers
> - Use devm_kasprintf()
> - Remove unnecessary parens
> - Use --attest for long option
>
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> MAINTAINERS | 1 +
> drivers/platform/x86/intel/sdsi.c | 210 +++++++++++++++++-
> include/uapi/linux/intel_sdsi.h | 81 +++++++
> 4 files changed, 292 insertions(+), 1 deletion(-)
> create mode 100644 include/uapi/linux/intel_sdsi.h
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index c472423412bf..20dcc2dbcaf6 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -382,6 +382,7 @@ Code Seq# Include File Comments
> <mailto:mathieu.desnoyers@efficios.com>
> 0xF8 all arch/x86/include/uapi/asm/amd_hsmp.h AMD HSMP EPYC system management interface driver
> <mailto:nchatrad@amd.com>
> +0xFC all linux/intel_sdsi.h
> 0xFD all linux/dm-ioctl.h
> 0xFE all linux/isst_if.h
> ==== ===== ======================================================= ================================================================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 846187625552..060bd3358cec 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11165,6 +11165,7 @@ INTEL SDSI DRIVER
> M: David E. Box <david.e.box@linux.intel.com>
> S: Supported
> F: drivers/platform/x86/intel/sdsi.c
> +F: include/uapi/linux/intel_sdsi.h
> F: tools/arch/x86/intel_sdsi/
> F: tools/testing/selftests/drivers/sdsi/
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 277e4f4b20ac..686dd9e4e026 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -11,9 +11,12 @@
> #include <linux/auxiliary_bus.h>
> #include <linux/bits.h>
> #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> #include <linux/device.h>
> #include <linux/iopoll.h>
> +#include <linux/intel_sdsi.h>
> #include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> #include <linux/module.h>
> #include <linux/overflow.h>
> #include <linux/pci.h>
> @@ -42,6 +45,7 @@
>
> #define SDSI_ENABLED_FEATURES_OFFSET 16
> #define SDSI_FEATURE_SDSI BIT(3)
> +#define SDSI_FEATURE_ATTESTATION BIT(12)
> #define SDSI_FEATURE_METERING BIT(26)
>
> #define SDSI_SOCKET_ID_OFFSET 64
> @@ -91,6 +95,7 @@ enum sdsi_command {
> SDSI_CMD_PROVISION_CAP = 0x0008,
> SDSI_CMD_READ_STATE = 0x0010,
> SDSI_CMD_READ_METER = 0x0014,
> + SDSI_CMD_ATTESTATION = 0x1012,
> };
>
> struct sdsi_mbox_info {
> @@ -109,12 +114,14 @@ struct disc_table {
> struct sdsi_priv {
> struct mutex mb_lock; /* Mailbox access lock */
> struct device *dev;
> + struct miscdevice miscdev;
> void __iomem *control_addr;
> void __iomem *mbox_addr;
> void __iomem *regs_addr;
> int control_size;
> int maibox_size;
> int registers_size;
> + int id;
> u32 guid;
> u32 features;
> };
> @@ -582,6 +589,97 @@ static const struct attribute_group sdsi_group = {
> };
> __ATTRIBUTE_GROUPS(sdsi);
>
> +/*
> + * SPDM transport
> + * Returns size of the response message or an error code on failure.
> + */
> +static int sdsi_spdm_exchange(void *private, const void *request,
> + size_t request_sz, void *response,
> + size_t response_sz)
> +{
> + struct sdsi_priv *priv = private;
> + struct sdsi_mbox_info info = {};
> + size_t spdm_msg_size, size;
> + int ret;
> +
> + /*
> + * For the attestation command, the mailbox write size is the sum of:
> + * Size of the SPDM request payload, padded for qword alignment
> + * 8 bytes for the mailbox command
> + * 8 bytes for the actual (non-padded) size of the SPDM request
> + */
> + if (request_sz > SDSI_SIZE_WRITE_MSG - 2 * sizeof(u64))
> + return -EOVERFLOW;
> +
> + info.size = round_up(request_sz, sizeof(u64)) + 2 * sizeof(u64);
> +
> + u64 *payload __free(kfree) = kzalloc(info.size, GFP_KERNEL);
> + if (!payload)
> + return -ENOMEM;
> +
> + memcpy(payload, request, request_sz);
> +
> + /* The non-padded SPDM payload size is the 2nd-to-last qword */
> + payload[(info.size / sizeof(u64)) - 2] = request_sz;
> +
> + /* Attestation mailbox command is the last qword of payload buffer */
> + payload[(info.size / sizeof(u64)) - 1] = SDSI_CMD_ATTESTATION;
> +
> + info.payload = payload;
> + info.buffer = response;
> +
> + ret = mutex_lock_interruptible(&priv->mb_lock);
> + if (ret)
> + return ret;
> + ret = sdsi_mbox_write(priv, &info, &size);
> + mutex_unlock(&priv->mb_lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * The read size is the sum of:
> + * Size of the SPDM response payload, padded for qword alignment
> + * 8 bytes for the actual (non-padded) size of the SPDM payload
> + */
> +
> + if (size < sizeof(u64)) {
> + dev_err(priv->dev,
> + "Attestation error: Mailbox reply size, %ld, too small\n",
> + size);

For size_t, %zu is the correct printf format. There are more of these
below but I won't mark them explicitly.

> + return -EPROTO;
> + }
> +
> + if (!IS_ALIGNED(size, sizeof(u64))) {
> + dev_err(priv->dev,
> + "Attestation error: Mailbox reply size, %ld, is not aligned\n",
> + size);
> + return -EPROTO;
> + }
> +
> + /*
> + * Get the SPDM response size from the last QWORD and check it fits
> + * with no more than 7 bytes of padding
> + */
> + spdm_msg_size = ((u64 *)info.buffer)[(size - sizeof(u64)) / sizeof(u64)];
> + if (!in_range(size - spdm_msg_size - sizeof(u64), 0, 8)) {
> + dev_err(priv->dev,
> + "Attestation error: Invalid SPDM response size, %ld\n",
> + spdm_msg_size);
> + return -EPROTO;
> + }
> +
> + if (spdm_msg_size > response_sz || spdm_msg_size < SPDM_HEADER_SIZE) {
> + dev_err(priv->dev, "Attestation error: Expected response size %ld, got %ld\n",
> + response_sz, spdm_msg_size);
> + return -EOVERFLOW;
> + }
> +
> + memcpy(response, info.buffer, spdm_msg_size);
> +
> + return spdm_msg_size;
> +}
> +
> static int sdsi_get_layout(struct sdsi_priv *priv, struct disc_table *table)
> {
> switch (table->guid) {
> @@ -649,6 +747,92 @@ static int sdsi_map_mbox_registers(struct sdsi_priv *priv, struct pci_dev *paren
> return 0;
> }
>
> +#define SDSI_SPDM_DRIVER_VERSION 1
> +
> +static int sdsi_spdm_get_info(struct sdsi_priv *priv,
> + struct sdsi_spdm_info __user *argp)
> +{
> + struct sdsi_spdm_info info;
> +
> + info.driver_version = SDSI_SPDM_DRIVER_VERSION;
> + info.api_version = priv->guid;
> + info.dev_no = priv->id;
> + info.max_request_size = SDSI_SIZE_WRITE_MSG - 2 * sizeof(u64);
> + info.max_response_size = SDSI_SIZE_READ_MSG - sizeof(u64);
> +
> + if (copy_to_user(argp, &info, sizeof(info)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static int sdsi_spdm_do_command(struct sdsi_priv *priv,
> + struct sdsi_spdm_command __user *argp)
> +{
> + u32 req_size, rsp_size;
> +
> + if (get_user(req_size, &argp->size))
> + return -EFAULT;
> +
> + if (req_size < 4 || req_size > sizeof(struct sdsi_spdm_message))
> + return -EINVAL;
> +
> + struct sdsi_spdm_message *request __free(kfree) =
> + kmalloc(req_size, GFP_KERNEL);
> + if (!request)
> + return -ENOMEM;
> +
> + struct sdsi_spdm_command *response __free(kfree) =
> + kmalloc(SDSI_SIZE_READ_MSG, GFP_KERNEL);
> + if (!response)
> + return -ENOMEM;
> +
> + if (copy_from_user(request, &argp->message, req_size))
> + return -EFAULT;
> +
> + rsp_size = sdsi_spdm_exchange(priv, request, req_size, response,
> + SDSI_SIZE_READ_MSG);
> + if (rsp_size < 0)
> + return rsp_size;
> +
> + if (put_user(rsp_size, &argp->size))
> + return -EFAULT;
> +
> + if (copy_to_user(&argp->message, response, rsp_size))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static long sdsi_spdm_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct sdsi_priv *priv;
> + long ret = -ENOTTY;
> +
> + priv = container_of(file->private_data, struct sdsi_priv, miscdev);
> +
> + switch (cmd) {
> + case SDSI_IF_SPDM_INFO:
> + ret = sdsi_spdm_get_info(priv,
> + (struct sdsi_spdm_info __user *)arg);
> + break;
> + case SDSI_IF_SPDM_COMMAND:
> + ret = sdsi_spdm_do_command(priv,
> + (struct sdsi_spdm_command __user *)arg);

You can return directly.

> + break;
> + default:
> + break;
> + }
> +
> + return ret;

return -ENOTTY;

and remove the ret variable entirely.

--
i.