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

[Thomas Gleixner <tglx@linutronix.de>]

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