On Tue, Dec 11, 2018 at 01:07:45PM -0700, Jordan Crouse wrote:
> Currently the data structure for llc-slice is devm allocated and
> stored as a global but never cleared if the probe function fails.
> This is a problem because devm managed memory gets freed on probe
> failure the API functions could access the pointer after it has been
> freed.
>
> Initialize the drv_data pointer to an error and reset it to an error
> on probe failure or device destroy and add protection to the API
> functions to make sure the memory doesn't get accessed.
>
> Signed-off-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>
> ---
> drivers/soc/qcom/llcc-sdm845.c | 6 +++
> drivers/soc/qcom/llcc-slice.c | 71 +++++++++++++++++++++++-------
> include/linux/soc/qcom/llcc-qcom.h | 6 +++
> 3 files changed, 66 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/soc/qcom/llcc-sdm845.c b/drivers/soc/qcom/llcc-sdm845.c
> index 2e1e4f0a5db8..86600d97c36d 100644
> --- a/drivers/soc/qcom/llcc-sdm845.c
> +++ b/drivers/soc/qcom/llcc-sdm845.c
> @@ -71,6 +71,11 @@ static struct llcc_slice_config sdm845_data[] = {
> SCT_ENTRY(LLCC_AUDHW, 22, 1024, 1, 1, 0xffc, 0x2, 0, 0, 1, 1, 0),
> };
>
> +static int sdm845_qcom_llcc_remove(struct platform_device *pdev)
> +{
> + return qcom_llcc_remove(pdev);
> +}
> +
> static int sdm845_qcom_llcc_probe(struct platform_device *pdev)
> {
> return qcom_llcc_probe(pdev, sdm845_data, ARRAY_SIZE(sdm845_data));
> @@ -87,6 +92,7 @@ static struct platform_driver sdm845_qcom_llcc_driver = {
> .of_match_table = sdm845_qcom_llcc_of_match,
> },
> .probe = sdm845_qcom_llcc_probe,
> + .remove = sdm845_qcom_llcc_remove,
> };
> module_platform_driver(sdm845_qcom_llcc_driver);
>
> diff --git a/drivers/soc/qcom/llcc-slice.c b/drivers/soc/qcom/llcc-slice.c
> index 80667f7be52c..8390bc006a31 100644
> --- a/drivers/soc/qcom/llcc-slice.c
> +++ b/drivers/soc/qcom/llcc-slice.c
> @@ -46,7 +46,7 @@
>
> #define BANK_OFFSET_STRIDE 0x80000
>
> -static struct llcc_drv_data *drv_data;
> +static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
I'd rather this be left null
>
> static const struct regmap_config llcc_regmap_config = {
> .reg_bits = 32,
> @@ -68,6 +68,9 @@ struct llcc_slice_desc *llcc_slice_getd(u32 uid)
> struct llcc_slice_desc *desc;
> u32 sz, count;
>
> + if (IS_ERR(drv_data))
IS_ERR_OR_NULL would catch the null from above. Are you using the EPROBE_DEFER
to probe defer somewhere else?
Regards,
Andy
