Re: [PATCH] soc: qcom: llcc-slice: Add error checks for API functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jordan

On Tue, Sep 25, 2018 at 2:56 AM Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> wrote:
>
> llcc_slice_getd can return a ERR_PTR code on failure. Add a IS_ERR_OR_NULL
> check to subsequent API calls that use struct llcc_slice_desc to guard
> against faults and to let the leaf drivers get away with safely using a
> ERR_PTR() encoded "pointer" in the aftermath of a llcc_slice_getd error.

To me calling llcc_slice_putd(), llcc_slice_{de}activate(), and others
with a error pointer sounds like we are on the wrong path anyways.
The users can check for the error pointer return value and assign
llcc_slice_desc to NULL, e.g., something that GPU is trying to do [1].
So we can simply check for NULL value of desc in these APIs.

[1] https://patchwork.freedesktop.org/patch/212400/

>
> Signed-off-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>
> ---
>  drivers/soc/qcom/llcc-slice.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/llcc-slice.c b/drivers/soc/qcom/llcc-slice.c
> index 192ca761b2cb..de1a35cd20ed 100644
> --- a/drivers/soc/qcom/llcc-slice.c
> +++ b/drivers/soc/qcom/llcc-slice.c
> @@ -95,7 +95,8 @@ EXPORT_SYMBOL_GPL(llcc_slice_getd);
>   */
>  void llcc_slice_putd(struct llcc_slice_desc *desc)
>  {
> -       kfree(desc);
> +       if (!IS_ERR_OR_NULL(desc))
> +               kfree(desc);

you will not need this check at all when desc = NULL.

>  }
>  EXPORT_SYMBOL_GPL(llcc_slice_putd);
>
> @@ -142,6 +143,9 @@ int llcc_slice_activate(struct llcc_slice_desc *desc)
>         int ret;
>         u32 act_ctrl_val;
>
> +       if (IS_ERR_OR_NULL(desc))

This can be simply replaced with NULL check.
           if (!desc)

and same in the below hunks.

Best regards
Vivek
> +               return -EINVAL;
> +
>         mutex_lock(&drv_data->lock);
>         if (test_bit(desc->slice_id, drv_data->bitmap)) {
>                 mutex_unlock(&drv_data->lock);
> @@ -176,6 +180,9 @@ int llcc_slice_deactivate(struct llcc_slice_desc *desc)
>         u32 act_ctrl_val;
>         int ret;
>
> +       if (IS_ERR_OR_NULL(desc))
> +               return -EINVAL;
> +
>         mutex_lock(&drv_data->lock);
>         if (!test_bit(desc->slice_id, drv_data->bitmap)) {
>                 mutex_unlock(&drv_data->lock);
> @@ -203,6 +210,9 @@ EXPORT_SYMBOL_GPL(llcc_slice_deactivate);
>   */
>  int llcc_get_slice_id(struct llcc_slice_desc *desc)
>  {
> +       if (IS_ERR_OR_NULL(desc))
> +               return -EINVAL;
> +
>         return desc->slice_id;
>  }
>  EXPORT_SYMBOL_GPL(llcc_get_slice_id);
> @@ -213,6 +223,9 @@ EXPORT_SYMBOL_GPL(llcc_get_slice_id);
>   */
>  size_t llcc_get_slice_size(struct llcc_slice_desc *desc)
>  {
> +       if (IS_ERR_OR_NULL(desc))
> +               return 0;
> +
>         return desc->slice_size;
>  }
>  EXPORT_SYMBOL_GPL(llcc_get_slice_size);
> --
> 2.18.0
>


-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux