From: Thomas Gleixner <tglx@linutronix.de>
To: Dan Williams <dan.j.williams@intel.com>,
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
Subject: Re: [PATCH v3 2/5] configfs-tsm: Introduce a shared ABI for attestation reports
Date: Fri, 01 Sep 2023 20:06:31 +0200 [thread overview]
Message-ID: <87cyz14vko.ffs@tglx> (raw)
In-Reply-To: <64f11081985e_31c2db29440@dwillia2-xfh.jf.intel.com.notmuch>
On Thu, Aug 31 2023 at 15:13, Dan Williams wrote:
> 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.
That's a pretty theoretical problem. Under the assumption that one
syscall takes 50ns the wraparound on a 64bit variable will take ~29247
years to complete.
I think the important point is that the generation check there ensures
that the expected sequence takes place and can't be overwritten by
accident or malice, no?
Thanks,
tglx
next prev parent reply other threads:[~2023-09-01 18:06 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
2023-09-01 18:06 ` Thomas Gleixner [this message]
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=87cyz14vko.ffs@tglx \
--to=tglx@linutronix.de \
--cc=James.Bottomley@hansenpartnership.com \
--cc=dan.j.williams@intel.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 \
/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.