linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/slab: Do not hash pointers when debugging slab
@ 2017-12-07 10:17 Geert Uytterhoeven
  2017-12-07 11:13 ` Christopher Lameter
  2017-12-10 20:45 ` Linus Torvalds
  0 siblings, 2 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2017-12-07 10:17 UTC (permalink / raw)
  To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton
  Cc: Tobin C . Harding, linux-mm, linux-kernel, Geert Uytterhoeven

If CONFIG_DEBUG_SLAB/CONFIG_DEBUG_SLAB_LEAK are enabled, the slab code
prints extra debug information when e.g. corruption is detected.
This includes pointers, which are not very useful when hashed.

Fix this by using %px to print unhashed pointers instead.

Fixes: ad67b74d2469d9b8 ("printk: hash addresses printed with %p")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
It's been ages I needed the above options.  But of course I need them
just after the introduction of address hashing...
---
 mm/slab.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 183e996dde5ff37a..70be5823227dcb3e 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1585,7 +1585,7 @@ static void print_objinfo(struct kmem_cache *cachep, void *objp, int lines)
 	}
 
 	if (cachep->flags & SLAB_STORE_USER) {
-		pr_err("Last user: [<%p>](%pSR)\n",
+		pr_err("Last user: [<%px>](%pSR)\n",
 		       *dbg_userword(cachep, objp),
 		       *dbg_userword(cachep, objp));
 	}
@@ -1621,7 +1621,7 @@ static void check_poison_obj(struct kmem_cache *cachep, void *objp)
 			/* Mismatch ! */
 			/* Print header */
 			if (lines == 0) {
-				pr_err("Slab corruption (%s): %s start=%p, len=%d\n",
+				pr_err("Slab corruption (%s): %s start=%px, len=%d\n",
 				       print_tainted(), cachep->name,
 				       realobj, size);
 				print_objinfo(cachep, objp, 0);
@@ -1650,13 +1650,13 @@ static void check_poison_obj(struct kmem_cache *cachep, void *objp)
 		if (objnr) {
 			objp = index_to_obj(cachep, page, objnr - 1);
 			realobj = (char *)objp + obj_offset(cachep);
-			pr_err("Prev obj: start=%p, len=%d\n", realobj, size);
+			pr_err("Prev obj: start=%px, len=%d\n", realobj, size);
 			print_objinfo(cachep, objp, 2);
 		}
 		if (objnr + 1 < cachep->num) {
 			objp = index_to_obj(cachep, page, objnr + 1);
 			realobj = (char *)objp + obj_offset(cachep);
-			pr_err("Next obj: start=%p, len=%d\n", realobj, size);
+			pr_err("Next obj: start=%px, len=%d\n", realobj, size);
 			print_objinfo(cachep, objp, 2);
 		}
 	}
@@ -2608,7 +2608,7 @@ static void slab_put_obj(struct kmem_cache *cachep,
 	/* Verify double free bug */
 	for (i = page->active; i < cachep->num; i++) {
 		if (get_free_obj(page, i) == objnr) {
-			pr_err("slab: double free detected in cache '%s', objp %p\n",
+			pr_err("slab: double free detected in cache '%s', objp %px\n",
 			       cachep->name, objp);
 			BUG();
 		}
@@ -2772,7 +2772,7 @@ static inline void verify_redzone_free(struct kmem_cache *cache, void *obj)
 	else
 		slab_error(cache, "memory outside object was overwritten");
 
-	pr_err("%p: redzone 1:0x%llx, redzone 2:0x%llx\n",
+	pr_err("%px: redzone 1:0x%llx, redzone 2:0x%llx\n",
 	       obj, redzone1, redzone2);
 }
 
@@ -3078,7 +3078,7 @@ static void *cache_alloc_debugcheck_after(struct kmem_cache *cachep,
 		if (*dbg_redzone1(cachep, objp) != RED_INACTIVE ||
 				*dbg_redzone2(cachep, objp) != RED_INACTIVE) {
 			slab_error(cachep, "double free, or memory outside object was overwritten");
-			pr_err("%p: redzone 1:0x%llx, redzone 2:0x%llx\n",
+			pr_err("%px: redzone 1:0x%llx, redzone 2:0x%llx\n",
 			       objp, *dbg_redzone1(cachep, objp),
 			       *dbg_redzone2(cachep, objp));
 		}
@@ -3091,7 +3091,7 @@ static void *cache_alloc_debugcheck_after(struct kmem_cache *cachep,
 		cachep->ctor(objp);
 	if (ARCH_SLAB_MINALIGN &&
 	    ((unsigned long)objp & (ARCH_SLAB_MINALIGN-1))) {
-		pr_err("0x%p: not aligned to ARCH_SLAB_MINALIGN=%d\n",
+		pr_err("0x%px: not aligned to ARCH_SLAB_MINALIGN=%d\n",
 		       objp, (int)ARCH_SLAB_MINALIGN);
 	}
 	return objp;
@@ -4283,7 +4283,7 @@ static void show_symbol(struct seq_file *m, unsigned long address)
 		return;
 	}
 #endif
-	seq_printf(m, "%p", (void *)address);
+	seq_printf(m, "%px", (void *)address);
 }
 
 static int leaks_show(struct seq_file *m, void *p)
-- 
2.7.4

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

* Re: [PATCH] mm/slab: Do not hash pointers when debugging slab
  2017-12-07 10:17 [PATCH] mm/slab: Do not hash pointers when debugging slab Geert Uytterhoeven
@ 2017-12-07 11:13 ` Christopher Lameter
  2017-12-07 18:43   ` Kees Cook
  2017-12-10 20:45 ` Linus Torvalds
  1 sibling, 1 reply; 7+ messages in thread
From: Christopher Lameter @ 2017-12-07 11:13 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton,
	Tobin C . Harding, linux-mm, linux-kernel, Kees Cook

On Thu, 7 Dec 2017, Geert Uytterhoeven wrote:

> If CONFIG_DEBUG_SLAB/CONFIG_DEBUG_SLAB_LEAK are enabled, the slab code
> prints extra debug information when e.g. corruption is detected.
> This includes pointers, which are not very useful when hashed.
>
> Fix this by using %px to print unhashed pointers instead.

Acked-by: Christoph Lameter <cl@linux.com>

These SLAB config options are only used for testing so this is ok.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm/slab: Do not hash pointers when debugging slab
  2017-12-07 11:13 ` Christopher Lameter
@ 2017-12-07 18:43   ` Kees Cook
  2017-12-07 20:15     ` Christopher Lameter
  0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2017-12-07 18:43 UTC (permalink / raw)
  To: Christopher Lameter
  Cc: Geert Uytterhoeven, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Tobin C . Harding, Linux-MM, LKML, Linus Torvalds

On Thu, Dec 7, 2017 at 3:13 AM, Christopher Lameter <cl@linux.com> wrote:
> On Thu, 7 Dec 2017, Geert Uytterhoeven wrote:
>
>> If CONFIG_DEBUG_SLAB/CONFIG_DEBUG_SLAB_LEAK are enabled, the slab code
>> prints extra debug information when e.g. corruption is detected.
>> This includes pointers, which are not very useful when hashed.
>>
>> Fix this by using %px to print unhashed pointers instead.
>
> Acked-by: Christoph Lameter <cl@linux.com>
>
> These SLAB config options are only used for testing so this is ok.

Most systems use SLUB so I can't say how common CONFIG_DEBUG_SLAB is.
(Though, FWIW with SLUB, CONFIG_SLUB_DEBUG is very common.)

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm/slab: Do not hash pointers when debugging slab
  2017-12-07 18:43   ` Kees Cook
@ 2017-12-07 20:15     ` Christopher Lameter
  0 siblings, 0 replies; 7+ messages in thread
From: Christopher Lameter @ 2017-12-07 20:15 UTC (permalink / raw)
  To: Kees Cook
  Cc: Geert Uytterhoeven, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Tobin C . Harding, Linux-MM, LKML, Linus Torvalds

On Thu, 7 Dec 2017, Kees Cook wrote:

> > These SLAB config options are only used for testing so this is ok.
>
> Most systems use SLUB so I can't say how common CONFIG_DEBUG_SLAB is.
> (Though, FWIW with SLUB, CONFIG_SLUB_DEBUG is very common.)

CONFIG_SLUB_DEBUG is on by default because it compiles into the kernel the
runtime configurable debugging framework. It does not activate any
debugging.

CONFIG_SLUB_DEBUG_ON is the equivalent to CONFIG_SLAB_DEBUG. The kernel
will boot with debugging on without any extra kernel options with these.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm/slab: Do not hash pointers when debugging slab
  2017-12-07 10:17 [PATCH] mm/slab: Do not hash pointers when debugging slab Geert Uytterhoeven
  2017-12-07 11:13 ` Christopher Lameter
@ 2017-12-10 20:45 ` Linus Torvalds
  2017-12-11  8:08   ` Geert Uytterhoeven
  2017-12-11 12:00   ` Christopher Lameter
  1 sibling, 2 replies; 7+ messages in thread
From: Linus Torvalds @ 2017-12-10 20:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Tobin C . Harding, linux-mm,
	Linux Kernel Mailing List

On Thu, Dec 7, 2017 at 2:17 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
>         if (cachep->flags & SLAB_STORE_USER) {
> -               pr_err("Last user: [<%p>](%pSR)\n",
> +               pr_err("Last user: [<%px>](%pSR)\n",
>                        *dbg_userword(cachep, objp),
>                        *dbg_userword(cachep, objp));

Is there actually any point to the %px at all?

Why not remove it? the _real_ information is printed out by %pSR, and
that's both sufficient and useful in ways %px isn't.

> -                               pr_err("Slab corruption (%s): %s start=%p, len=%d\n",
> +                               pr_err("Slab corruption (%s): %s start=%px, len=%d\n",
>                                        print_tainted(), cachep->name,
>                                        realobj, size);

and here, is the pointer actually interesting, or should we just give
the offset to the allocation?

But if the pointer is interesting, then ack.

              Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm/slab: Do not hash pointers when debugging slab
  2017-12-10 20:45 ` Linus Torvalds
@ 2017-12-11  8:08   ` Geert Uytterhoeven
  2017-12-11 12:00   ` Christopher Lameter
  1 sibling, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2017-12-11  8:08 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Geert Uytterhoeven, Christoph Lameter, Pekka Enberg,
	David Rientjes, Joonsoo Kim, Andrew Morton, Tobin C . Harding,
	linux-mm, Linux Kernel Mailing List

Hi Linus,

On Sun, Dec 10, 2017 at 9:45 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Dec 7, 2017 at 2:17 AM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
>> -                               pr_err("Slab corruption (%s): %s start=%p, len=%d\n",
>> +                               pr_err("Slab corruption (%s): %s start=%px, len=%d\n",
>>                                        print_tainted(), cachep->name,
>>                                        realobj, size);
>
> and here, is the pointer actually interesting, or should we just give
> the offset to the allocation?

The pointer may help to identify e.g. an empty list_head in the written data.

> But if the pointer is interesting, then ack.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm/slab: Do not hash pointers when debugging slab
  2017-12-10 20:45 ` Linus Torvalds
  2017-12-11  8:08   ` Geert Uytterhoeven
@ 2017-12-11 12:00   ` Christopher Lameter
  1 sibling, 0 replies; 7+ messages in thread
From: Christopher Lameter @ 2017-12-11 12:00 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Geert Uytterhoeven, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Tobin C . Harding, linux-mm,
	Linux Kernel Mailing List

On Sun, 10 Dec 2017, Linus Torvalds wrote:

> On Thu, Dec 7, 2017 at 2:17 AM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> >
> >         if (cachep->flags & SLAB_STORE_USER) {
> > -               pr_err("Last user: [<%p>](%pSR)\n",
> > +               pr_err("Last user: [<%px>](%pSR)\n",
> >                        *dbg_userword(cachep, objp),
> >                        *dbg_userword(cachep, objp));
>
> Is there actually any point to the %px at all?
>
> Why not remove it? the _real_ information is printed out by %pSR, and
> that's both sufficient and useful in ways %px isn't.

This pointer refers to code so we can remove it.

>
> > -                               pr_err("Slab corruption (%s): %s start=%p, len=%d\n",
> > +                               pr_err("Slab corruption (%s): %s start=%px, len=%d\n",
> >                                        print_tainted(), cachep->name,
> >                                        realobj, size);
>
> and here, is the pointer actually interesting, or should we just give
> the offset to the allocation?
>
> But if the pointer is interesting, then ack.

The pointer here is to an slab object which could be important if one
wants to find the pointer value  in a hexdump of another object (f.e.
listhead) or other pointer information that is being inspected
in a debugging session.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

end of thread, other threads:[~2017-12-11 12:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-07 10:17 [PATCH] mm/slab: Do not hash pointers when debugging slab Geert Uytterhoeven
2017-12-07 11:13 ` Christopher Lameter
2017-12-07 18:43   ` Kees Cook
2017-12-07 20:15     ` Christopher Lameter
2017-12-10 20:45 ` Linus Torvalds
2017-12-11  8:08   ` Geert Uytterhoeven
2017-12-11 12:00   ` Christopher Lameter

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).