On Tue, Feb 27, 2018 at 3:20 PM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote:
> fw_add_devm_name() adds device's name onto the devres for the
> device so that prior to suspend we cache the firmware onto memory,
> so that on resume the firmware is reliably available. We never
> were checking for success for this call though, meaning in some
> really rare cases we my have never setup the firmware cache for
> a device, which could in turn make resume fail.
>
> This is all theoretical, no known issues have been reported.
> This small issue has been present way since the addition of the
> devres firmware cache names on v3.7.
>
> Fixes: f531f05ae9437 ("firmware loader: store firmware name into devres list")
> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> ---
> drivers/base/firmware_loader.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
> index 21dd31ef08ae..48932581c70c 100644
> --- a/drivers/base/firmware_loader.c
> +++ b/drivers/base/firmware_loader.c
> @@ -431,6 +431,7 @@ int assign_fw(struct firmware *fw, struct device *device,
> unsigned int opt_flags)
> {
> struct fw_priv *fw_priv = fw->priv;
> + int ret;
>
> mutex_lock(&fw_lock);
> if (!fw_priv->size || fw_state_is_aborted(fw_priv)) {
> @@ -447,8 +448,13 @@ int assign_fw(struct firmware *fw, struct device *device,
> */
> /* don't cache firmware handled without uevent */
> if (device && (opt_flags & FW_OPT_UEVENT) &&
> - !(opt_flags & FW_OPT_NOCACHE))
> - fw_add_devm_name(device, fw_priv->fw_name);
> + !(opt_flags & FW_OPT_NOCACHE)) {
> + ret = fw_add_devm_name(device, fw_priv->fw_name);
> + if (ret && ret != 1) {
Why not if (ret < 0) ?
-Kees
> + mutex_unlock(&fw_lock);
> + return ret;
> + }
> + }
>
> /*
> * After caching firmware image is started, let it piggyback
> --
> 2.16.2
>
--
Kees Cook
Pixel Security
