[PATCH] osd/erasurecode: Fix memory leak in jerasure_matrix_to_bitmatrix()

[PATCH] osd/erasurecode: Fix memory leak in jerasure_matrix_to_bitmatrix()

[Xing Lin]

Free allocated memory before return because of NULL input

Signed-off-by: Xing Lin <xinglin@cs.utah.edu>
---
 src/osd/ErasureCodePluginJerasure/jerasure.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/osd/ErasureCodePluginJerasure/jerasure.c b/src/osd/ErasureCodePluginJerasure/jerasure.c
index 9efae02..1bb4b1d 100755
--- a/src/osd/ErasureCodePluginJerasure/jerasure.c
+++ b/src/osd/ErasureCodePluginJerasure/jerasure.c
@@ -276,7 +276,10 @@ int *jerasure_matrix_to_bitmatrix(int k, int m, int w, int *matrix)
   int rowelts, rowindex, colindex, elt, i, j, l, x;
 
   bitmatrix = talloc(int, k*m*w*w);
-  if (matrix == NULL) { return NULL; }
+  if (matrix == NULL) {
+    free(bitmatrix);
+    return NULL; 
+  }
 
   rowelts = k * w;
   rowindex = 0;
-- 
1.8.3.4 (Apple Git-47)

Re: [PATCH] osd/erasurecode: Fix memory leak in jerasure_matrix_to_bitmatrix()

[Sage Weil]

Hi!

On Sat, 2 Nov 2013, Xing Lin wrote:
> Free allocated memory before return because of NULL input
> 
> Signed-off-by: Xing Lin <xinglin@cs.utah.edu>
> ---
>  src/osd/ErasureCodePluginJerasure/jerasure.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/osd/ErasureCodePluginJerasure/jerasure.c b/src/osd/ErasureCodePluginJerasure/jerasure.c
> index 9efae02..1bb4b1d 100755
> --- a/src/osd/ErasureCodePluginJerasure/jerasure.c
> +++ b/src/osd/ErasureCodePluginJerasure/jerasure.c
> @@ -276,7 +276,10 @@ int *jerasure_matrix_to_bitmatrix(int k, int m, int w, int *matrix)
>    int rowelts, rowindex, colindex, elt, i, j, l, x;
>  
>    bitmatrix = talloc(int, k*m*w*w);
> -  if (matrix == NULL) { return NULL; }
> +  if (matrix == NULL) {
> +    free(bitmatrix);
> +    return NULL; 
> +  }

I have a feeling the author meant to say if (bitmatrix == NULL), since all 
the other talloc calls in this file tend to be followed by a NULL check?

sage


>  
>    rowelts = k * w;
>    rowindex = 0;
> -- 
> 1.8.3.4 (Apple Git-47)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH] osd/erasurecode: Fix memory leak in jerasure_matrix_to_bitmatrix()

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]



On 02/11/2013 08:03, Sage Weil wrote:
> Hi!
> 
> On Sat, 2 Nov 2013, Xing Lin wrote:
>> Free allocated memory before return because of NULL input
>>
>> Signed-off-by: Xing Lin <xinglin@cs.utah.edu>
>> ---
>>  src/osd/ErasureCodePluginJerasure/jerasure.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/osd/ErasureCodePluginJerasure/jerasure.c b/src/osd/ErasureCodePluginJerasure/jerasure.c
>> index 9efae02..1bb4b1d 100755
>> --- a/src/osd/ErasureCodePluginJerasure/jerasure.c
>> +++ b/src/osd/ErasureCodePluginJerasure/jerasure.c
>> @@ -276,7 +276,10 @@ int *jerasure_matrix_to_bitmatrix(int k, int m, int w, int *matrix)
>>    int rowelts, rowindex, colindex, elt, i, j, l, x;
>>  
>>    bitmatrix = talloc(int, k*m*w*w);
>> -  if (matrix == NULL) { return NULL; }
>> +  if (matrix == NULL) {
>> +    free(bitmatrix);
>> +    return NULL; 
>> +  }
> 
> I have a feeling the author meant to say if (bitmatrix == NULL), since all 
> the other talloc calls in this file tend to be followed by a NULL check?

Yes. Good catch Xing Lin :-)

Cheers

> 
> sage
> 
> 
>>  
>>    rowelts = k * w;
>>    rowindex = 0;
>> -- 
>> 1.8.3.4 (Apple Git-47)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

RE: [PATCH] osd/erasurecode: Fix memory leak in jerasure_matrix_to_bitmatrix()

[philippe.raipin]

Hi !

