All of
 help / color / mirror / Atom feed
From: Russ Weight <>
To: Greg KH <>, Moritz Fischer <>
Cc: Tom Rix <>, <>,
Subject: Re: [PATCH 02/12] fpga: sec-mgr: enable secure updates
Date: Tue, 2 Nov 2021 09:25:10 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 8/4/21 8:12 AM, Greg KH wrote:
> On Wed, Aug 04, 2021 at 07:58:34AM -0700, Moritz Fischer wrote:
>> On Wed, Aug 04, 2021 at 09:37:45AM +0200, Greg KH wrote:
>>> On Tue, Aug 03, 2021 at 12:02:24PM -0700, Russ Weight wrote:
>>>> On 8/2/21 10:49 PM, Greg KH wrote:
>>>>>> If the request_firmware() implementation is not acceptable, then would
>>>>>> you agree that an IOCTL implementation is our best option?
>>>>> There is no difference in the end between using an ioctl, or a sysfs
>>>>> file, to provide the filename of your firmware, don't get hung up on
>>>>> that.
>>>> I meant to suggest that passing file data (not a filename) through an
>>>> IOCTL might be better for this use case than trying to use request_firmware.
>>>> We have to, somehow, allow the user to point us to the desired image
>>>> data (which could be a root-entry-hash, or an FPGA image). We can't
>>>> really use a fixed filename modified by device version as many of
>>>> the devices do.
>>> Ah, yes, a "normal" write command might be best for this as that can be
>>> properly containerized and controlled.
>>>>> By providing a "filename", you are going around all of the namespace and
>>>>> other "container" protection that the kernel provides, and allowing
>>>>> processes to potentially load files that are normally outside of their
>>>>> scope to the hardware.  If you are willing to allow that security
>>>>> "escape", wonderful, but you better document the heck out of it and
>>>>> explain why this is allowed for your special hardware and use case.
>>>>> As you are expecting this to work "in the cloud", I do not think that
>>>>> the operators of such hardware are really going to be all that happy to
>>>>> see this type of interface given these reasons.
>>>>> What is wrong with the current fpga firmware api that somehow is lacking
>>>>> for your special hardware, that other devices do not have to worry
>>>>> about?
>>>> The existing framework wants to update the live image in the FPGA,
>>>> whereas for this device, we are passing signed data to BMC firmware
>>>> which will store it in FLASH to be loaded on a subsequent boot of
>>>> the card.
>>>> The existing framework needs to manage FPGA state, whereas for this
>>>> device, it is just a transfer of signed data. We also have to handle
>>>> a total transfer/authentication time of up to 45 minutes, so we are
>>>> using a kernel worker thread for the update.
>>>> Perhaps the name, fpga security manager, is wrong? Maybe something
>>>> like fpga_sec_image_xfer is better?
>>> It does not sound like this has anything to do with "security", and
>>> rather is just a normal firmware upload, so "fpga_image_upload()"
>>> perhaps?
>> I had originally suggested 'load' and 'persist' or 'load' and 'update or
>> something of that sort.
>> Taking one step back, maybe the case could be made for a generic
>> 'persistent firmware' update framework that addresses use-cases that
>> require updating firmware that may take extended periods of time.
> There should not be a problem with using the existing firmware layer for
> images that take long periods of time, as long as you are not wanting to
> see any potential progress :)
> So how about just adding anything missing to the existing firmware
> subsystem.  It's attempting to handle all use cases already, if it is
> missing one, no harm in adding more options there...
Hi Greg,

We have had a lot of internal (to Intel) discussion about how to
organize the support for uploading FPGA images. It would be helpful
to know which of the following two options you find the least
disturbing :-)

Background: We are uploading signed, self-describing images that are
authenticated and dispositioned by the Card BMC. These could result
in FLASH updates for FPGA images, BMC images, firmware, or security
keys.  They could also result in a temporary authentication
certificate being loaded into RAM as part of a multi-step key
provisioning process.

(a) A single API that facilitates the upload of a data stream
without analyzing the stream contents, relying on the lower-level
driver and/or HW to accept or reject the data.

(b) Multiple, targeted APIs (e.g. IOCTL_FPGA_IMAGE_UPDATE,
each interpret the stream type and reject them if they don't
correspond to the API target.

Do you have a preference between (a) and (b)?

- Russ
> thanks,
> greg k-h

  parent reply	other threads:[~2021-11-02 16:44 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-17  2:31 [PATCH 00/12] FPGA Security Manager for 5.14 Moritz Fischer
2021-05-17  2:31 ` [PATCH 01/12] fpga: sec-mgr: fpga security manager class driver Moritz Fischer
2021-05-17  5:18   ` Greg KH
2021-05-17 17:45     ` Russ Weight
2021-05-17 17:55       ` Greg KH
2021-05-17 18:25         ` Russ Weight
2021-05-19 20:42           ` Tom Rix
2021-05-21  1:10             ` Russ Weight
2021-05-21  4:58               ` Greg KH
2021-05-21 15:15                 ` Russ Weight
2021-05-17  2:31 ` [PATCH 02/12] fpga: sec-mgr: enable secure updates Moritz Fischer
2021-05-17  5:32   ` Greg KH
2021-05-17 19:37     ` Russ Weight
2021-07-30  1:23       ` Russ Weight
2021-07-30 11:18         ` Greg KH
2021-08-02 18:31           ` Russ Weight
2021-08-03  5:49             ` Greg KH
2021-08-03 19:02               ` Russ Weight
2021-08-04  7:37                 ` Greg KH
2021-08-04 14:58                   ` Moritz Fischer
2021-08-04 15:12                     ` Greg KH
2021-08-04 19:47                       ` Moritz Fischer
2021-11-02 16:25                       ` Russ Weight [this message]
2021-11-02 17:06                         ` Greg KH
2021-05-17  2:31 ` [PATCH 03/12] fpga: sec-mgr: expose sec-mgr update status Moritz Fischer
2021-05-17  2:31 ` [PATCH 04/12] fpga: sec-mgr: expose sec-mgr update errors Moritz Fischer
2021-05-17  2:31 ` [PATCH 05/12] fpga: sec-mgr: expose sec-mgr update size Moritz Fischer
2021-05-17  2:31 ` [PATCH 06/12] fpga: sec-mgr: enable cancel of secure update Moritz Fischer
2021-05-17  2:31 ` [PATCH 07/12] fpga: sec-mgr: expose hardware error info Moritz Fischer
2021-05-17  7:10   ` Greg KH
2021-05-17 19:49     ` Russ Weight
2021-05-17  2:31 ` [PATCH 08/12] fpga: m10bmc-sec: create max10 bmc secure update driver Moritz Fischer
2021-05-17  5:30   ` Greg KH
2021-05-17 20:09     ` Russ Weight
2021-05-17  2:31 ` [PATCH 09/12] fpga: m10bmc-sec: expose max10 flash update count Moritz Fischer
2021-05-17  2:31 ` [PATCH 10/12] fpga: m10bmc-sec: expose max10 canceled keys in sysfs Moritz Fischer
2021-05-17  2:31 ` [PATCH 11/12] fpga: m10bmc-sec: add max10 secure update functions Moritz Fischer
2021-05-17  2:32 ` [PATCH 12/12] fpga: m10bmc-sec: add max10 get_hw_errinfo callback func Moritz Fischer

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \

* 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.