[patch V3 01/32] mm/slab: Fix broken stack trace storage

[patch V3 01/32] mm/slab: Fix broken stack trace storage

[Thomas Gleixner]

kstack_end() is broken on interrupt stacks as they are not guaranteed to be
sized THREAD_SIZE and THREAD_SIZE aligned.

Use the stack tracer instead. Remove the pointless pointer increment at the
end of the function while at it.

Fixes: 98eb235b7feb ("[PATCH] page unmapping debug") - History tree
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: linux-mm@kvack.org
---
 mm/slab.c |   28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1470,33 +1470,29 @@ static bool is_debug_pagealloc_cache(str
 static void store_stackinfo(struct kmem_cache *cachep, unsigned long *addr,
 			    unsigned long caller)
 {
-	int size = cachep->object_size;
+	int size = cachep->object_size / sizeof(unsigned long);
 
 	addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
 
-	if (size < 5 * sizeof(unsigned long))
+	if (size < 5)
 		return;
 
 	*addr++ = 0x12345678;
 	*addr++ = caller;
 	*addr++ = smp_processor_id();
-	size -= 3 * sizeof(unsigned long);
+#ifdef CONFIG_STACKTRACE
 	{
-		unsigned long *sptr = &caller;
-		unsigned long svalue;
-
-		while (!kstack_end(sptr)) {
-			svalue = *sptr++;
-			if (kernel_text_address(svalue)) {
-				*addr++ = svalue;
-				size -= sizeof(unsigned long);
-				if (size <= sizeof(unsigned long))
-					break;
-			}
-		}
+		struct stack_trace trace = {
+			.max_entries	= size - 4;
+			.entries	= addr;
+			.skip		= 3;
+		};
 
+		save_stack_trace(&trace);
+		addr += trace.nr_entries;
 	}
-	*addr++ = 0x87654321;
+#endif
+	*addr = 0x87654321;
 }
 
 static void slab_kernel_map(struct kmem_cache *cachep, void *objp,

Re: [patch V3 01/32] mm/slab: Fix broken stack trace storage

[Andy Lutomirski]

On Sun, Apr 14, 2019 at 9:02 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> kstack_end() is broken on interrupt stacks as they are not guaranteed to be
> sized THREAD_SIZE and THREAD_SIZE aligned.
>
> Use the stack tracer instead. Remove the pointless pointer increment at the
> end of the function while at it.
>
> Fixes: 98eb235b7feb ("[PATCH] page unmapping debug") - History tree
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: linux-mm@kvack.org
> ---
>  mm/slab.c |   28 ++++++++++++----------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
>
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1470,33 +1470,29 @@ static bool is_debug_pagealloc_cache(str
>  static void store_stackinfo(struct kmem_cache *cachep, unsigned long *addr,
>                             unsigned long caller)
>  {
> -       int size = cachep->object_size;
> +       int size = cachep->object_size / sizeof(unsigned long);
>
>         addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
>
> -       if (size < 5 * sizeof(unsigned long))
> +       if (size < 5)
>                 return;
>
>         *addr++ = 0x12345678;
>         *addr++ = caller;
>         *addr++ = smp_processor_id();
> -       size -= 3 * sizeof(unsigned long);
> +#ifdef CONFIG_STACKTRACE
>         {
> -               unsigned long *sptr = &caller;
> -               unsigned long svalue;
> -
> -               while (!kstack_end(sptr)) {
> -                       svalue = *sptr++;
> -                       if (kernel_text_address(svalue)) {
> -                               *addr++ = svalue;
> -                               size -= sizeof(unsigned long);
> -                               if (size <= sizeof(unsigned long))
> -                                       break;
> -                       }
> -               }
> +               struct stack_trace trace = {
> +                       .max_entries    = size - 4;
> +                       .entries        = addr;
> +                       .skip           = 3;
> +               };

This looks correct, but I think that it would have been clearer if you
left the size -= 3 above.  You're still incrementing addr, but you're
not decrementing size, so they're out of sync and the resulting code
is hard to follow.

--Andy

Re: [patch V3 01/32] mm/slab: Fix broken stack trace storage

[Thomas Gleixner]

On Sun, 14 Apr 2019, Andy Lutomirski wrote:
> > +               struct stack_trace trace = {
> > +                       .max_entries    = size - 4;
> > +                       .entries        = addr;
> > +                       .skip           = 3;
> > +               };
> 
> This looks correct, but I think that it would have been clearer if you
> left the size -= 3 above.  You're still incrementing addr, but you're
> not decrementing size, so they're out of sync and the resulting code
> is hard to follow.

What about the below?

--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1480,10 +1480,12 @@ static void store_stackinfo(struct kmem_
 	*addr++ = 0x12345678;
 	*addr++ = caller;
 	*addr++ = smp_processor_id();
+	size -= 3;
 #ifdef CONFIG_STACKTRACE
 	{
 		struct stack_trace trace = {
-			.max_entries	= size - 4;
+			/* Leave one for the end marker below */
+			.max_entries	= size - 1;
 			.entries	= addr;
 			.skip		= 3;
 		};

[patch V4 01/32] mm/slab: Fix broken stack trace storage

[Thomas Gleixner]

kstack_end() is broken on interrupt stacks as they are not guaranteed to be
sized THREAD_SIZE and THREAD_SIZE aligned.

Use the stack tracer instead. Remove the pointless pointer increment at the
end of the function while at it.

Fixes: 98eb235b7feb ("[PATCH] page unmapping debug") - History tree
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: linux-mm@kvack.org
---
V4: Made the code simpler to understand (Andy) and make it actually compile
---
 mm/slab.c |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1470,33 +1470,31 @@ static bool is_debug_pagealloc_cache(str
 static void store_stackinfo(struct kmem_cache *cachep, unsigned long *addr,
 			    unsigned long caller)
 {
-	int size = cachep->object_size;
+	int size = cachep->object_size / sizeof(unsigned long);
 
 	addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
 
-	if (size < 5 * sizeof(unsigned long))
+	if (size < 5)
 		return;
 
 	*addr++ = 0x12345678;
 	*addr++ = caller;
 	*addr++ = smp_processor_id();
-	size -= 3 * sizeof(unsigned long);
+	size -= 3;
+#ifdef CONFIG_STACKTRACE
 	{
-		unsigned long *sptr = &caller;
-		unsigned long svalue;
-
-		while (!kstack_end(sptr)) {
-			svalue = *sptr++;
-			if (kernel_text_address(svalue)) {
-				*addr++ = svalue;
-				size -= sizeof(unsigned long);
-				if (size <= sizeof(unsigned long))
-					break;
-			}
-		}
+		struct stack_trace trace = {
+			/* Leave one for the end marker below */
+			.max_entries	= size - 1,
+			.entries	= addr,
+			.skip		= 3,
+		};
 
+		save_stack_trace(&trace);
+		addr += trace.nr_entries;
 	}
-	*addr++ = 0x87654321;
+#endif
+	*addr = 0x87654321;
 }
 
 static void slab_kernel_map(struct kmem_cache *cachep, void *objp,

Re: [patch V4 01/32] mm/slab: Fix broken stack trace storage

[Josh Poimboeuf]

On Mon, Apr 15, 2019 at 11:02:58AM +0200, Thomas Gleixner wrote:
> kstack_end() is broken on interrupt stacks as they are not guaranteed to be
> sized THREAD_SIZE and THREAD_SIZE aligned.
> 
> Use the stack tracer instead. Remove the pointless pointer increment at the
> end of the function while at it.
> 
> Fixes: 98eb235b7feb ("[PATCH] page unmapping debug") - History tree
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: linux-mm@kvack.org
> ---
> V4: Made the code simpler to understand (Andy) and make it actually compile
> ---
>  mm/slab.c |   30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1470,33 +1470,31 @@ static bool is_debug_pagealloc_cache(str
>  static void store_stackinfo(struct kmem_cache *cachep, unsigned long *addr,
>  			    unsigned long caller)
>  {
> -	int size = cachep->object_size;
> +	int size = cachep->object_size / sizeof(unsigned long);
>  
>  	addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
>  
> -	if (size < 5 * sizeof(unsigned long))
> +	if (size < 5)
>  		return;
>  
>  	*addr++ = 0x12345678;
>  	*addr++ = caller;
>  	*addr++ = smp_processor_id();
> -	size -= 3 * sizeof(unsigned long);
> +	size -= 3;
> +#ifdef CONFIG_STACKTRACE
>  	{
> -		unsigned long *sptr = &caller;
> -		unsigned long svalue;
> -
> -		while (!kstack_end(sptr)) {
> -			svalue = *sptr++;
> -			if (kernel_text_address(svalue)) {
> -				*addr++ = svalue;
> -				size -= sizeof(unsigned long);
> -				if (size <= sizeof(unsigned long))
> -					break;
> -			}
> -		}
> +		struct stack_trace trace = {
> +			/* Leave one for the end marker below */
> +			.max_entries	= size - 1,
> +			.entries	= addr,
> +			.skip		= 3,
> +		};
>  
> +		save_stack_trace(&trace);
> +		addr += trace.nr_entries;
>  	}
> -	*addr++ = 0x87654321;
> +#endif
> +	*addr = 0x87654321;

Looks like stack_trace.nr_entries isn't initialized?  (though this code
gets eventually replaced by a later patch)

Who actually reads this stack trace?  I couldn't find a consumer.

-- 
Josh

Re: [patch V4 01/32] mm/slab: Fix broken stack trace storage

[Thomas Gleixner]

On Mon, 15 Apr 2019, Josh Poimboeuf wrote:
> On Mon, Apr 15, 2019 at 11:02:58AM +0200, Thomas Gleixner wrote:
> >  	addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
> >  
> > -	if (size < 5 * sizeof(unsigned long))
> > +	if (size < 5)
> >  		return;
> >  
> >  	*addr++ = 0x12345678;
> >  	*addr++ = caller;
> >  	*addr++ = smp_processor_id();
> > -	size -= 3 * sizeof(unsigned long);
> > +	size -= 3;
> > +#ifdef CONFIG_STACKTRACE
> >  	{
> > -		unsigned long *sptr = &caller;
> > -		unsigned long svalue;
> > -
> > -		while (!kstack_end(sptr)) {
> > -			svalue = *sptr++;
> > -			if (kernel_text_address(svalue)) {
> > -				*addr++ = svalue;
> > -				size -= sizeof(unsigned long);
> > -				if (size <= sizeof(unsigned long))
> > -					break;
> > -			}
> > -		}
> > +		struct stack_trace trace = {
> > +			/* Leave one for the end marker below */
> > +			.max_entries	= size - 1,
> > +			.entries	= addr,
> > +			.skip		= 3,
> > +		};
> >  
> > +		save_stack_trace(&trace);
> > +		addr += trace.nr_entries;
> >  	}
> > -	*addr++ = 0x87654321;
> > +#endif
> > +	*addr = 0x87654321;
> 
> Looks like stack_trace.nr_entries isn't initialized?  (though this code
> gets eventually replaced by a later patch)

struct initializer initialized the non mentioned fields to 0, if I'm not
totally mistaken.

> Who actually reads this stack trace?  I couldn't find a consumer.

It's stored directly in the memory pointed to by @addr and that's the freed
cache memory. If that is used later (UAF) then the stack trace can be
printed to see where it was freed.

Thanks,

	tglx

Re: [patch V4 01/32] mm/slab: Fix broken stack trace storage

[Josh Poimboeuf]

On Mon, Apr 15, 2019 at 06:07:44PM +0200, Thomas Gleixner wrote:
> On Mon, 15 Apr 2019, Josh Poimboeuf wrote:
> > On Mon, Apr 15, 2019 at 11:02:58AM +0200, Thomas Gleixner wrote:
> > >  	addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
> > >  
> > > -	if (size < 5 * sizeof(unsigned long))
> > > +	if (size < 5)
> > >  		return;
> > >  
> > >  	*addr++ = 0x12345678;
> > >  	*addr++ = caller;
> > >  	*addr++ = smp_processor_id();
> > > -	size -= 3 * sizeof(unsigned long);
> > > +	size -= 3;
> > > +#ifdef CONFIG_STACKTRACE
> > >  	{
> > > -		unsigned long *sptr = &caller;
> > > -		unsigned long svalue;
> > > -
> > > -		while (!kstack_end(sptr)) {
> > > -			svalue = *sptr++;
> > > -			if (kernel_text_address(svalue)) {
> > > -				*addr++ = svalue;
> > > -				size -= sizeof(unsigned long);
> > > -				if (size <= sizeof(unsigned long))
> > > -					break;
> > > -			}
> > > -		}
> > > +		struct stack_trace trace = {
> > > +			/* Leave one for the end marker below */
> > > +			.max_entries	= size - 1,
> > > +			.entries	= addr,
> > > +			.skip		= 3,
> > > +		};
> > >  
> > > +		save_stack_trace(&trace);
> > > +		addr += trace.nr_entries;
> > >  	}
> > > -	*addr++ = 0x87654321;
> > > +#endif
> > > +	*addr = 0x87654321;
> > 
> > Looks like stack_trace.nr_entries isn't initialized?  (though this code
> > gets eventually replaced by a later patch)
> 
> struct initializer initialized the non mentioned fields to 0, if I'm not
> totally mistaken.

Hm, it seems you are correct.  And I thought I knew C.

> > Who actually reads this stack trace?  I couldn't find a consumer.
> 
> It's stored directly in the memory pointed to by @addr and that's the freed
> cache memory. If that is used later (UAF) then the stack trace can be
> printed to see where it was freed.

Right... but who reads it?

-- 
Josh

Re: [patch V4 01/32] mm/slab: Fix broken stack trace storage

[Peter Zijlstra]

On Mon, Apr 15, 2019 at 06:07:44PM +0200, Thomas Gleixner wrote:
> On Mon, 15 Apr 2019, Josh Poimboeuf wrote:
> > > +		struct stack_trace trace = {
> > > +			/* Leave one for the end marker below */
> > > +			.max_entries	= size - 1,
> > > +			.entries	= addr,
> > > +			.skip		= 3,
> > > +		};

> > Looks like stack_trace.nr_entries isn't initialized?  (though this code
> > gets eventually replaced by a later patch)
> 
> struct initializer initialized the non mentioned fields to 0, if I'm not
> totally mistaken.

Correct.

Re: [patch V3 01/32] mm/slab: Fix broken stack trace storage

[Andy Lutomirski]

On Sun, Apr 14, 2019 at 9:34 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Sun, 14 Apr 2019, Andy Lutomirski wrote:
> > > +               struct stack_trace trace = {
> > > +                       .max_entries    = size - 4;
> > > +                       .entries        = addr;
> > > +                       .skip           = 3;
> > > +               };
> >
> > This looks correct, but I think that it would have been clearer if you
> > left the size -= 3 above.  You're still incrementing addr, but you're
> > not decrementing size, so they're out of sync and the resulting code
> > is hard to follow.
>
> What about the below?
>
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1480,10 +1480,12 @@ static void store_stackinfo(struct kmem_
>         *addr++ = 0x12345678;
>         *addr++ = caller;
>         *addr++ = smp_processor_id();
> +       size -= 3;
>  #ifdef CONFIG_STACKTRACE
>         {
>                 struct stack_trace trace = {
> -                       .max_entries    = size - 4;
> +                       /* Leave one for the end marker below */
> +                       .max_entries    = size - 1;
>                         .entries        = addr;
>                         .skip           = 3;
>                 };

Looks good to me.

Re: [patch V4 01/32] mm/slab: Fix broken stack trace storage

[Andy Lutomirski]

On Mon, Apr 15, 2019 at 9:17 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Mon, Apr 15, 2019 at 06:07:44PM +0200, Thomas Gleixner wrote:
> > On Mon, 15 Apr 2019, Josh Poimboeuf wrote:
> > > On Mon, Apr 15, 2019 at 11:02:58AM +0200, Thomas Gleixner wrote:
> > > >   addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
> > > >
> > > > - if (size < 5 * sizeof(unsigned long))
> > > > + if (size < 5)
> > > >           return;
> > > >
> > > >   *addr++ = 0x12345678;
> > > >   *addr++ = caller;
> > > >   *addr++ = smp_processor_id();
> > > > - size -= 3 * sizeof(unsigned long);
> > > > + size -= 3;
> > > > +#ifdef CONFIG_STACKTRACE
> > > >   {
> > > > -         unsigned long *sptr = &caller;
> > > > -         unsigned long svalue;
> > > > -
> > > > -         while (!kstack_end(sptr)) {
> > > > -                 svalue = *sptr++;
> > > > -                 if (kernel_text_address(svalue)) {
> > > > -                         *addr++ = svalue;
> > > > -                         size -= sizeof(unsigned long);
> > > > -                         if (size <= sizeof(unsigned long))
> > > > -                                 break;
> > > > -                 }
> > > > -         }
> > > > +         struct stack_trace trace = {
> > > > +                 /* Leave one for the end marker below */
> > > > +                 .max_entries    = size - 1,
> > > > +                 .entries        = addr,
> > > > +                 .skip           = 3,
> > > > +         };
> > > >
> > > > +         save_stack_trace(&trace);
> > > > +         addr += trace.nr_entries;
> > > >   }
> > > > - *addr++ = 0x87654321;
> > > > +#endif
> > > > + *addr = 0x87654321;
> > >
> > > Looks like stack_trace.nr_entries isn't initialized?  (though this code
> > > gets eventually replaced by a later patch)
> >
> > struct initializer initialized the non mentioned fields to 0, if I'm not
> > totally mistaken.
>
> Hm, it seems you are correct.  And I thought I knew C.
>
> > > Who actually reads this stack trace?  I couldn't find a consumer.
> >
> > It's stored directly in the memory pointed to by @addr and that's the freed
> > cache memory. If that is used later (UAF) then the stack trace can be
> > printed to see where it was freed.
>
> Right... but who reads it?

That seems like a reasonable question.  After some grepping and some
git searching, it looks like there might not be any users.  I found
SLAB_STORE_USER, but that seems to be independent.

So maybe the whole mess should just be deleted.  If anyone ever
notices, they can re-add it better.

--Andy

Re: [patch V4 01/32] mm/slab: Fix broken stack trace storage

[Thomas Gleixner]

On Mon, 15 Apr 2019, Josh Poimboeuf wrote:
> On Mon, Apr 15, 2019 at 06:07:44PM +0200, Thomas Gleixner wrote:
> > > 
> > > Looks like stack_trace.nr_entries isn't initialized?  (though this code
> > > gets eventually replaced by a later patch)
> > 
> > struct initializer initialized the non mentioned fields to 0, if I'm not
> > totally mistaken.
> 
> Hm, it seems you are correct.  And I thought I knew C.

:)

> > > Who actually reads this stack trace?  I couldn't find a consumer.
> > 
> > It's stored directly in the memory pointed to by @addr and that's the freed
> > cache memory. If that is used later (UAF) then the stack trace can be
> > printed to see where it was freed.
> 
> Right... but who reads it?

Indeed. I didn't check but I know that I saw that info printed at least a
decade ago. Looks like that debug magic in slab.c has seen major changes
since then.

Thanks,

	tglx

Re: [patch V4 01/32] mm/slab: Fix broken stack trace storage

[Thomas Gleixner]

On Mon, 15 Apr 2019, Andy Lutomirski wrote:
> On Mon, Apr 15, 2019 at 9:17 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Mon, Apr 15, 2019 at 06:07:44PM +0200, Thomas Gleixner wrote:
> > > > Looks like stack_trace.nr_entries isn't initialized?  (though this code
> > > > gets eventually replaced by a later patch)
> > >
> > > struct initializer initialized the non mentioned fields to 0, if I'm not
> > > totally mistaken.
> >
> > Hm, it seems you are correct.  And I thought I knew C.
> >
> > > > Who actually reads this stack trace?  I couldn't find a consumer.
> > >
> > > It's stored directly in the memory pointed to by @addr and that's the freed
> > > cache memory. If that is used later (UAF) then the stack trace can be
> > > printed to see where it was freed.
> >
> > Right... but who reads it?
> 
> That seems like a reasonable question.  After some grepping and some
> git searching, it looks like there might not be any users.  I found

Anymore. There was something 10y+ ago...

> SLAB_STORE_USER, but that seems to be independent.
> 
> So maybe the whole mess should just be deleted.  If anyone ever
> notices, they can re-add it better.

No objections from my side, but the mm people might have opinions.

Thanks,

	tglx

Re: [patch V4 01/32] mm/slab: Fix broken stack trace storage

[Vlastimil Babka]

On 4/15/19 11:22 PM, Thomas Gleixner wrote:
> On Mon, 15 Apr 2019, Andy Lutomirski wrote:
>> On Mon, Apr 15, 2019 at 9:17 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>>> On Mon, Apr 15, 2019 at 06:07:44PM +0200, Thomas Gleixner wrote:
>>>>> Looks like stack_trace.nr_entries isn't initialized?  (though this code
>>>>> gets eventually replaced by a later patch)
>>>>
>>>> struct initializer initialized the non mentioned fields to 0, if I'm not
>>>> totally mistaken.
>>>
>>> Hm, it seems you are correct.  And I thought I knew C.
>>>
>>>>> Who actually reads this stack trace?  I couldn't find a consumer.
>>>>
>>>> It's stored directly in the memory pointed to by @addr and that's the freed
>>>> cache memory. If that is used later (UAF) then the stack trace can be
>>>> printed to see where it was freed.
>>>
>>> Right... but who reads it?
>>
>> That seems like a reasonable question.  After some grepping and some
>> git searching, it looks like there might not be any users.  I found
> 
> Anymore. There was something 10y+ ago.

In theory it can be useful in a crash dump. But I don't see any related
debugging
check that would trigger a panic, in order to get one.

>> SLAB_STORE_USER, but that seems to be independent.
>>
>> So maybe the whole mess should just be deleted.  If anyone ever
>> notices, they can re-add it better.
> 
> No objections from my side, but the mm people might have opinions.

Anyone who wants to debug wrong slab usage probably uses SLUB anyway, so
I don't think it's a problem to remove broken SLAB debugging. Perhaps
even SLAB itself will be removed soon if there's performance data
supporting it [1].

[1]
https://lore.kernel.org/linux-mm/20190412112816.GD18914@techsingularity.net/T/#u

> Thanks,
> 
> 	tglx
> 

[patch V5 01/32] mm/slab: Remove broken stack trace storage

[Thomas Gleixner]

kstack_end() is broken on interrupt stacks as they are not guaranteed to be
sized THREAD_SIZE and THREAD_SIZE aligned.

As SLAB seems not to be used much with debugging enabled and might just go
away completely according to:

  https://lkml.kernel.org/r/612f9b99-a75b-6aeb-cf92-7dc5421cd950@suse.cz

just remove the bogus code instead of trying to fix it.

Fixes: 98eb235b7feb ("[PATCH] page unmapping debug") - History tree
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: linux-mm@kvack.org
---
V5: Remove the cruft.
V4: Make it actually work
V2: Made the code simpler to understand (Andy)
---
 mm/slab.c |   22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1470,33 +1470,17 @@ static bool is_debug_pagealloc_cache(str
 static void store_stackinfo(struct kmem_cache *cachep, unsigned long *addr,
 			    unsigned long caller)
 {
-	int size = cachep->object_size;
+	int size = cachep->object_size / sizeof(unsigned long);
 
 	addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
 
-	if (size < 5 * sizeof(unsigned long))
+	if (size < 4)
 		return;
 
 	*addr++ = 0x12345678;
 	*addr++ = caller;
 	*addr++ = smp_processor_id();
-	size -= 3 * sizeof(unsigned long);
-	{
-		unsigned long *sptr = &caller;
-		unsigned long svalue;
-
-		while (!kstack_end(sptr)) {
-			svalue = *sptr++;
-			if (kernel_text_address(svalue)) {
-				*addr++ = svalue;
-				size -= sizeof(unsigned long);
-				if (size <= sizeof(unsigned long))
-					break;
-			}
-		}
-
-	}
-	*addr++ = 0x87654321;
+	*addr = 0x87654321;
 }
 
 static void slab_kernel_map(struct kmem_cache *cachep, void *objp,

Re: [patch V5 01/32] mm/slab: Remove broken stack trace storage

[Vlastimil Babka]

On 4/16/19 4:10 PM, Thomas Gleixner wrote:
> kstack_end() is broken on interrupt stacks as they are not guaranteed to be
> sized THREAD_SIZE and THREAD_SIZE aligned.
> 
> As SLAB seems not to be used much with debugging enabled and might just go
> away completely according to:
> 
>   https://lkml.kernel.org/r/612f9b99-a75b-6aeb-cf92-7dc5421cd950@suse.cz
> 
> just remove the bogus code instead of trying to fix it.
> 
> Fixes: 98eb235b7feb ("[PATCH] page unmapping debug") - History tree
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: linux-mm@kvack.org

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks.

> ---
> V5: Remove the cruft.
> V4: Make it actually work
> V2: Made the code simpler to understand (Andy)
> ---
>  mm/slab.c |   22 +++-------------------
>  1 file changed, 3 insertions(+), 19 deletions(-)
> 
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1470,33 +1470,17 @@ static bool is_debug_pagealloc_cache(str
>  static void store_stackinfo(struct kmem_cache *cachep, unsigned long *addr,
>  			    unsigned long caller)
>  {
> -	int size = cachep->object_size;
> +	int size = cachep->object_size / sizeof(unsigned long);
>  
>  	addr = (unsigned long *)&((char *)addr)[obj_offset(cachep)];
>  
> -	if (size < 5 * sizeof(unsigned long))
> +	if (size < 4)
>  		return;
>  
>  	*addr++ = 0x12345678;
>  	*addr++ = caller;
>  	*addr++ = smp_processor_id();
> -	size -= 3 * sizeof(unsigned long);
> -	{
> -		unsigned long *sptr = &caller;
> -		unsigned long svalue;
> -
> -		while (!kstack_end(sptr)) {
> -			svalue = *sptr++;
> -			if (kernel_text_address(svalue)) {
> -				*addr++ = svalue;
> -				size -= sizeof(unsigned long);
> -				if (size <= sizeof(unsigned long))
> -					break;
> -			}
> -		}
> -
> -	}
> -	*addr++ = 0x87654321;
> +	*addr = 0x87654321;
>  }
>  
>  static void slab_kernel_map(struct kmem_cache *cachep, void *objp,
>