[lkml]   [2010]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
Subject[PATCH RFC] ARM ETM driver: Do not deref potentially null pointer and don't allocate and free mem while holding lock.

Looking at etb_read() in arch/arm/kernel/etm.c I noticed two things.

1. We are allocting and freeing 'buf' with vmalloc() while holding a
mutex locked. I cannot see any reason why we have to hold the mutex
just to allocate and free a bit of memory, so I moved it outside the

2. If the memory allocation fails we'll dereference a null pointer
further down since we never check the vmalloc() return value.
for (i = 0; i < length / 4; i++)
buf[i] = etb_readl(t, ETBR_READMEM);
The best way I could find to handle this was to simply return 0 when
we can't allocate memory, but there might be a better option that I
just couldn't find - in any case it is better than crashing the

Please consider merging, but please also review carefully first since I'm
not familliar with this code.

CC me on replies please.

Signed-off-by: Jesper Juhl <>
etm.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
note: completely untested patch since I have neither hardware nor
toolchain to test it, so please review carefully and test before applying

diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c
index 11db628..30f845b 100644
--- a/arch/arm/kernel/etm.c
+++ b/arch/arm/kernel/etm.c
@@ -274,7 +274,10 @@ static ssize_t etb_read(struct file *file, char __user *data,
long length;
struct tracectx *t = file->private_data;
u32 first = 0;
- u32 *buf;
+ u32 *buf = vmalloc(length);
+ if (!buf)
+ return 0;


@@ -292,7 +295,6 @@ static ssize_t etb_read(struct file *file, char __user *data,
etb_writel(t, first, ETBR_READADDR);

length = min(total * 4, (int)len);
- buf = vmalloc(length);

dev_dbg(t->dev, "ETB buffer length: %d\n", total);
dev_dbg(t->dev, "ETB status reg: %x\n", etb_readl(t, ETBR_STATUS));
@@ -310,10 +312,10 @@ static ssize_t etb_read(struct file *file, char __user *data,

length -= copy_to_user(data, buf, length);
- vfree(buf);

+ vfree(buf);

return length;

Jesper Juhl <>
Don't top-post
Plain text mails only, please.

 \ /
  Last update: 2010-11-07 20:27    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean