From: Linus WALLEIJ on
Hi Liu,

> Hi,linus.walleij
>
> Check kmalloc return value before use the buffer
>
>
> Signed-off-by: LiuQi <lingjiujianke(a)gmail.com>
> ---
> arch/arm/mach-u300/dummyspichip.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-u300/dummyspichip.c
> b/arch/arm/mach-u300/dummyspichip.c
> index 962f9de..4f2af7c 100644
> --- a/arch/arm/mach-u300/dummyspichip.c
> +++ b/arch/arm/mach-u300/dummyspichip.c
> @@ -63,6 +63,11 @@ static ssize_t dummy_looptest(struct device *dev,
> goto out;
> }
> bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
> + if (bigtxbuf_virtual == NULL) {
> + status = -ENOMEM;
> + kfree(bigtxbuf_virtual);

kfree():ing NULL is OK, but you just checked it to be NULL so why?

> + goto out;
> + }
>
> /* Fill TXBUF with some happy pattern */
> memset(bigtxbuf_virtual, 0xAA, DMA_TEST_SIZE);

Otherwise the bug it fixes is good to fix.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Steven Liu on
Oh,I think I have a mistake


Signed-off-by: root <root(a)T-bagwell.(none)>
---
arch/arm/mach-u300/dummyspichip.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-u300/dummyspichip.c
b/arch/arm/mach-u300/dummyspichip.c
index 5f55012..df19f9b 100644
--- a/arch/arm/mach-u300/dummyspichip.c
+++ b/arch/arm/mach-u300/dummyspichip.c
@@ -64,6 +64,11 @@ static ssize_t dummy_looptest(struct device *dev,
goto out;
}
bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
+ if (bigrxbuf_virtual == NULL) {
+ status = -ENOMEM;
+ kfree(bigtxbuf_virtual);
+ goto out;
+ }

/* Fill TXBUF with some happy pattern */
memset(bigtxbuf_virtual, 0xAA, DMA_TEST_SIZE);
--
1.6.0.3





what a bout this one, please?


Best regards!



2010/5/7 Linus WALLEIJ <linus.walleij(a)stericsson.com>:
> Hi Liu,
>
>> Hi,linus.walleij
>>
>> � � � � � Check kmalloc return value before use the buffer
>>
>>
>> Signed-off-by: LiuQi <lingjiujianke(a)gmail.com>
>> ---
>> �arch/arm/mach-u300/dummyspichip.c | � �5 +++++
>> �1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-u300/dummyspichip.c
>> b/arch/arm/mach-u300/dummyspichip.c
>> index 962f9de..4f2af7c 100644
>> --- a/arch/arm/mach-u300/dummyspichip.c
>> +++ b/arch/arm/mach-u300/dummyspichip.c
>> @@ -63,6 +63,11 @@ static ssize_t dummy_looptest(struct device *dev,
>> � � � � � � � goto out;
>> � � � }
>> � � � bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
>> + � � if (bigtxbuf_virtual == NULL) {
>> + � � � � � � status = -ENOMEM;
>> + � � � � � � kfree(bigtxbuf_virtual);
>
> kfree():ing NULL is OK, but you just checked it to be NULL so why?
>
>> + � � � � � � goto out;
>> + � � }
>>
>> � � � /* Fill TXBUF with some happy pattern */
>> � � � memset(bigtxbuf_virtual, 0xAA, DMA_TEST_SIZE);
>
> Otherwise the bug it fixes is good to fix.
>
> Yours,
> Linus Walleij
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nigel Cunningham on
Hi.

No commit comment?

On 07/05/10 17:17, Steven Liu wrote:
> Signed-off-by: LiuQi<lingjiujianke(a)gmail.com>
> ---
> arch/arm/mach-u300/dummyspichip.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-u300/dummyspichip.c
> b/arch/arm/mach-u300/dummyspichip.c
> index 5f55012..df19f9b 100644
> --- a/arch/arm/mach-u300/dummyspichip.c
> +++ b/arch/arm/mach-u300/dummyspichip.c
> @@ -64,6 +64,11 @@ static ssize_t dummy_looptest(struct device *dev,
> goto out;
> }
> bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
> + if (bigrxbuf_virtual == NULL) {
> + status = -ENOMEM;
> + kfree(bigtxbuf_virtual);

Why kfree something you know is NULL?

Regards,

Nigel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Steven Liu on
Hi, guy,

the code in arch/arm/mach-u300/dummyspichip.c is

bigtxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
if (bigtxbuf_virtual == NULL) {
status = -ENOMEM;
goto out;
}
bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);


if kmalloc memory space for bigrxbuf_virtual is NULL, when it have
kmalloc DMA_TEST_SIZE memory space for bigtxbuf_virtual,so ,if kmalloc
memory for bigtxbuf_virtual success and kmalloc memory for
bigrxbuf_virtual faild,i think we must kfree bigtxbuf_virtual memory



best regards,



LiuQi
2010/5/7 Nigel Cunningham <nigel(a)tuxonice.net>:
> Hi.
>
> No commit comment?
>
> On 07/05/10 17:17, Steven Liu wrote:
>>
>> Signed-off-by: LiuQi<lingjiujianke(a)gmail.com>
>> ---
>> �arch/arm/mach-u300/dummyspichip.c | � �5 +++++
>> �1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-u300/dummyspichip.c
>> b/arch/arm/mach-u300/dummyspichip.c
>> index 5f55012..df19f9b 100644
>> --- a/arch/arm/mach-u300/dummyspichip.c
>> +++ b/arch/arm/mach-u300/dummyspichip.c
>> @@ -64,6 +64,11 @@ static ssize_t dummy_looptest(struct device *dev,
>> � � � � � � � �goto out;
>> � � � �}
>> � � � �bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
>> + � � � if (bigrxbuf_virtual == NULL) {
>> + � � � � � � � status = -ENOMEM;
>> + � � � � � � � kfree(bigtxbuf_virtual);
>
> Why kfree something you know is NULL?
>
> Regards,
>
> Nigel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nigel Cunningham on
Hi.

On 07/05/10 19:47, Steven Liu wrote:
> Hi, guy,
>
> the code in arch/arm/mach-u300/dummyspichip.c is
>
> bigtxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
> if (bigtxbuf_virtual == NULL) {
> status = -ENOMEM;
> goto out;
> }
> bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
>
>
> if kmalloc memory space for bigrxbuf_virtual is NULL, when it have
> kmalloc DMA_TEST_SIZE memory space for bigtxbuf_virtual,so ,if kmalloc
> memory for bigtxbuf_virtual success and kmalloc memory for
> bigrxbuf_virtual faild,i think we must kfree bigtxbuf_virtual memory

Ah, I see. I misread. Humble apologies :)

The other point still applies: You need to add a commit comment - even a
simple one.

Regards,

Nigel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/