[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-fpga
Subject:    Re: [PATCH 02/12] fpga: sec-mgr: enable secure updates
From:       Greg KH <gregkh () linuxfoundation ! org>
Date:       2021-07-30 11:18:46
Message-ID: YQPgFn/z024U06HJ () kroah ! com
[Download RAW message or body]

On Thu, Jul 29, 2021 at 06:23:12PM -0700, Russ Weight wrote:
> 
> 
> On 5/17/21 12:37 PM, Russ Weight wrote:
> > On 5/16/21 10:32 PM, Greg KH wrote:
> >> On Sun, May 16, 2021 at 07:31:50PM -0700, Moritz Fischer wrote:
> >>> From: Russ Weight <russell.h.weight@intel.com>
> >>>
> >>> Extend the FPGA Security Manager class driver to
> >>> include an update/filename sysfs node that can be used
> >>> to initiate a secure update.  The filename of a secure
> >>> update file (BMC image, FPGA image, Root Entry Hash image,
> >>> or Code Signing Key cancellation image) can be written to
> >>> this sysfs entry to cause a secure update to occur.
> >> Why is userspace responsible for triggering this?  Passing a "filename"
> >> into the kernel and having it do something with it is ripe for major
> >> problems, please do not.
> >>
> > I am using the "request_firmware" framework, which accepts a filename
> > and finds the firmware file under /lib/firmware.
> >
> > Is this not an acceptable use for request_firmware?
> >
> > - Russ
> 
> Hi Greg,
> 
> The dev_release fixes that you asked for in the FPGA Manager, Bridge, and
> Region code are almost complete. I'm trying to get back to the FPGA
> security manager patch set. Your previous comments challenged some basic
> assumptions. If it is OK, I would like to get some clarity before I rework
> the patches.

Note, I do not have the time, nor the inclination, to help your company
out with design reviews at this point in time.  If you have questions
about this, please discuss it with the open source managers at Intel,
they know the current situation quite well.

I am glad to review patches that have gone through the proper internal
Intel patch review process and then sent out to the community.

That being said, I will make one comment on your questions below:

> (1) request_firmware(). We had assumed that making use of the existing
> request_firmware() would be preferred. This requires providing a filename
> under /lib/firmware to the framework. You commented (above): "Passing a
> 'filename' into the kernel and having it do something with it is ripe for
> problems, please do not." Unless you have additional comments on this, I
> will plan to NOT use the request_firmware framework.

request_firmware() should always be used for requesting firmware for a
device.  Having an api where you write a random filename to a sysfs file
and have that loaded by the kernel seems ripe for disaster though, I can
not think of any other in-kernel user of the firmware api that does
this.  Or are there examples that I have just missed?

thanks,

greg k-h
[prev in list] [next in list] [prev in thread] [next in thread]