All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	linux-kernel@vger.kernel.org,
	Alan Stern <stern@rowland.harvard.edu>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: Re: [PATCH] sysfs: Allow bin_attributes to be added to groups
Date: Fri, 26 Apr 2024 20:59:51 +0200	[thread overview]
Message-ID: <Ziv5p3SMEIw3XZkK@wunner.de> (raw)
In-Reply-To: <662be72549aaf_db82d294c7@dwillia2-xfh.jf.intel.com.notmuch>

On Fri, Apr 26, 2024 at 10:40:53AM -0700, Dan Williams wrote:
> Lukas Wunner wrote:
> > Commit dfa87c824a9a ("sysfs: allow attributes to be added to groups")
> > introduced dynamic addition of sysfs attributes to groups.
> > 
> > Allow the same for bin_attributes, in support of a forthcoming commit
> > which adds various bin_attributes every time a PCI device is
> > authenticated.
> > 
> > Addition of bin_attributes to groups differs from regular attributes in
> > that different kernfs_ops are selected by sysfs_add_bin_file_mode_ns()
> > vis-à-vis sysfs_add_file_mode_ns().
> > 
> > So call either of those two functions from sysfs_add_file_to_group()
> > based on an additional boolean parameter and add two wrapper functions,
> > one for bin_attributes and another for regular attributes.
> > 
> > Removal of bin_attributes from groups does not require a differentiation
> > for bin_attributes and can use the same code path as regular attributes.
> > 
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > Cc: Alan Stern <stern@rowland.harvard.edu>
> > ---
> > Submitting this ahead of my PCI device authentication v2 patches.
> > Not sure if the patch is acceptable without an accompanying user,
> > but even if it's not, perhaps someone has early review feedback
> > or wants to provide an Acked-by?  Thank you!
> 
> On the one hand it makes sense from a symmetry perspective, on the other
> hand the expectation and the infrastructure for dynamic sysfs visibilty
> has increased since 2007.
> 
> That is why I would like to see the use case to understand why a
> dynamically added a bin_attribute is needed compared with a statically
> defined attribute with dynamic visibility.

I assume "would like to see" means "on the mailing list", which would
(or will) be as part of the PCI device authentication v2 patches.

But in case you're curious, the use case is the log of signatures
presented by the device.  Each signature is exposed as a bin_attribute
in a signatures/ directory below a PCI device's sysfs directory,
for verification by a remote attester (or user space in general).

Here's the code:

https://github.com/l1k/linux/commit/ca420b22af05

The signature's bin_attribute is accompanied by several other
bin_attributes containing ancillary data, such as nonces
(to allow user space to ascertain that a fresh nonce has been used).

The signatures/ directory is an empty attribute group to which the
signatures received from the device are dynamically added using
sysfs_add_bin_file_to_group() (introduced by the present patch).

All of that was done to address an objection raised by James Bottomley
at Plumbers that it is "not good enough" if the kernel keeps the
challenge-response received from the device to itself and doesn't
allow a remote attester to verify it.

Here is the recording, in his own words:

https://www.youtube.com/watch?v=ZqMIlZ5lPAw&t=2345s

A static attribute with dynamic visibility doesn't cut it in this case:

Each signature needs a unique filename and the number of signatures
that can be generated is essentially unlimited.  (Though older ones
are likely uninteresting and can be culled.)  If you want to expose,
say, up to 1000 signatures per device, you'd have to allocate an array
of 1000 signatures (for each device!), even though the actual number
of signatures received might be much lower.  It would be a waste of
memory.  It is much more economical to add signature attributes
dynamically on demand.

It has the additional benefit that it allows user space to dynamically
adjust the maximum number of signatures retained in the log.
That's more difficult to implement with static attributes, as you'd
have to reallocate the attributes array and adjust all the pointers
pointing to it.

Thanks,

Lukas

  reply	other threads:[~2024-04-26 18:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-25  7:44 [PATCH] sysfs: Allow bin_attributes to be added to groups Lukas Wunner
2024-04-26 17:40 ` Dan Williams
2024-04-26 18:59   ` Lukas Wunner [this message]
2024-04-27 10:47     ` Greg Kroah-Hartman
2024-04-27 16:32     ` 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=Ziv5p3SMEIw3XZkK@wunner.de \
    --to=lukas@wunner.de \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=dan.j.williams@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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.