kmemleak hexdump proposal

Hello.
What do you think about ability to 'watch' leaked region? (hex + ascii).
(done via lib/hexdump.c)
To turn on hex dump:
echo "hexdump=on" > /sys/kernel/debug/kmemleak
/**
Or (as alternative):
echo "hexdump=f6aac7f8" > /sys/kernel/debug/kmemleak
where f6aac7f8 - object's pointer.
**/
cat /sys/kernel/debug/kmemleak
unreferenced object 0xf6aac7f8 (size 32):
comm "swapper", pid 1, jiffies 4294877610
HEX dump:
70 6e 70 20 30 30 3a 30 61 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 pnp 00:0a.ZZZZZZZZZZZZZZZZZZZZZ.
backtrace:
[] kmemleak_alloc+0x11b/0x2b0
[] kmem_cache_alloc+0x111/0x1c0
[] reserve_range+0x3e/0x1b0
[] system_pnp_probe+0x94/0x140
[] pnp_device_probe+0x84/0x100
[] driver_probe_device+0x89/0x170
[] __driver_attach+0x99/0xa0
[] bus_for_each_dev+0x58/0x90
[] driver_attach+0x24/0x40
[] bus_add_driver+0xc4/0x290
[] driver_register+0x70/0x130
[] pnp_register_driver+0x26/0x40
[] pnp_system_init+0x1b/0x2e
[] do_one_initcall+0x3f/0x1a0
[] kernel_init+0x13e/0x1a6
[] kernel_thread_helper+0x7/0x10
unreferenced object 0xf63f4d18 (size 192):
comm "swapper", pid 1, jiffies 4294878292
HEX dump:
3c 06 00 00 00 00 00 00 78 69 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 /sys/kernel/debug/kmemleak
I guess it could safe someone's time.
(May be, showed examples aren't so good. Just to demonstrate the idea.)
(concept. feel free to ask for comments.)
diff -u -p
--- kmemleak.c 2009-06-28 20:18:59.000000000 +0300
+++ linux-2.6-sergey/mm/kmemleak.c 2009-06-28 20:21:29.000000000 +0300
@@ -160,6 +160,13 @@ struct kmemleak_object {
/* flag set to not scan the object */
#define OBJECT_NO_SCAN (1 pointer;
+ int len = object->size;
+ int i, linelen, remaining = object->size;
+ unsigned char linebuf[200];
+
+ seq_printf(seq, "HEX dump:\n");
+
+ for (i = 0; i < len; i += HEX_ROW_SIZE) {
+ linelen = min(remaining, HEX_ROW_SIZE);
+ remaining -= HEX_ROW_SIZE;
+ hex_dump_to_buffer(ptr + i, linelen, HEX_ROW_SIZE, HEX_GROUP_SIZE,
+ linebuf, sizeof(linebuf), HEX_ASCII);
+
+ seq_printf(seq, "%s\n", linebuf);
+ }
+
+ seq_printf(seq, "\n");
+}
+
+
/*
* Printing of the unreferenced objects information to the seq file. The
* print_unreferenced function must be called with the object->lock held.
@@ -301,10 +334,17 @@ static void print_unreferenced(struct se

seq_printf(seq, "unreferenced object 0x%08lx (size %zu):\n",
object->pointer, object->size);
+
seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
object->comm, object->pid, object->jiffies);
- seq_printf(seq, " backtrace:\n");

+ /* check whether hex dump should be printed*/
+ if (atomic_read(&kmemleak_hex_dump))
+ object_hex_dump(seq, object);
+
+
+ seq_printf(seq, " backtrace:\n");
+
for (i = 0; i < object->trace_len; i++) {
void *ptr = (void *)object->trace[i];
seq_printf(seq, " [] %pS\n", ptr, ptr);
@@ -1269,6 +1309,12 @@ static ssize_t kmemleak_write(struct fil
start_scan_thread();
else if (strncmp(buf, "scan=off", 8) == 0)
stop_scan_thread();
+ else if (strncmp(buf, "hexdump=on", 10) == 0) {
+ atomic_set(&kmemleak_hex_dump, 1);
+ }
+ else if (strncmp(buf, "hexdump=off", 11) == 0) {
+ atomic_set(&kmemleak_hex_dump, 0);
+ }
else if (strncmp(buf, "scan=", 5) == 0) {
unsigned long secs;
int err;
Sergey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hi Sergey,
On Sun, Jun 28, 2009 at 8:36 PM, Sergey
Senozhatsky wrote:
> What do you think about ability to 'watch' leaked region? (hex + ascii).
> (done via lib/hexdump.c)
What's your use case for this? I'm usually more interested in the
stack trace when there's a memory leak.
Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Mon, 2009-06-29 at 12:43 +0300, Pekka Enberg wrote:
> On Sun, Jun 28, 2009 at 8:36 PM, Sergey
> Senozhatsky wrote:
> > What do you think about ability to 'watch' leaked region? (hex + ascii).
> > (done via lib/hexdump.c)
>
> What's your use case for this? I'm usually more interested in the
> stack trace when there's a memory leak.
I once had a need for such feature when investigating a memory leak (it
was more like debugging kmemleak) but a script combining dd, od
and /dev/kmem did the trick (I also work in an embedded world where I
have a halting debugger connected most of the times).
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On (06/29/09 12:43), Pekka Enberg wrote:
> Hi Sergey,
>
> On Sun, Jun 28, 2009 at 8:36 PM, Sergey
> Senozhatsky wrote:
> > What do you think about ability to 'watch' leaked region? (hex + ascii).
> > (done via lib/hexdump.c)
>
> What's your use case for this? I'm usually more interested in the
> stack trace when there's a memory leak.
>
> Pekka
>
Hello Pekka,
Well, it's not easy to come up with something strong.
I agree, that stack gives you almost all you need.
HEX dump can give you a _tip_ in case you're not sure.
for example:
unreferenced object 0xf6aac7f8 (size 32):
comm "swapper", pid 1, jiffies 4294877610
HEX dump:
70 6e 70 20 30 30 3a 30 61 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 pnp 00:0a.ZZZZZZZZZZZZZZZZZZZZZ.
backtrace:
[] kmemleak_alloc+0x11b/0x2b0
[] kmem_cache_alloc+0x111/0x1c0
[] reserve_range+0x3e/0x1b0
[] system_pnp_probe+0x94/0x140
[] pnp_device_probe+0x84/0x100
[] driver_probe_device+0x89/0x170
[] __driver_attach+0x99/0xa0
[] bus_for_each_dev+0x58/0x90
[] driver_attach+0x24/0x40
[] bus_add_driver+0xc4/0x290
[] driver_register+0x70/0x130
[] pnp_register_driver+0x26/0x40
[] pnp_system_init+0x1b/0x2e
[] do_one_initcall+0x3f/0x1a0
[] kernel_init+0x13e/0x1a6
[] kernel_thread_helper+0x7/0x10
- Ah, pnp 00:0a. Got it.
or
- Ah, pnp 00:0a. No.. It's false. (EXAMPLE)
Or something like that :-)
Sergey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hi Sergey,
On Mon, 2009-06-29 at 13:19 +0300, Sergey Senozhatsky wrote:
> Well, it's not easy to come up with something strong.
> I agree, that stack gives you almost all you need.
>
> HEX dump can give you a _tip_ in case you're not sure.
Don't get me wrong, I'm not against it in any way. If Catalin is
interested in merging this kind of functionality, go for it! You might
want to consider unconditionally enabling the hexdump. If the
information is valuable, we should print it all the time.
Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Mon, 2009-06-29 at 13:19 +0300, Pekka Enberg wrote:
> On Mon, 2009-06-29 at 13:19 +0300, Sergey Senozhatsky wrote:
> > Well, it's not easy to come up with something strong.
> > I agree, that stack gives you almost all you need.
> >
> > HEX dump can give you a _tip_ in case you're not sure.
>
> Don't get me wrong, I'm not against it in any way. If Catalin is
> interested in merging this kind of functionality, go for it! You might
> want to consider unconditionally enabling the hexdump. If the
> information is valuable, we should print it all the time.
Though I prefer to do as much as possible in user space, I think this
feature would be useful.
Anyway, I may not include it before the next merging window (when is
actually the best time for new features). Currently, my main focus is on
reducing the false positives.
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hi Catalin,
On Mon, 2009-06-29 at 11:38 +0100, Catalin Marinas wrote:
> Anyway, I may not include it before the next merging window (when is
> actually the best time for new features). Currently, my main focus is on
> reducing the false positives.
Yes, this new feature shouldn't go into 2.6.31. That said, you _do_ want
to include it in linux-next now if you're interested in pushing it to
2.6.32.
Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On (06/29/09 13:19), Pekka Enberg wrote:
> Hi Sergey,
>
> On Mon, 2009-06-29 at 13:19 +0300, Sergey Senozhatsky wrote:
> > Well, it's not easy to come up with something strong.
> > I agree, that stack gives you almost all you need.
> >
> > HEX dump can give you a _tip_ in case you're not sure.
>
> Don't get me wrong, I'm not against it in any way.
I don't think so ;) _IF_ even so - it's absolutely normal, to my mind.
> If Catalin is interested in merging this kind of
> functionality, go for it! You might want to consider unconditionally
> enabling the hexdump. If the information is valuable, we should print
> it all the time.
I guess it's valuable enougth to print it, but not valuable enougth to print it all the time.
So, let me say - it's valuable enougth to print 'on demand', I guess. (I may be wrong).
BTW, printing it all the time we can spam kmemleak (in case there are objects sized 2K, 4K and so on).
That's why I wrote about hexdump=OBJECT_POINTER.
>
> Pekka
>
Sergey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On (06/29/09 11:38), Catalin Marinas wrote:
> On Mon, 2009-06-29 at 13:19 +0300, Pekka Enberg wrote:
> > On Mon, 2009-06-29 at 13:19 +0300, Sergey Senozhatsky wrote:
> > > Well, it's not easy to come up with something strong.
> > > I agree, that stack gives you almost all you need.
> > >
> > > HEX dump can give you a _tip_ in case you're not sure.
> >
> > Don't get me wrong, I'm not against it in any way. If Catalin is
> > interested in merging this kind of functionality, go for it! You might
> > want to consider unconditionally enabling the hexdump. If the
> > information is valuable, we should print it all the time.
>
> Though I prefer to do as much as possible in user space,
Agreed. Good example is 'function filtering' ;)
> I think this feature would be useful.
>
So, I'll continue my work. (given patch didn't even passed ./checkpatch.pl).
Ok?
> Anyway, I may not include it before the next merging window (when is
> actually the best time for new features). Currently, my main focus is on
> reducing the false positives.
Sure. No problems.
>
> --
> Catalin
>
Sergey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Mon, 2009-06-29 at 13:45 +0300, Sergey Senozhatsky wrote:
> BTW, printing it all the time we can spam kmemleak (in case there are objects sized 2K, 4K and so on).
> That's why I wrote about hexdump=OBJECT_POINTER.
I'm more in favour of an on/off hexdump feature (maybe even permanently
on) and with a limit to the number of bytes it displays. For larger
blocks, the hexdump=OBJECT_POINTER is easily achievable in user space
via /dev/kmem.
My proposal is for an always on hexdump but with no more than 2-3 lines
of hex values. As Pekka said, I should get it into linux-next before the
next merging window.
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On (06/29/09 11:58), Catalin Marinas wrote:
> On Mon, 2009-06-29 at 13:45 +0300, Sergey Senozhatsky wrote:
> > BTW, printing it all the time we can spam kmemleak (in case there are objects sized 2K, 4K and so on).
> > That's why I wrote about hexdump=OBJECT_POINTER.
>
> I'm more in favour of an on/off hexdump feature (maybe even permanently
> on) and with a limit to the number of bytes it displays. For larger
> blocks, the hexdump=OBJECT_POINTER is easily achievable in user space
> via /dev/kmem.
>
Yeah. Good point.
> My proposal is for an always on hexdump but with no more than 2-3 lines
> of hex values.
I like it.
> As Pekka said, I should get it into linux-next before the
> next merging window.
I'll send new patch to you (today evening)/(tomorrow).
Ok?
>
> --
> Catalin
>
Sergey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hello.
This is actually draft. We'll discuss details during next merge window (or earlier).
Thanks.
---
hex dump prints not more than HEX_MAX_LINES lines by HEX_ROW_SIZE (16 or 32) bytes.
( min(object->size, HEX_MAX_LINES * HEX_ROW_SIZE) ).
Example (HEX_ROW_SIZE 16):
unreferenced object 0xf68b59b8 (size 32):
comm "swapper", pid 1, jiffies 4294877610
hex dump (first 32 bytes):
70 6e 70 20 30 30 3a 30 31 00 5a 5a 5a 5a 5a 5a pnp 00:01.ZZZZZZ
5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZZZZZZZZZZZZZ.
backtrace:
[] kmemleak_alloc+0x11b/0x2b0
[] kmem_cache_alloc+0x111/0x1c0
[] reserve_range+0x3e/0x1b0
[] system_pnp_probe+0x94/0x140
[] pnp_device_probe+0x84/0x100
[] driver_probe_device+0x89/0x170
[] __driver_attach+0x99/0xa0
[] bus_for_each_dev+0x58/0x90
[] driver_attach+0x24/0x40
[] bus_add_driver+0xc4/0x290
[] driver_register+0x70/0x130
[] pnp_register_driver+0x26/0x40
[] pnp_system_init+0x1b/0x2e
[] do_one_initcall+0x3f/0x1a0
[] kernel_init+0x13e/0x1a6
[] kernel_thread_helper+0x7/0x10
Example (HEX_ROW_SIZE 32):
unreferenced object 0xf5e2e130 (size 2048):
comm "modprobe", pid 2084, jiffies 4294880769
hex dump (first 64 bytes):
24 97 ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff $...............................
fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff ................................
backtrace:
[] kmemleak_alloc+0x11b/0x2b0
[] __kmalloc+0x16d/0x210
[] pcpu_mem_alloc+0x2d/0x80
[] pcpu_extend_area_map+0x62/0x100
[] pcpu_alloc+0x317/0x480
[] __alloc_percpu+0x17/0x30
[] alloc_disk_node+0x54/0x150
[] alloc_disk+0x1c/0x40
[] loop_alloc+0x6b/0x170 [loop]
[] 0xfd61e092
[] do_one_initcall+0x3f/0x1a0
[] sys_init_module+0xdd/0x220
[] sysenter_do_call+0x12/0x22
[] 0xffffffff
unreferenced object 0xf5d3cd00 (size 32):
comm "consolechars", pid 2433, jiffies 4294894598
hex dump (first 32 bytes):
60 15 58 c1 00 00 00 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 `.X.....ZZZZZZZZZZZZZZZZZZZZZZZ.
backtrace:
[] kmemleak_alloc+0x11b/0x2b0
[] kmem_cache_alloc+0x111/0x1c0
[] tty_ldisc_try_get+0x2b/0x130
[] tty_ldisc_get+0x37/0x70
[] tty_ldisc_reinit+0x34/0x70
[] tty_ldisc_release+0x35/0x60
[] tty_release_dev+0x33e/0x500
[] tty_release+0x20/0x40
[] __fput+0xed/0x200
[] fput+0x22/0x40
[] filp_close+0x49/0x90
[] put_files_struct+0xb5/0xe0
[] exit_files+0x45/0x60
[] do_exit+0x133/0x6c0
[] do_group_exit+0x45/0xa0
[] sys_exit_group+0x22/0x40
hexdump.c can print prefix:
case DUMP_PREFIX_ADDRESS:
f6998df0: 3c 06 00 00 00 00 00 00 78 69 00 00 .... pointer, object->size);
seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
object->comm, object->pid, object->jiffies);
+
+ /* check whether hex dump should be printed */
+ if (atomic_read(&kmemleak_hex_dump))
+ hex_dump_object(seq, object);
+
seq_printf(seq, " backtrace:\n");

for (i = 0; i < object->trace_len; i++) {
@@ -1269,6 +1314,10 @@ static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
start_scan_thread();
else if (strncmp(buf, "scan=off", 8) == 0)
stop_scan_thread();
+ else if (strncmp(buf, "hexdump=on", 10) == 0)
+ atomic_set(&kmemleak_hex_dump, 1);
+ else if (strncmp(buf, "hexdump=off", 11) == 0)
+ atomic_set(&kmemleak_hex_dump, 0);
else if (strncmp(buf, "scan=", 5) == 0) {
unsigned long secs;
int err;
Sergey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/