[PATCH 1/2] bus: arm-ccn: Check memory allocation failure

[PATCH 1/2] bus: arm-ccn: Check memory allocation failure

[Christophe JAILLET]

Check memory allocation failures and return -ENOMEM in such cases

This avoids a potential NULL pointer dereference.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/bus/arm-ccn.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
index e8c6946fed9d..c0e851a0a3d7 100644
--- a/drivers/bus/arm-ccn.c
+++ b/drivers/bus/arm-ccn.c
@@ -1271,6 +1271,10 @@ static int arm_ccn_pmu_init(struct arm_ccn *ccn)
 		int len = snprintf(NULL, 0, "ccn_%d", ccn->dt.id);
 
 		name = devm_kzalloc(ccn->dev, len + 1, GFP_KERNEL);
+		if (!name) {
+			err = -ENOMEM;
+			goto error_choose_name;
+		}
 		snprintf(name, len + 1, "ccn_%d", ccn->dt.id);
 	}
 
@@ -1318,6 +1322,7 @@ static int arm_ccn_pmu_init(struct arm_ccn *ccn)
 
 error_pmu_register:
 error_set_affinity:
+error_choose_name:
 	ida_simple_remove(&arm_ccn_pmu_ida, ccn->dt.id);
 	for (i = 0; i < ccn->num_xps; i++)
 		writel(0, ccn->xp[i].base + CCN_XP_DT_CONTROL);
-- 
2.11.0

Re: [PATCH 1/2] bus: arm-ccn: Check memory allocation failure

[Scott Branden]

Change looks good.


On 17-08-27 03:06 AM, Christophe JAILLET wrote:
> Check memory allocation failures and return -ENOMEM in such cases
>
> This avoids a potential NULL pointer dereference.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Acked-by: Scott Branden <scott.branden@broadcom.com>
> ---
>   drivers/bus/arm-ccn.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
> index e8c6946fed9d..c0e851a0a3d7 100644
> --- a/drivers/bus/arm-ccn.c
> +++ b/drivers/bus/arm-ccn.c
> @@ -1271,6 +1271,10 @@ static int arm_ccn_pmu_init(struct arm_ccn *ccn)
>   		int len = snprintf(NULL, 0, "ccn_%d", ccn->dt.id);
>   
>   		name = devm_kzalloc(ccn->dev, len + 1, GFP_KERNEL);
> +		if (!name) {
> +			err = -ENOMEM;
> +			goto error_choose_name;
> +		}
>   		snprintf(name, len + 1, "ccn_%d", ccn->dt.id);
>   	}
>   
> @@ -1318,6 +1322,7 @@ static int arm_ccn_pmu_init(struct arm_ccn *ccn)
>   
>   error_pmu_register:
>   error_set_affinity:
> +error_choose_name:
>   	ida_simple_remove(&arm_ccn_pmu_ida, ccn->dt.id);
>   	for (i = 0; i < ccn->num_xps; i++)
>   		writel(0, ccn->xp[i].base + CCN_XP_DT_CONTROL);

Re: [PATCH 1/2] bus: arm-ccn: Check memory allocation failure

[Pawel Moll]

On Sun, 2017-08-27 at 12:06 +0200, Christophe JAILLET wrote:
> Check memory allocation failures and return -ENOMEM in such cases
> 
> This avoids a potential NULL pointer dereference.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

This is an obvious bug, thanks for spotting and fixing it! I'll include
this patch next time I send a pull request for CCN fixes.

May I also ask how have you noticed it? Some automated tool or just
manual code inspection?

Regards

Pawel

Re: [PATCH 1/2] bus: arm-ccn: Check memory allocation failure

[Christophe JAILLET]

Le 29/08/2017 à 10:49, Pawel Moll a écrit :
> On Sun, 2017-08-27 at 12:06 +0200, Christophe JAILLET wrote:
>> Check memory allocation failures and return -ENOMEM in such cases
>>
>> This avoids a potential NULL pointer dereference.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> This is an obvious bug, thanks for spotting and fixing it! I'll include
> this patch next time I send a pull request for CCN fixes.
>
> May I also ask how have you noticed it? Some automated tool or just
> manual code inspection?
>
> Regards
>
> Pawel
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Hi,

this has been found using coccinelle and the following script:

----------------------

// find calls to kmalloc or equivalent function
@call@
expression ptr;
position p;
@@

(
    ptr@p = kmalloc(...)
|
    ptr@p = kzalloc(...)
|
    ptr@p = kcalloc(...)
|
    ptr@p = kmemdup(...)
|
    ptr@p = kstrdup(...)
|
    ptr@p = kstrdup_const(...)
|
    ptr@p = kstrndup(...)
|
    ptr@p = kmalloc_array(...)
|
    ptr@p = devm_kmalloc(...)
|
    ptr@p = devm_kzalloc(...)
|
    ptr@p = devm_kcalloc(...)
|
    ptr@p = devm_kmalloc_array(...)
|
    ptr@p = devm_kmemdup(...)
|
    ptr@p = devm_kstrdup(...)
)


// Find ok calls with allocation failure check
//... when != ptr
@ok@
expression ptr;
position call.p;
identifier f;
@@

ptr@p = f(...);
...
(
    (ptr = NULL || ...)
|
    (ptr = 0 || ...)
|
    (ptr != NULL || ...)
|
    ((ptr) = NULL || ...)
|
    ((ptr) = 0 || ...)
|
    ((ptr) != NULL || ...)
|
    (BUG_ON(ptr = NULL))
)


// Find bad calls without any check
@depends on !ok@
expression ptr;
position call.p;
identifier f;
constant C;
@@

*  ptr@p = f(...);
    ...
(
    return -C;
|
    return ret;
|
    return err;
)