In this case, it should be more efficient to do the NULL check before the talloc (you'll save the free operation and a useless memory allocation)

Cheers,
Philippe


-----Message d'origine-----
De : ceph-devel-owner@vger.kernel.org [mailto:ceph-devel-owner@vger.kernel.org] De la part de Loic Dachary
Envoyé : samedi 2 novembre 2013 10:17
À : Sage Weil; Xing Lin
Cc : ceph-devel@vger.kernel.org
Objet : Re: [PATCH] osd/erasurecode: Fix memory leak in jerasure_matrix_to_bitmatrix()



On 02/11/2013 08:03, Sage Weil wrote:
> Hi!
> 
> On Sat, 2 Nov 2013, Xing Lin wrote:
>> Free allocated memory before return because of NULL input
>>
>> Signed-off-by: Xing Lin <xinglin@cs.utah.edu>
>> ---
>>  src/osd/ErasureCodePluginJerasure/jerasure.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/osd/ErasureCodePluginJerasure/jerasure.c 
>> b/src/osd/ErasureCodePluginJerasure/jerasure.c
>> index 9efae02..1bb4b1d 100755
>> --- a/src/osd/ErasureCodePluginJerasure/jerasure.c
>> +++ b/src/osd/ErasureCodePluginJerasure/jerasure.c
>> @@ -276,7 +276,10 @@ int *jerasure_matrix_to_bitmatrix(int k, int m, int w, int *matrix)
>>    int rowelts, rowindex, colindex, elt, i, j, l, x;
>>  
>>    bitmatrix = talloc(int, k*m*w*w);
>> -  if (matrix == NULL) { return NULL; }
>> +  if (matrix == NULL) {
>> +    free(bitmatrix);
>> +    return NULL;
>> +  }
> 
> I have a feeling the author meant to say if (bitmatrix == NULL), since 
> all the other talloc calls in this file tend to be followed by a NULL check?

Yes. Good catch Xing Lin :-)

Cheers

> 
> sage
> 
> 
>>  
>>    rowelts = k * w;
>>    rowindex = 0;
>> --
>> 1.8.3.4 (Apple Git-47)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" 
>> in the body of a message to majordomo@vger.kernel.org More majordomo 
>> info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" 
> in the body of a message to majordomo@vger.kernel.org More majordomo 
> info at  http://vger.kernel.org/majordomo-info.html
> 

--
Loïc Dachary, Artisan Logiciel Libre

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] osd/erasurecode: Fix memory leak in jerasure_matrix_to_bitmatrix()

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 2686 bytes --]



On 04/11/2013 18:44, philippe.raipin@orange.com wrote:
> Hi !
> 
> In this case, it should be more efficient to do the NULL check before the talloc (you'll save the free operation and a useless memory allocation)
> 

Hi,

The check is on bitmatrix actually ;-)

Cheers

> Cheers,
> Philippe
> 
> 
> -----Message d'origine-----
> De : ceph-devel-owner@vger.kernel.org [mailto:ceph-devel-owner@vger.kernel.org] De la part de Loic Dachary
> Envoyé : samedi 2 novembre 2013 10:17
> À : Sage Weil; Xing Lin
> Cc : ceph-devel@vger.kernel.org
> Objet : Re: [PATCH] osd/erasurecode: Fix memory leak in jerasure_matrix_to_bitmatrix()
> 
> 
> 
> On 02/11/2013 08:03, Sage Weil wrote:
>> Hi!
>>
>> On Sat, 2 Nov 2013, Xing Lin wrote:
>>> Free allocated memory before return because of NULL input
>>>
>>> Signed-off-by: Xing Lin <xinglin@cs.utah.edu>
>>> ---
>>>  src/osd/ErasureCodePluginJerasure/jerasure.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/osd/ErasureCodePluginJerasure/jerasure.c 
>>> b/src/osd/ErasureCodePluginJerasure/jerasure.c
>>> index 9efae02..1bb4b1d 100755
>>> --- a/src/osd/ErasureCodePluginJerasure/jerasure.c
>>> +++ b/src/osd/ErasureCodePluginJerasure/jerasure.c
>>> @@ -276,7 +276,10 @@ int *jerasure_matrix_to_bitmatrix(int k, int m, int w, int *matrix)
>>>    int rowelts, rowindex, colindex, elt, i, j, l, x;
>>>  
>>>    bitmatrix = talloc(int, k*m*w*w);
>>> -  if (matrix == NULL) { return NULL; }
>>> +  if (matrix == NULL) {
>>> +    free(bitmatrix);
>>> +    return NULL;
>>> +  }
>>
>> I have a feeling the author meant to say if (bitmatrix == NULL), since 
>> all the other talloc calls in this file tend to be followed by a NULL check?
> 
> Yes. Good catch Xing Lin :-)
> 
> Cheers
> 
>>
>> sage
>>
>>
>>>  
>>>    rowelts = k * w;
>>>    rowindex = 0;
>>> --
>>> 1.8.3.4 (Apple Git-47)
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" 
>>> in the body of a message to majordomo@vger.kernel.org More majordomo 
>>> info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" 
>> in the body of a message to majordomo@vger.kernel.org More majordomo 
>> info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> --
> Loïc Dachary, Artisan Logiciel Libre
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]