Re: [discuss] Improve and merge a driver proposed in 2013: sysfs interfaces to access TXT config space

[Jonathan McDowell <noodles@fb.com>]

On Thu, 2022-02-17 at 13:37 +0100, Pavel Machek wrote:
> On Thu 2022-02-17 13:34:40, greg@kroah.com wrote:
> > On Thu, Feb 17, 2022 at 11:47:21AM +0000, Dmitrii Okunev wrote:
> > > Hello!
> > > 
> > > As far as I see the patch wasn't merged. And I see that this is
> > > the only unsolved thread in the discussion:
> > > 
> > > On Thu, 2013-05-16 at 18:03 +0200, Pavel Machek wrote:
> > > > On Tue 2013-05-14 01:24:43, Qiaowei Ren wrote:
> > > > > These interfaces are located in
> > > > > /sys/devices/platform/intel_txt/config,
> > > > > and including totally 37 files, providing access to Intel TXT
> > > > > configuration registers.
> > > > 
> > > > This looks like very wrong interface... equivalent of /dev/mem.
> > > 
> > > As an active user of these registers I hope it will be merged, so
> > > I would like to improve this patch (or rewrite it from scratch)
> > > to make that happen. Otherwise one have to do hackery around
> > > `/dev/mem`, which also creates problems with proper access
> > > control.
> > > 
> > > To be able to improve the patch, could somebody clarify why
> > > exactly this is a "very wrong interface"?
> > > 
> > > > > +What:          /sys/devices/platform/intel_txt/config/STS_ra
> > > > > w
> > > > > +Date:          May 2013
> > > > > +KernelVersion: 3.9
> > > > > +Contact:       "Qiaowei Ren" <qiaowei.ren@intel.com>
> > > > > +Description:   TXT.STS is the general status register. This
> > > > > read-
> > > > > only register
> > > > > +               is used by AC modules and the MLE to get the
> > > > > status
> > > > > of various
> > > > > +               Intel TXT features.
> > > > 
> > > > This is not enough to allow people to understand what this
> > > > does/should do, nor does it allow (for example) ARM people to
> > > > implement something compatible.
> > > > 
> > > > Is there specific reason why "better" interface is impossible?
> > > 
> > > I would love to reuse Intel's public documentation [1] to provide
> > > a proper description (with bit layout of the value).
> > > 
> > > [1] https://cdrdv2.intel.com/v1/dl/getContent/315168
> > > 
> > > > [...], nor does it allow (for example) ARM people to
> > > > implement something compatible.
> > > 
> > > Do I understand correctly that a proper documentation of the
> > > registers solves the problem?
> > > 
> > > > Is there specific reason why "better" interface is impossible?
> > > 
> > > What are specific problems with the current interface?
> > 
> > What do you mean by "current" here?  You are referring to an email
> > from 2013, 9 years ago.
> > 
> > If you want to propose the change again, correctly update the patch
> > and submit it that way.
> 
> I don't believe taking hardware registers and exposing them 1-to-1 in
> sysfs is the way to go.
> 
> We would like same /sys interface on different hardware, and simply
> exposing Intel's registers in /sys will not do the job.

So, for our particular use case what we want to be able to see is the
status of the TXT device, so when attestation fails it's possible to
diagnose where that might have happened. At a minimum details from the
status register are folded into the first measurement, and the error
register can provide valuable insight as to what the TXT device thinks
failed.

At present these details are retrieved from /dev/mem, but this is less
than ideal and prevents the use of, say, kernel lockdown. As a result
we'd like to export the appropriate details via sysfs. These are likely
to be extremely security block implementation specific, so I'm not
clear that a generic agnostic interface is appropriate to retrieve
these details.

Do you have the same objection to a read only set of information
(rather than the full control offered by the initial submission)?

J.