linux-kernel - Re: [PATCH] platform/x86/intel/ifs: Allow non-default names for IFS image

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c416b93b-8adc-d9a8-ef0d-594cce6d9a9d@redhat.com>
Date:   Sun, 10 Jul 2022 16:08:39 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     "Joseph, Jithu" <jithu.joseph@...el.com>, markgross@...nel.org,
        ashok.raj@...el.com, tony.luck@...el.com, ravi.v.shankar@...el.com,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        patches@...ts.linux.dev
Subject: Re: [PATCH] platform/x86/intel/ifs: Allow non-default names for IFS
 image

Hi Greg,

On 7/10/22 15:53, Greg KH wrote:
> On Sun, Jul 10, 2022 at 03:42:29PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 7/10/22 12:15, Greg KH wrote:
>>> On Fri, Jul 08, 2022 at 11:34:40AM -0700, Joseph, Jithu wrote:
>>>>
>>>>
>>>> On 7/8/2022 8:28 AM, Greg KH wrote:
>>>>> On Fri, Jul 08, 2022 at 08:19:38AM -0700, Jithu Joseph wrote:
>>>>>> Existing implementation limits IFS image to be loaded only from
>>>>>> a default file-name (ff-mm-ss.scan).
>>>>>>
>>>>
>>>>>
>>>>> Ick, but now what namespace are you saying that path is in?  If you need
>>>>> debugging stuff, then put the api/interface in debugfs and use it there,
>>>>> don't overload the existing sysfs api to do something different here.
>>>>
>>>> The namespace related confusion could be because, the original commit message
>>>> was not using full path-names. The below write-up tries to be more clear on this
>>>>
>>>> Existing implementation limits IFS images to be loaded only from
>>>> a default file-name /lib/firmware/intel/ifs/ff-mm-ss.scan.
>>>>
>>>> But there are situations where there may be multiple scan files
>>>> that can be run on a particular system stored in /lib/firmware/intel/ifs
>>>>
>>>> E.g.
>>>> 1. Because test contents are larger than the memory reserved for IFS by BIOS
>>>> 2. To provide increased test coverage
>>>> 3. Custom test files to debug certain specific issues in the field
>>>>
>>>> Renaming each of these to ff-mm-ss.scan and then loading might be
>>>> possible in some environments. But on systems where /lib is read-only
>>>> this is not a practical solution.
>>>>
>>>> Extend the semantics of the driver /sys/devices/virtual/misc/intel_ifs_0/reload
>>>> file:
>>>>
>>>>   Writing "1" remains the legacy behavior to load from the default
>>>>   ff-mm-ss.scan file.
>>>>
>>>>   Writing some other string is interpreted as a filename in
>>>>   /lib/firmware/intel/ifs to be loaded instead of the default file.
>>>
>>> Ick, you are overloading an existing sysfs file to do different things
>>> based on random stuff.  This is a brand-new api that you are already
>>> messing with in crazy ways.  Why not just revert the whole thing and
>>> start over as obviously this was not tested well with real devices.
>>>
>>> And what is wrong with a firmware file called '1'?  :)
>>
>> Actually the Intel IFS stuff has landed in 5.19-rc# so it is
>> a bit late(ish) for dropping it now.
> 
> We can mark it BROKEN right now before -final happens as it seems that
> the api in 5.19-rc is not correct for its users.
> 
> Perhaps we should do that now to give people the chance to get it right?

That is a good idea. I've just send out a patch doing that.

I plan to submit one last pdx86 fixes pull-req to Linus once rc6 is out
(prepping it now and want to give the builders some time to build test it).

I'll include this in this fixes pull-req.

Regards,

Hans