* [PATCH] mm: slub: print_hex_dump() with DUMP_PREFIX_OFFSET
@ 2019-09-20 10:48 Miles Chen
2019-09-21 9:08 ` David Rientjes
0 siblings, 1 reply; 6+ messages in thread
From: Miles Chen @ 2019-09-20 10:48 UTC (permalink / raw)
To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton
Cc: linux-kernel, linux-mm, linux-mediatek, wsd_upstream, Miles Chen
Since commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
The use DUMP_PREFIX_OFFSET instead of DUMP_PREFIX_ADDRESS with
print_hex_dump() can generate more useful messages.
In the following example, it's easier get the offset of incorrect poison
value with DUMP_PREFIX_OFFSET.
Before:
Object 00000000e817b73b: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 00000000816f4601: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 00000000169d2bb8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 00000000f4c38716: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 00000000917e3491: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 00000000c0e33738: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 000000001755ddd1: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
After:
Object 00000000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 00000010: 63 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 00000020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 00000030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5
I think it might be worth to convert all DUMP_PREFIX_ADDRESS to
DUMP_PREFIX_OFFSET for the whole Linux kernel.
$ git grep 'DUMP_PREFIX_ADDRESS' | cut -f1 -d"/" | sort | uniq -c
3 arch
140 drivers
12 fs
1 include
5 lib
2 mm
Signed-off-by: Miles Chen <miles.chen@mediatek.com>
---
mm/slub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/slub.c b/mm/slub.c
index 8834563cdb4b..89d91c1eb33d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -528,7 +528,7 @@ static void print_section(char *level, char *text, u8 *addr,
unsigned int length)
{
metadata_access_enable();
- print_hex_dump(level, text, DUMP_PREFIX_ADDRESS, 16, 1, addr,
+ print_hex_dump(level, text, DUMP_PREFIX_OFFSET, 16, 1, addr,
length, 1);
metadata_access_disable();
}
--
2.18.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: slub: print_hex_dump() with DUMP_PREFIX_OFFSET
2019-09-20 10:48 [PATCH] mm: slub: print_hex_dump() with DUMP_PREFIX_OFFSET Miles Chen
@ 2019-09-21 9:08 ` David Rientjes
2019-09-21 16:00 ` Matthew Wilcox
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Rientjes @ 2019-09-21 9:08 UTC (permalink / raw)
To: Miles Chen
Cc: Christoph Lameter, Pekka Enberg, Joonsoo Kim, Andrew Morton,
linux-kernel, linux-mm, linux-mediatek, wsd_upstream
On Fri, 20 Sep 2019, Miles Chen wrote:
> Since commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
> The use DUMP_PREFIX_OFFSET instead of DUMP_PREFIX_ADDRESS with
> print_hex_dump() can generate more useful messages.
>
> In the following example, it's easier get the offset of incorrect poison
> value with DUMP_PREFIX_OFFSET.
>
> Before:
> Object 00000000e817b73b: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 00000000816f4601: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 00000000169d2bb8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 00000000f4c38716: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 00000000917e3491: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 00000000c0e33738: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 000000001755ddd1: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
>
> After:
> Object 00000000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 00000010: 63 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 00000020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Object 00000030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5
>
> I think it might be worth to convert all DUMP_PREFIX_ADDRESS to
> DUMP_PREFIX_OFFSET for the whole Linux kernel.
>
I agree it looks nicer for poisoning, I'm not sure that every caller of
print_section() is the same, however. For example trace() seems better
off as DUMP_PREFIX_ADDRESS since it already specifies the address of the
object being allocated or freed and offset here wouldn't really be useful,
no?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: slub: print_hex_dump() with DUMP_PREFIX_OFFSET
2019-09-21 9:08 ` David Rientjes
@ 2019-09-21 16:00 ` Matthew Wilcox
2019-09-22 17:05 ` Miles Chen
2019-09-21 17:00 ` cl
2019-09-22 16:06 ` Miles Chen
2 siblings, 1 reply; 6+ messages in thread
From: Matthew Wilcox @ 2019-09-21 16:00 UTC (permalink / raw)
To: David Rientjes
Cc: Miles Chen, Christoph Lameter, Pekka Enberg, Joonsoo Kim,
Andrew Morton, linux-kernel, linux-mm, linux-mediatek,
wsd_upstream
On Sat, Sep 21, 2019 at 02:08:59AM -0700, David Rientjes wrote:
> On Fri, 20 Sep 2019, Miles Chen wrote:
>
> > Since commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
> > The use DUMP_PREFIX_OFFSET instead of DUMP_PREFIX_ADDRESS with
> > print_hex_dump() can generate more useful messages.
> >
> > In the following example, it's easier get the offset of incorrect poison
> > value with DUMP_PREFIX_OFFSET.
> >
> > Before:
> > Object 00000000e817b73b: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000816f4601: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000169d2bb8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000f4c38716: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000917e3491: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000c0e33738: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 000000001755ddd1: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> >
> > After:
> > Object 00000000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000010: 63 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5
>
> I agree it looks nicer for poisoning, I'm not sure that every caller of
> print_section() is the same, however. For example trace() seems better
> off as DUMP_PREFIX_ADDRESS since it already specifies the address of the
> object being allocated or freed and offset here wouldn't really be useful,
> no?
While it looks nicer, it might be less useful for debugging. The point of
obfuscated %p is that you can compare two "pointer" values for equality.
So if you know that you freed object 00000000e817b73b from an earlier
printk, then you can match it up to this dump. It's obviously not
perfect since we're only getting the pointers at addresses that are
multiples of 16, but it's a help.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: slub: print_hex_dump() with DUMP_PREFIX_OFFSET
2019-09-21 9:08 ` David Rientjes
2019-09-21 16:00 ` Matthew Wilcox
@ 2019-09-21 17:00 ` cl
2019-09-22 16:06 ` Miles Chen
2 siblings, 0 replies; 6+ messages in thread
From: cl @ 2019-09-21 17:00 UTC (permalink / raw)
To: David Rientjes
Cc: Miles Chen, Pekka Enberg, Joonsoo Kim, Andrew Morton,
linux-kernel, linux-mm, linux-mediatek, wsd_upstream
On Sat, 21 Sep 2019, David Rientjes wrote:
> I agree it looks nicer for poisoning, I'm not sure that every caller of
> print_section() is the same, however. For example trace() seems better
> off as DUMP_PREFIX_ADDRESS since it already specifies the address of the
> object being allocated or freed and offset here wouldn't really be useful,
> no?
The address is printed earlier before the object dump. Maybe that is
sufficient and we could even reduce the number of digits further to have
the display more compact? In this case two hex digits would do the trick.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: slub: print_hex_dump() with DUMP_PREFIX_OFFSET
2019-09-21 9:08 ` David Rientjes
2019-09-21 16:00 ` Matthew Wilcox
2019-09-21 17:00 ` cl
@ 2019-09-22 16:06 ` Miles Chen
2 siblings, 0 replies; 6+ messages in thread
From: Miles Chen @ 2019-09-22 16:06 UTC (permalink / raw)
To: David Rientjes
Cc: Christoph Lameter, Pekka Enberg, Joonsoo Kim, Andrew Morton,
linux-kernel, linux-mm, linux-mediatek, wsd_upstream
On Sat, 2019-09-21 at 02:08 -0700, David Rientjes wrote:
> On Fri, 20 Sep 2019, Miles Chen wrote:
>
> > Since commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
> > The use DUMP_PREFIX_OFFSET instead of DUMP_PREFIX_ADDRESS with
> > print_hex_dump() can generate more useful messages.
> >
> > In the following example, it's easier get the offset of incorrect poison
> > value with DUMP_PREFIX_OFFSET.
> >
> > Before:
> > Object 00000000e817b73b: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000816f4601: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000169d2bb8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000f4c38716: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000917e3491: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000000c0e33738: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 000000001755ddd1: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> >
> > After:
> > Object 00000000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000010: 63 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > Object 00000030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5
> >
> > I think it might be worth to convert all DUMP_PREFIX_ADDRESS to
> > DUMP_PREFIX_OFFSET for the whole Linux kernel.
> >
>
> I agree it looks nicer for poisoning, I'm not sure that every caller of
> print_section() is the same, however. For example trace() seems better
> off as DUMP_PREFIX_ADDRESS since it already specifies the address of the
> object being allocated or freed and offset here wouldn't really be useful,
> no?
Thanks for the reply. I agree not all caller of print_section() is the
same case. Converting all of them is not a good idea.
For mm/slub.c, let me explain with the following example.
The offset is useful for use-after-free debugging. I often trace the
"free track" and find the data structure used and compare the data
structure with the offset of the incorrect poison value and see if the
incorrect poison value matches some member of that data structure.
Assume we have a data structure "something" and it has a member called
"m", offset = 780.
struct something *p; // sizeof(struct something) is 1024
p = kmalloc(sizeof(struct something), GFP_KERNEL);
kfree(p);
(p->m) = 'c'; // assume p->member is at offset=780
When we see the log: (using DUMP_PREFIX_ADDRESS)
We have to "count" the offset.
INFO: Slab 0x(____ptrval____) objects=21 used=21 fp=0x(____ptrval____)
flags=0x10200
INFO: Object 0x(____ptrval____) @offset=7808 fp=0x(____ptrval____)
Redzone ____ptrval____: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone ____ptrval____: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone ____ptrval____: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone ____ptrval____: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone ____ptrval____: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone ____ptrval____: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone ____ptrval____: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone ____ptrval____: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
...
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 63 6b 6b 6b
kkkkkkkkkkkkckkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object ____ptrval____: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
The log of using DUMP_PREFIX_OFFSET:
We still have to "count", but it is easier.
INFO: Slab 0x(____ptrval____) objects=21 used=21 fp=0x(____ptrval____)
flags=0x10200
INFO: Object 0x(____ptrval____) @offset=7808 fp=0x(____ptrval____)
Redzone 00000000: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone 00000010: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone 00000020: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone 00000030: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone 00000040: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone 00000050: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone 00000060: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Redzone 00000070: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb
bb ................
Object 00000000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000040: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000050: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000060: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
...
Object 00000270: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000280: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000290: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 000002a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 000002b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 000002c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 000002d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 000002e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 000002f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000300: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 63 6b 6b 6b
kkkkkkkkkkkkckkk
Object 00000310: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000320: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000330: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000340: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000350: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000360: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000370: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
Object 00000380: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
kkkkkkkkkkkkkkkk
cheers,
Miles
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: slub: print_hex_dump() with DUMP_PREFIX_OFFSET
2019-09-21 16:00 ` Matthew Wilcox
@ 2019-09-22 17:05 ` Miles Chen
0 siblings, 0 replies; 6+ messages in thread
From: Miles Chen @ 2019-09-22 17:05 UTC (permalink / raw)
To: Matthew Wilcox
Cc: David Rientjes, Christoph Lameter, Pekka Enberg, Joonsoo Kim,
Andrew Morton, linux-kernel, linux-mm, linux-mediatek,
wsd_upstream
On Sat, 2019-09-21 at 09:00 -0700, Matthew Wilcox wrote:
> On Sat, Sep 21, 2019 at 02:08:59AM -0700, David Rientjes wrote:
> > On Fri, 20 Sep 2019, Miles Chen wrote:
> >
> > > Since commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
> > > The use DUMP_PREFIX_OFFSET instead of DUMP_PREFIX_ADDRESS with
> > > print_hex_dump() can generate more useful messages.
> > >
> > > In the following example, it's easier get the offset of incorrect poison
> > > value with DUMP_PREFIX_OFFSET.
> > >
> > > Before:
> > > Object 00000000e817b73b: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 00000000816f4601: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 00000000169d2bb8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 00000000f4c38716: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 00000000917e3491: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 00000000c0e33738: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 000000001755ddd1: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > >
> > > After:
> > > Object 00000000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 00000010: 63 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 00000020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> > > Object 00000030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5
> >
> > I agree it looks nicer for poisoning, I'm not sure that every caller of
> > print_section() is the same, however. For example trace() seems better
> > off as DUMP_PREFIX_ADDRESS since it already specifies the address of the
> > object being allocated or freed and offset here wouldn't really be useful,
> > no?
>
> While it looks nicer, it might be less useful for debugging. The point of
> obfuscated %p is that you can compare two "pointer" values for equality.
> So if you know that you freed object 00000000e817b73b from an earlier
> printk, then you can match it up to this dump. It's obviously not
> perfect since we're only getting the pointers at addresses that are
> multiples of 16, but it's a help.
Thanks for the reply.
The value of 00000000e817b73b dump and the value earlier printk should
be the same, Otherwise we have a serious problem because:
printf("%p", bad_ptr);
print_hex_dump(bad_ptr);
and we see a different hashed address of bad_ptr in print_hex_dump().
I think it's a rare case but we still have a chance to see that case.
DUMP_PREFIX_ADDRESS is useful for that case.
On the other hand, DUMP_PREFIX_OFFSET is useful for finding the offset
of incorrect poison value easier.
Maybe I can print the offset of the bad poison value in
check_bytes_and_report(). e.g.,
@@ -736,6 +736,7 @@ static int check_bytes_and_report(struct kmem_cache
*s, struct page *page,
{
u8 *fault;
u8 *end;
+ u8 *addr = page_address(page);
metadata_access_enable();
fault = memchr_inv(start, value, bytes);
@@ -748,8 +749,9 @@ static int check_bytes_and_report(struct kmem_cache
*s, struct page *page,
end--;
slab_bug(s, "%s overwritten", what);
- pr_err("INFO: 0x%p-0x%p. First byte 0x%x instead of 0x%x\n",
- fault, end - 1, fault[0],
value);
+ pr_err("INFO: 0x%p-0x%p. First byte 0x%x instead of 0x%x,
offset=%tu\n",
+ fault, end - 1, fault[0], value,
fault -
+ addr);
and we can see the offset printed out:
=============================================================================
BUG kmalloc-1k (Tainted: G B ): Poison overwritten
-----------------------------------------------------------------------------
INFO: 0x(____ptrval____)-0x(____ptrval____). First byte 0x63 instead of
0x6b, offset=7052
INFO: Object 0x(____ptrval____) @offset=6272 fp=0x(____ptrval____)
and we can get the offset by: 7052 - 6272 = 780.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-22 17:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-20 10:48 [PATCH] mm: slub: print_hex_dump() with DUMP_PREFIX_OFFSET Miles Chen
2019-09-21 9:08 ` David Rientjes
2019-09-21 16:00 ` Matthew Wilcox
2019-09-22 17:05 ` Miles Chen
2019-09-21 17:00 ` cl
2019-09-22 16:06 ` Miles Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).