On Thu, 13 Aug 2020 22:00:22 +0200,
Prateek Sood wrote:
>
> vfree() is being called on paged buffer allocated
> using alloc_page() and mapped using vmap().
>
> Freeing of pages in vfree() relies on nr_pages of
> struct vm_struct. vmap() does not update nr_pages.
> It can lead to memory leaks.
>
> Fixes: ddaf29fd9bb6 ("firmware: Free temporary page table after vmapping")
> Signed-off-by: Prateek Sood <prsood@xxxxxxxxxxxxxx>
> Reviewed-by: Takashi Iwai <tiwai@xxxxxxx>
Note that, for including to stable kernels properly, you'd need to put
a Cc-to-stable line around the signed-off-by section in the patch
itself. OTOH, you don't have to send the patch itself to stable when
you submit it.
thanks,
Takashi
> ---
> drivers/base/firmware_loader/firmware.h | 2 ++
> drivers/base/firmware_loader/main.c | 17 +++++++++++------
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
> index 933e2192..d08efc7 100644
> --- a/drivers/base/firmware_loader/firmware.h
> +++ b/drivers/base/firmware_loader/firmware.h
> @@ -142,10 +142,12 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
> void fw_free_paged_buf(struct fw_priv *fw_priv);
> int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed);
> int fw_map_paged_buf(struct fw_priv *fw_priv);
> +bool fw_is_paged_buf(struct fw_priv *fw_priv);
> #else
> static inline void fw_free_paged_buf(struct fw_priv *fw_priv) {}
> static inline int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed) { return -ENXIO; }
> static inline int fw_map_paged_buf(struct fw_priv *fw_priv) { return -ENXIO; }
> +static inline bool fw_is_paged_buf(struct fw_priv *fw_priv) { return false; }
> #endif
>
> #endif /* __FIRMWARE_LOADER_H */
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index ca871b1..36bf455 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -252,9 +252,11 @@ static void __free_fw_priv(struct kref *ref)
> list_del(&fw_priv->list);
> spin_unlock(&fwc->lock);
>
> - fw_free_paged_buf(fw_priv); /* free leftover pages */
> - if (!fw_priv->allocated_size)
> + if (fw_is_paged_buf(fw_priv))
> + fw_free_paged_buf(fw_priv);
> + else if (!fw_priv->allocated_size)
> vfree(fw_priv->data);
> +
> kfree_const(fw_priv->fw_name);
> kfree(fw_priv);
> }
> @@ -268,6 +270,11 @@ static void free_fw_priv(struct fw_priv *fw_priv)
> }
>
> #ifdef CONFIG_FW_LOADER_PAGED_BUF
> +bool fw_is_paged_buf(struct fw_priv *fw_priv)
> +{
> + return fw_priv->is_paged_buf;
> +}
> +
> void fw_free_paged_buf(struct fw_priv *fw_priv)
> {
> int i;
> @@ -275,6 +282,8 @@ void fw_free_paged_buf(struct fw_priv *fw_priv)
> if (!fw_priv->pages)
> return;
>
> + vunmap(fw_priv->data);
> +
> for (i = 0; i < fw_priv->nr_pages; i++)
> __free_page(fw_priv->pages[i]);
> kvfree(fw_priv->pages);
> @@ -328,10 +337,6 @@ int fw_map_paged_buf(struct fw_priv *fw_priv)
> if (!fw_priv->data)
> return -ENOMEM;
>
> - /* page table is no longer needed after mapping, let's free */
> - kvfree(fw_priv->pages);
> - fw_priv->pages = NULL;
> -
> return 0;
> }
> #endif
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>
