| 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: <CAOCOHw4WYM66itYRmu35_AP9ZMoTvmXMqRmS5LfAcBNAGdZaow@mail.gmail.com>
Date: Mon, 26 Jun 2017 13:22:41 -0700
From: Bjorn Andersson <bjorn.andersson@...aro.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: "Luis R. Rodriguez" <mcgrof@...nel.org>,
Daniel Wagner <wagi@...om.org>,
David Woodhouse <dwmw2@...radead.org>, rafal@...ecki.pl,
Arend van Spriel <arend.vanspriel@...adcom.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>, yi1.li@...ux.intel.com,
atull@...nsource.altera.com,
Moritz Fischer <moritz.fischer@...us.com>, pmladek@...e.com,
Johannes Berg <johannes.berg@...el.com>,
emmanuel.grumbach@...el.com, luciano.coelho@...el.com,
Kalle Valo <kvalo@...eaurora.org>, luto@...nel.org,
Linus Torvalds <torvalds@...ux-foundation.org>,
Kees Cook <keescook@...omium.org>,
"AKASHI, Takahiro" <takahiro.akashi@...aro.org>,
David Howells <dhowells@...hat.com>, pjones@...hat.com,
Hans de Goede <hdegoede@...hat.com>, alan@...ux.intel.com,
tytso@....edu, lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] firmware: remove request_firmware_into_buf()
On Fri, Jun 23, 2017 at 9:03 AM, Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
> As Luis pointed out, there are no in-kernel users of
> request_firmware_into_buf(), so remove it, and the now unused internal
> flag, which simplifies the logic around buffer handling a bit.
>
This API was implemented to reduce the memory pressure during firmware
load in the Qualcomm remoteprocs, but it wasn't available when I
upstreamed that code and I apparently forgot to send out the patch
moving us over to use this API...
Especially when loading the Qualcomm modem we have a couple of files
that we request_firmware() that are 10-15MB in size, so this
functionality is definitely wanted.
As we are calling release_firmware() immediately following the
request_firmware() I did attempt to just call
kernel_read_file_from_path() directly, but as I don't have access to
the fw_path[] this becomes inconsistent, so I would like to keep
request_firmware_into_buf().
Regards,
Bjorn
> Reported-by: "Luis R. Rodriguez" <mcgrof@...nel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
> Documentation/driver-api/firmware/fallback-mechanisms.rst | 3 -
> Documentation/driver-api/firmware/firmware_cache.rst | 2
> Documentation/driver-api/firmware/request_firmware.rst | 5 -
> drivers/base/firmware_class.c | 37 --------------
> include/linux/firmware.h | 8 ---
> 5 files changed, 4 insertions(+), 51 deletions(-)
>
>
> diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> index d19354794e67..0c3562148161 100644
> --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
> +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> @@ -38,8 +38,7 @@ fallback mechanism:
> * Race against access with the root filesystem upon bootup.
>
> * Races upon resume from suspend. This is resolved by the firmware cache, but
> - the firmware cache is only supported if you use uevents, and its not
> - supported for request_firmware_into_buf().
> + the firmware cache is only supported if you use uevents.
>
> * Firmware is not accessible through typical means:
> * It cannot be installed into the root filesystem
> diff --git a/Documentation/driver-api/firmware/firmware_cache.rst b/Documentation/driver-api/firmware/firmware_cache.rst
> index 2210e5bfb332..e53f8189fac1 100644
> --- a/Documentation/driver-api/firmware/firmware_cache.rst
> +++ b/Documentation/driver-api/firmware/firmware_cache.rst
> @@ -24,7 +24,7 @@ root filesystem mounts.
> Some implementation details about the firmware cache setup:
>
> * The firmware cache is setup by adding a devres entry for each device that
> - uses all synchronous call except :c:func:`request_firmware_into_buf`.
> + uses a synchronous call.
>
> * If an asynchronous call is used the firmware cache is only set up for a
> device if if the second argument (uevent) to request_firmware_nowait() is
> diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
> index cc0aea880824..a5dacf96ab0f 100644
> --- a/Documentation/driver-api/firmware/request_firmware.rst
> +++ b/Documentation/driver-api/firmware/request_firmware.rst
> @@ -25,11 +25,6 @@ request_firmware_direct
> .. kernel-doc:: drivers/base/firmware_class.c
> :functions: request_firmware_direct
>
> -request_firmware_into_buf
> --------------------------
> -.. kernel-doc:: drivers/base/firmware_class.c
> - :functions: request_firmware_into_buf
> -
> Asynchronous firmware requests
> ==============================
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index ac350c518e0c..a913400c873e 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -196,7 +196,6 @@ static int __fw_state_check(struct fw_state *fw_st, enum fw_status status)
> #define FW_OPT_FALLBACK 0
> #endif
> #define FW_OPT_NO_WARN (1U << 3)
> -#define FW_OPT_NOCACHE (1U << 4)
>
> struct firmware_cache {
> /* firmware_buf instance will be added into the below list */
> @@ -1143,16 +1142,14 @@ static int assign_firmware_buf(struct firmware *fw, struct device *device,
> * should be fixed in devres or driver core.
> */
> /* don't cache firmware handled without uevent */
> - if (device && (opt_flags & FW_OPT_UEVENT) &&
> - !(opt_flags & FW_OPT_NOCACHE))
> + if (device && (opt_flags & FW_OPT_UEVENT))
> fw_add_devm_name(device, buf->fw_id);
>
> /*
> * After caching firmware image is started, let it piggyback
> * on request firmware.
> */
> - if (!(opt_flags & FW_OPT_NOCACHE) &&
> - buf->fwc->state == FW_LOADER_START_CACHE) {
> + if (buf->fwc->state == FW_LOADER_START_CACHE) {
> if (fw_cache_piggyback_on_request(buf->fw_id))
> kref_get(&buf->ref);
> }
> @@ -1292,36 +1289,6 @@ int request_firmware_direct(const struct firmware **firmware_p,
> EXPORT_SYMBOL_GPL(request_firmware_direct);
>
> /**
> - * request_firmware_into_buf - load firmware into a previously allocated buffer
> - * @firmware_p: pointer to firmware image
> - * @name: name of firmware file
> - * @device: device for which firmware is being loaded and DMA region allocated
> - * @buf: address of buffer to load firmware into
> - * @size: size of buffer
> - *
> - * This function works pretty much like request_firmware(), but it doesn't
> - * allocate a buffer to hold the firmware data. Instead, the firmware
> - * is loaded directly into the buffer pointed to by @buf and the @firmware_p
> - * data member is pointed at @buf.
> - *
> - * This function doesn't cache firmware either.
> - */
> -int
> -request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
> - struct device *device, void *buf, size_t size)
> -{
> - int ret;
> -
> - __module_get(THIS_MODULE);
> - ret = _request_firmware(firmware_p, name, device, buf, size,
> - FW_OPT_UEVENT | FW_OPT_FALLBACK |
> - FW_OPT_NOCACHE);
> - module_put(THIS_MODULE);
> - return ret;
> -}
> -EXPORT_SYMBOL(request_firmware_into_buf);
> -
> -/**
> * release_firmware: - release the resource associated with a firmware image
> * @fw: firmware resource to release
> **/
> diff --git a/include/linux/firmware.h b/include/linux/firmware.h
> index b1f9f0ccb8ac..5c41c5e75b5c 100644
> --- a/include/linux/firmware.h
> +++ b/include/linux/firmware.h
> @@ -47,8 +47,6 @@ int request_firmware_nowait(
> void (*cont)(const struct firmware *fw, void *context));
> int request_firmware_direct(const struct firmware **fw, const char *name,
> struct device *device);
> -int request_firmware_into_buf(const struct firmware **firmware_p,
> - const char *name, struct device *device, void *buf, size_t size);
>
> void release_firmware(const struct firmware *fw);
> #else
> @@ -77,11 +75,5 @@ static inline int request_firmware_direct(const struct firmware **fw,
> return -EINVAL;
> }
>
> -static inline int request_firmware_into_buf(const struct firmware **firmware_p,
> - const char *name, struct device *device, void *buf, size_t size)
> -{
> - return -EINVAL;
> -}
> -
> #endif
> #endif
Powered by blists - more mailing lists