public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Parisc: Check kmalloc return value before use the buffer in ccio-dma.c
@ 2010-05-06  2:42 wzt.wzt
  2010-05-06  4:03 ` Kyle McMartin
  0 siblings, 1 reply; 4+ messages in thread
From: wzt.wzt @ 2010-05-06  2:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-parisc, kyle, deller, jejb

Check kmalloc return value before use the buffer.

Signed-off-by: Zhitong Wang <zhitong.wangzt@alibaba-inc.com>

---
 drivers/parisc/ccio-dma.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index f511e70..f465417 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -1407,6 +1407,11 @@ static void __init ccio_init_resources(struct ioc *ioc)
 	struct resource *res = ioc->mmio_region;
 	char *name = kmalloc(14, GFP_KERNEL);
 
+	if (!name) {
+		printk(KERN_ERR "%s() failed to get enough memory\n", __func__);
+		return ;
+	}
+
 	snprintf(name, 14, "GSC Bus [%d/]", ioc->hw_path);
 
 	ccio_init_resource(res, name, &ioc->ioc_regs->io_io_low);
-- 
1.6.5.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Parisc: Check kmalloc return value before use the buffer in ccio-dma.c
  2010-05-06  2:42 [PATCH] Parisc: Check kmalloc return value before use the buffer in ccio-dma.c wzt.wzt
@ 2010-05-06  4:03 ` Kyle McMartin
  2010-05-06 10:59   ` Matthew Wilcox
  0 siblings, 1 reply; 4+ messages in thread
From: Kyle McMartin @ 2010-05-06  4:03 UTC (permalink / raw)
  To: wzt.wzt; +Cc: linux-kernel, linux-parisc, kyle, deller, jejb

On Thu, May 06, 2010 at 10:42:08AM +0800, wzt.wzt@gmail.com wrote:
> +	if (!name) {
> +		printk(KERN_ERR "%s() failed to get enough memory\n", __func__);
> +		return ;
> +	}
> +
>  	snprintf(name, 14, "GSC Bus [%d/]", ioc->hw_path);

This code can't fail, since the failure case won't be handled by the
probe function, and will continue along without properly initializing
the resources and will fail later. While we could add proper error
handling, if these functions are called, the ccio IOMMU exists on the
machine, and without it, we can't do any DMA (amongst other things.)
So in that case, if this kmalloc fails (which it really shouldn't...)
we're pretty much screwed.

We can change it to GFP_ATOMIC|__GFP_NOFAIL so it will retry infinitely,
or we can panic in the error path with a nice error message so the users
knows why his machine isn't going to work.

I suggest the latter since you'll have the test to squelch gcc warnings
and such.

regards, Kyle


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Parisc: Check kmalloc return value before use the buffer in ccio-dma.c
  2010-05-06  4:03 ` Kyle McMartin
@ 2010-05-06 10:59   ` Matthew Wilcox
  2010-05-06 20:08     ` Kyle McMartin
  0 siblings, 1 reply; 4+ messages in thread
From: Matthew Wilcox @ 2010-05-06 10:59 UTC (permalink / raw)
  To: Kyle McMartin; +Cc: wzt.wzt, linux-kernel, linux-parisc, deller, jejb

On Thu, May 06, 2010 at 12:03:31AM -0400, Kyle McMartin wrote:
> We can change it to GFP_ATOMIC|__GFP_NOFAIL so it will retry infinitely,
> or we can panic in the error path with a nice error message so the users
> knows why his machine isn't going to work.

If we can't initialise the IOMMU, can we even get a panic message out to
the user?

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Parisc: Check kmalloc return value before use the buffer in ccio-dma.c
  2010-05-06 10:59   ` Matthew Wilcox
@ 2010-05-06 20:08     ` Kyle McMartin
  0 siblings, 0 replies; 4+ messages in thread
From: Kyle McMartin @ 2010-05-06 20:08 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Kyle McMartin, wzt.wzt, linux-kernel, linux-parisc, deller, jejb

On Thu, May 06, 2010 at 04:59:48AM -0600, Matthew Wilcox wrote:
> On Thu, May 06, 2010 at 12:03:31AM -0400, Kyle McMartin wrote:
> > We can change it to GFP_ATOMIC|__GFP_NOFAIL so it will retry infinitely,
> > or we can panic in the error path with a nice error message so the users
> > knows why his machine isn't going to work.
> 
> If we can't initialise the IOMMU, can we even get a panic message out to
> the user?
> 

Excellent question! I'm not sure. IODC console will still be working up
until it gets bonked, so early_console is probably still functional up
until that point.

--Kyle

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-05-06 20:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-06  2:42 [PATCH] Parisc: Check kmalloc return value before use the buffer in ccio-dma.c wzt.wzt
2010-05-06  4:03 ` Kyle McMartin
2010-05-06 10:59   ` Matthew Wilcox
2010-05-06 20:08     ` Kyle McMartin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox