lkml.org 
[lkml]   [2023]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 2/5] tsm: Introduce a shared ABI for attestation reports
    From
    Date
    On Mon, 2023-08-14 at 00:43 -0700, Dan Williams wrote:
    > One of the common operations of a TSM (Trusted Security Module) is to
    > provide a way for a TVM (confidential computing guest execution
    > environment) to take a measurement of its launch state, sign it and
    > submit it to a verifying party. Upon successful attestation that
    > verifies the integrity of the TVM additional secrets may be deployed.
    > The concept is common across TSMs, but the implementations are
    > unfortunately vendor specific. While the industry grapples with a common
    > definition of this attestation format [1], Linux need not make this
    > problem worse by defining a new ABI per TSM that wants to perform a
    > similar operation. The current momentum has been to invent new ioctl-ABI
    > per TSM per function which at best is an abdication of the kernel's
    > responsibility to make common infrastructure concepts share common ABI.
    >
    > The proposal, targeted to conceptually work with TDX, SEV, COVE if not
    > more, is to define a sysfs interface to retrieve the TSM-specific blob.
    >
    >     echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
    >     hexdump /sys/class/tsm/tsm0/outblob
    >
    > This approach later allows for the standardization of the attestation
    > blob format without needing to change the Linux ABI. Until then, the
    > format of 'outblob' is determined by the parent device for 'tsm0'.
    >
    > The expectation is that this is a boot time exchange that need not be
    > regenerated, making it amenable to a sysfs interface. In case userspace
    > does try to generate multiple attestation reports it includes conflict
    > detection so userspace can be sure no other thread changed the
    > parameters from its last configuration step to the blob retrieval.
    >
    > TSM specific options are encoded as 'extra' attributes on the TSM device
    > with the expectation that vendors reuse the same options for similar
    > concepts. The current options are defined by SEV-SNP's need for a
    > 'privilege level' concept (VMPL), and the option to retrieve a
    > certificate chain in addition to the attestation report ("extended"
    > format).
    >
    > Link:
    > http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch
    >  [1]
    > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
    > Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
    > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
    > Cc: Peter Gonda <pgonda@google.com>
    > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    > Cc: Samuel Ortiz <sameo@rivosinc.com>
    > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
    > ---
    >  Documentation/ABI/testing/sysfs-class-tsm |   47 +++++
    >  MAINTAINERS                               |    8 +
    >  drivers/virt/coco/Kconfig                 |    4
    >  drivers/virt/coco/Makefile                |    1
    >  drivers/virt/coco/tdx-guest/Kconfig       |    1
    >  drivers/virt/coco/tsm.c                   |  290
    > +++++++++++++++++++++++++++++
    >  include/linux/tsm.h                       |   45 +++++
    >  7 files changed, 396 insertions(+)
    >  create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
    >  create mode 100644 drivers/virt/coco/tsm.c
    >  create mode 100644 include/linux/tsm.h


    > +static ssize_t outblob_read(struct file *f, struct kobject *kobj,
    > +                           struct bin_attribute *bin_attr, char *buf,
    > +                           loff_t offset, size_t count)
    > +{
    > +       guard(rwsem_read)(&tsm_rwsem);
    > +       if (!tsm_report.desc.inblob_len)
    > +               return -EINVAL;
    > +
    > +       if (!tsm_report.outblob ||
    > +           tsm_report.read_generation != tsm_report.write_generation) {
    > +               const struct tsm_ops *ops = provider.ops;
    > +               size_t outblob_len;
    > +               u8 *outblob;
    > +
    > +               kvfree(tsm_report.outblob);

    I think tsm_report.outblob needs to be set to NULL here otherwise there is
    the possibility of a double-free if ops->report_new returns an error.

    Roy

    > +               outblob = ops->report_new(provider.dev->parent,
    > +                                         &tsm_report.desc, &outblob_len);
    > +               if (IS_ERR(outblob))
    > +                       return PTR_ERR(outblob);
    > +               tsm_report.outblob_len = outblob_len;
    > +               tsm_report.outblob = outblob;
    > +               tsm_report.read_generation = tsm_report.write_generation;
    > +       }
    > +
    > +       return memory_read_from_buffer(buf, count, &offset,
    > +                                      tsm_report.outblob,
    > +                                      tsm_report.outblob_len);
    > +}

    \
     
     \ /
      Last update: 2023-08-22 09:30    [W:7.240 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site