| 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
| ||
|
Message-ID: <CAGXu5jJXMxx2Xna_M_g1wsS59XnYKqHAGdxhzDtk3PhnLL-gQQ@mail.gmail.com>
Date: Tue, 8 May 2018 15:42:33 -0700
From: Kees Cook <keescook@...omium.org>
To: "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc: Greg KH <gregkh@...uxfoundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Josh Triplett <josh@...htriplett.org>, maco@...roid.com,
Andy Gross <andy.gross@...aro.org>,
David Brown <david.brown@...aro.org>,
bjorn.andersson@...aro.org, teg@...m.no, wagi@...om.org,
Hans de Goede <hdegoede@...hat.com>,
Andres Rodriguez <andresx7@...il.com>,
Mimi Zohar <zohar@...ux.vnet.ibm.com>,
Jakub Kicinski <kubakici@...pl>, Shuah Khan <shuah@...nel.org>,
Martin Fuzzey <mfuzzey@...keon.com>,
David Howells <dhowells@...hat.com>, pali.rohar@...il.com,
Takashi Iwai <tiwai@...e.de>,
Kalle Valo <kvalo@...eaurora.org>,
Arend van Spriel <arend.vanspriel@...adcom.com>,
Rafał Miłecki <zajec5@...il.com>,
Nicolas Broeking <nbroeking@...com>,
Vikram Mulukutla <markivx@...eaurora.org>,
Mark Brown <broonie@...nel.org>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
David Woodhouse <dwmw2@...radead.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Abhay_Salunke@...l.com, jewalt@...innovations.com,
oneukum@...e.com, Bitterblue of Monsea <cantabile.desu@...il.com>,
Alexei Starovoitov <ast@...com>, hare <hare@...e.com>,
"James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>, khc@...waw.pl,
"David S. Miller" <davem@...emloft.net>,
Arve Hjønnevåg <arve@...roid.com>,
tkjos@...roid.com, Jonathan Corbet <corbet@....net>,
mchehab+samsung@...nel.org, LKML <linux-kernel@...r.kernel.org>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
linux-scsi@...r.kernel.org,
linux-wireless <linux-wireless@...r.kernel.org>,
Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation
over FW_LOADER
On Tue, May 8, 2018 at 11:12 AM, Luis R. Rodriguez <mcgrof@...nel.org> wrote:
> If you try to read FW_LOADER today it speaks of old riddles and
> unless you have been following development closely you will loose
Typo: lose
> track of what is what. Even the documentation for PREVENT_FIRMWARE_BUILD
> is a bit fuzzy and how it fits into this big picture.
>
> Give the FW_LOADER kconfig documentation some love with more up to
> date developments and recommendations. While at it, wrap the FW_LOADER
> code into its own menu to compartamentalize and make it clearer which
Typo: compartmentalize
> components really are part of the FW_LOADER. This should also make
> it easier to later move these kconfig entries into the firmware_loader/
> directory later.
>
> This also now recommends using firmwared [0] for folks left needing a uevent
> handler in userspace for the sysfs firmware fallback mechanis given udev's
> uevent firmware mechanism was ripped out a while ago.
>
> [0] https://github.com/teg/firmwared
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org>
> ---
> drivers/base/Kconfig | 165 ++++++++++++++++++++++++++++++++++---------
> 1 file changed, 131 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 29b0eb452b3a..a4fe86caecca 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -70,39 +70,64 @@ config STANDALONE
> If unsure, say Y.
>
> config PREVENT_FIRMWARE_BUILD
> - bool "Prevent firmware from being built"
> + bool "Disable drivers features which enable custom firmware building"
> default y
> help
> - Say yes to avoid building firmware. Firmware is usually shipped
> - with the driver and only when updating the firmware should a
> - rebuild be made.
> - If unsure, say Y here.
> + Say yes to disable driver features which enable building a custom
> + driver firmwar at kernel build time. These drivers do not use the
Typo: firmware
> + kernel firmware API to load firmware (CONFIG_FW_LOADER), instead they
> + use their own custom loading mechanism. The required firmware is
> + usually shipped with the driver, building the driver firmware
> + should only be needed if you have an updated firmware source.
> +
> + Firmware should not be being built as part of kernel, these days
> + you should always prevent this and say Y here. There are only two
> + old drivers which enable building of its firmware at kernel build
> + time:
> +
> + o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
> + o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
> +
> +menu "Firmware loader"
>
> config FW_LOADER
> - tristate "Userspace firmware loading support" if EXPERT
> + tristate "Firmware loading facility" if EXPERT
> default y
> ---help---
> - This option is provided for the case where none of the in-tree modules
> - require userspace firmware loading support, but a module built
> - out-of-tree does.
> + This enables the firmware loading facility in the kernel. The kernel
> + will first look for built-in firmware, if it has any. Next, it will
> + look for the requested firmware in a series of filesystem paths:
> +
> + o firmware_class path module parameter or kernel boot param
> + o /lib/firmware/updates/UTS_RELEASE
> + o /lib/firmware/updates
> + o /lib/firmware/UTS_RELEASE
> + o /lib/firmware
> +
> + Enabling this feature only increases your kernel image by about
> + 828 bytes, enable this option unless you are certain you don't
> + need firmware.
> +
> + You typically want this built-in (=y) but you can also enable this
> + as a module, in which case the firmware_class module will be built.
> + You also want to be sure to enable this built-in if you are going to
> + enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
> +
> +if FW_LOADER
>
> config EXTRA_FIRMWARE
> - string "External firmware blobs to build into the kernel binary"
> - depends on FW_LOADER
> + string "Build these firmware blobs into the kernel binary"
Maybe "Build named firmware blobs ..." "these" took me a while to figure out.
> help
> - Various drivers in the kernel source tree may require firmware,
> - which is generally available in your distribution's linux-firmware
> - package.
> + Device drivers which require firmware can typically deal with
> + having the kernel load firmware from the various supported
> + /lib/firmware/ paths. This option enables you to build into the
> + kernel firmware files. Built-in firmware searches are preceeded
Typo: preceded
> + over firmware lookups using your filesystem over the supported
> + /lib/firmware paths documented on CONFIG_FW_LOADER.
>
> - The linux-firmware package should install firmware into
> - /lib/firmware/ on your system, so they can be loaded by userspace
> - helpers on request.
> -
> - This option allows firmware to be built into the kernel for the case
> - where the user either cannot or doesn't want to provide it from
> - userspace at runtime (for example, when the firmware in question is
> - required for accessing the boot device, and the user doesn't want to
> - use an initrd).
> + This may be useful for testing or if the firmware is required early on
> + in boot and cannot rely on the firmware being placed in an initrd or
> + initramfs.
>
> This option is a string and takes the (space-separated) names of the
> firmware files -- the same names that appear in MODULE_FIRMWARE()
> @@ -113,7 +138,7 @@ config EXTRA_FIRMWARE
> For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
> the usb8388.bin file into /lib/firmware, and build the kernel. Then
> any request_firmware("usb8388.bin") will be satisfied internally
> - without needing to call out to userspace.
> + inside the kernel without ever looking at your filesystem at runtime.
>
> WARNING: If you include additional firmware files into your binary
> kernel image that are not available under the terms of the GPL,
> @@ -130,22 +155,94 @@ config EXTRA_FIRMWARE_DIR
> looks for the firmware files listed in the EXTRA_FIRMWARE option.
>
> config FW_LOADER_USER_HELPER
> - bool
> + bool "Enable the firmware sysfs fallback mechanism"
> + help
> + This option enables a sysfs loading facility to enable firmware
> + loading to the kernel through userspace as a fallback mechanism
> + if and only if the kernel's direct filesystem lookup for the
> + firmware failed using the different /lib/firmware/ paths, or the
> + path specified in the firmware_class path module parameter, or the
> + firmware_class path kernel boot parameter if the firmware_class is
> + built-in. For details on how to work with the sysfs fallback mechanism
> + refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
> +
> + The direct filesystem lookup for firwmare is always used first now.
Typo: firmware
> +
> + If the kernel's direct filesystem lookup for firware fails to find
> + the requested firmware a sysfs fallback loading facility is made
> + available and userspace is informed about this through uevents.
> + The uevent can be supressed if the driver explicitly requested it,
> + this is known as the driver using the custom fallback mechanism.
> + If the custom fallback mechanism is used userspace must always
> + acknowledge failure to find firmware as the timeout for the fallback
> + mechanism is disabled, and failed requests will linger forever.
> +
> + This used to be the default firmware loading facility, and udev used
> + to listen for uvents to load firmware for the kernel. The firmware
> + loading facility functionality in udev has been removed, as such it
> + can no longer be relied upon as a fallback mechanism. Linux no longer
> + relies on or uses a fallback mechanism in userspace. If you need to
> + rely on one refer to the permissively licensed firmwared:
Typo: firmware
> +
> + https://github.com/teg/firmwared
> +
> + Since this was the default firmware loading facility at one point,
> + old userspace may exist which relies upon it, and as such this
> + mechanism can never be removed from the kernel.
> +
> + You should only enable this functionality if you are certain you
> + require a fallback mechanism and have a userspace mechanism ready to
> + load firmware in case it is not found. One main reason for this may
> + be if you have drivers which require firmware built-in and for
> + whatever reason cannot place the required firmware in initramfs.
> + Another reason kernels may have this feature enabled is to support a
> + driver which explicitly relies on this fallback mechanism. Only two
> + drivers need this today:
> +
> + o CONFIG_LEDS_LP55XX_COMMON
> + o CONFIG_DELL_RBU
> +
> + Outside of supporting the above drivers, another reason for needing
> + this may be that your firmware resides outside of the paths the kernel
> + looks for and cannot possibily be specified using the firmware_class
> + path module parameter or kernel firmware_class path boot parameter
> + if firmware_class is built-in.
> +
> + A modern use case may be to temporarily mount a custom partition
> + during provisioning which is only accessible to userspace, and then
> + to use it to look for and fetch the required firmware. Such type of
> + driver functionality may not even ever be desirable upstream by
> + vendors, and as such is only required to be supported as an interface
> + for provisioning. Since udev's firmware loading facility has been
> + removed you can use firmwared or a fork of it to customize how you
> + want to load firmware based on uevents issued.
> +
> + Enabling this option will increase your kernel image size by about
> + 13436 bytes.
> +
> + If you are unsure about this, say N here, unless you are Linux
> + distribution and need to support the above two drivers, or you are
> + certain you need to support some really custom firmware loading
> + facility in userspace.
>
> config FW_LOADER_USER_HELPER_FALLBACK
> - bool "Fallback user-helper invocation for firmware loading"
> - depends on FW_LOADER
> - select FW_LOADER_USER_HELPER
> + bool "Force the firmware sysfs fallback mechanism when possible"
> + depends on FW_LOADER_USER_HELPER
> help
> - This option enables / disables the invocation of user-helper
> - (e.g. udev) for loading firmware files as a fallback after the
> - direct file loading in kernel fails. The user-mode helper is
> - no longer required unless you have a special firmware file that
> - resides in a non-standard path. Moreover, the udev support has
> - been deprecated upstream.
> + Enabling this option forces a sysfs userspace fallback mechanism
> + to be used for all firmware requests which explicitly do not disable a
> + a fallback mechanism. Firmware calls which do prohibit a fallback
> + mechanism is request_firmware_direct(). This option is kept for
> + backward compatibility purposes given this precise mechanism can also
> + be enabled by setting the proc sysctl value to true:
> +
> + /proc/sys/kernel/firmware_config/force_sysfs_fallback
>
> If you are unsure about this, say N here.
>
> +endif # FW_LOADER
> +endmenu
> +
> config WANT_DEV_COREDUMP
> bool
> help
> --
> 2.17.0
>
-Kees
--
Kees Cook
Pixel Security
Powered by blists - more mailing lists