From: Dan Williams <dan.j.williams@intel.com>
To: Dionna Amalie Glaze <dionnaglaze@google.com>,
Dan Williams <dan.j.williams@intel.com>
Cc: <linux-coco@lists.linux.dev>,
Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com>,
James Bottomley <James.Bottomley@hansenpartnership.com>,
Peter Gonda <pgonda@google.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Samuel Ortiz <sameo@rivosinc.com>, <peterz@infradead.org>,
<linux-kernel@vger.kernel.org>, <tglx@linutronix.de>
Subject: Re: [PATCH v3 2/5] configfs-tsm: Introduce a shared ABI for attestation reports
Date: Thu, 31 Aug 2023 15:13:21 -0700 [thread overview]
Message-ID: <64f11081985e_31c2db29440@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <CAAH4kHYNhiewBZ8Z_yF2F+ogkv_1UV8=Gu0KVDdTJN4iQKyNMg@mail.gmail.com>
Dionna Amalie Glaze wrote:
> This is clean and approachable. Thanks for your implementation.
>
> > +static int try_advance_write_generation(struct tsm_report *report)
> > +{
> > + lockdep_assert_held_write(&tsm_rwsem);
> > +
> > + /*
> > + * malicious or broken userspace is attempting to wrap read_generation,
> > + * stop accepting updates until current report configuration is read.
> > + */
> > + if (report->write_generation == report->read_generation - 1)
> > + return -EBUSY;
> > + report->write_generation++;
> > + return 0;
> > +}
> > +
>
> This took me a while to wrap my head around.
> The property we want is that when we read outblob, it is the result of
> the attribute changes since the last read. If we write to an attribute
> 2^64 times, we could get write_generation to wrap around to equal
> read_generation without actually reading outblob. So we could end up
> given a badly cached result when we read outblob? Is that what this is
> preventing?
Correct. The criticism of kernfs based interfaces like sysfs and
configfs is that there is no facility to atomically modify a set of
attributes at once. The expectated mitigation is simply that userspace
is well behaved. For example, 2 invocations of fdisk can confuse each
other, so userspace is expected to run them serially and the kernel
otherwise lets userspace do silly things.
If a tool has any concern that it has exclusive ownership of the
interface this 'generation' attribute allows for a flow like:
gen=$(cat $report/generation)
dd if=userdata > $report/inblob
cat $report/outblob > report
gen2=$(cat $report/generation)
...and if $gen2 is not $((gen + 1)) then tooling can detect that the
"userspace is well behaved" expectation was violated.
Now configs is slightly better in this regard because objects can be
atomically created. So if 2 threads happen to pick the same name for the
report object then 'mkdir' will only succeed for one while the other
gets an EEXIST error. So for containers each can get their own
submission interface without worrying about other containers.
> I think I would word this to say,
>
> "Malicious or broken userspace has written enough times for
> read_generation == write_generation by modular arithmetic without an
> interim read. Stop accepting updates until the current report
> configuration is read."
That works for me.
> I think that at least in the SEV-SNP case, we can double-check from
> userspace that the report has values that we expected to configure the
> get_report with, so this isn't really an issue. I'm not sure what the
> use is of configuration that doesn't lead to observable (and
> checkable) differences, but I suppose this check doesn't hurt.
Right, the tool can also just double check that all the
parameters are the expected value in the output report, but if you want
to trust the kernel output without necessarily trusting other tools to
not stomp on your config item instance then 'generation' helps.
next prev parent reply other threads:[~2023-08-31 22:13 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-30 19:33 [PATCH v3 0/5] configfs-tsm: Attestation Report ABI Dan Williams
2023-08-30 19:33 ` [PATCH v3 1/5] virt: coco: Add a coco/Makefile and coco/Kconfig Dan Williams
2023-08-30 20:48 ` Kuppuswamy Sathyanarayanan
2023-08-30 19:33 ` [PATCH v3 2/5] configfs-tsm: Introduce a shared ABI for attestation reports Dan Williams
2023-08-30 20:45 ` Greg Kroah-Hartman
2023-08-30 20:57 ` Dan Williams
2023-08-30 22:08 ` Kuppuswamy Sathyanarayanan
2023-08-31 1:24 ` Dan Williams
2023-08-31 21:42 ` Dionna Amalie Glaze
2023-08-31 22:13 ` Dan Williams [this message]
2023-09-01 18:06 ` Thomas Gleixner
2023-09-01 18:47 ` Dan Williams
2023-09-01 19:06 ` Thomas Gleixner
2023-08-30 19:33 ` [PATCH v3 3/5] virt: sevguest: Prep for kernel internal {get, get_ext}_report() Dan Williams
2023-08-30 19:33 ` [PATCH v3 4/5] mm/slab: Add __free() support for kvfree Dan Williams
2023-08-30 20:46 ` Greg Kroah-Hartman
2023-09-07 8:59 ` Gupta, Pankaj
2023-08-30 19:33 ` [PATCH v3 5/5] virt: sevguest: Add TSM_REPORTS support for SNP_{GET, GET_EXT}_REPORT Dan Williams
2023-09-01 15:25 ` Jeremi Piotrowski
2023-09-01 16:38 ` Dan Williams
2023-09-04 2:14 ` Huang, Kai
2023-09-04 2:57 ` Kuppuswamy Sathyanarayanan
2023-09-05 23:21 ` Huang, Kai
2023-09-01 16:04 ` [PATCH v3 0/5] configfs-tsm: Attestation Report ABI Jeremi Piotrowski
2023-09-01 16:51 ` Dan Williams
2023-09-07 8:04 ` Samuel Ortiz
2023-09-25 19:26 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=64f11081985e_31c2db29440@dwillia2-xfh.jf.intel.com.notmuch \
--to=dan.j.williams@intel.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=dionnaglaze@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-coco@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=pgonda@google.com \
--cc=sameo@rivosinc.com \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.