From: Linus WALLEIJ on 7 May 2010 03:00 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 7 May 2010 03:20 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 7 May 2010 05:40 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 7 May 2010 05:50 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 7 May 2010 06:00 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/
|
Next
|
Last
Pages: 1 2 3 Prev: Two small fixes for 2.6.34 Next: [PATCH 1/2] DMA ENGINE: Do not reset 'private' of channel |