RE: [PATCH 2/2] arm64: print alloc free paths for address in registers

Maninder Singh
2021-03-24T16:13:33Z

Hi,

>
>This path is used for a number of failures that might have nothing to do
>with a use-after-free, and from the trimmed example below it looks like
>this could significantly bloat the panic and potentially cause important
>information to be lost from the log, especially given the large number
>of GPRs arm64 has.
>
>Given that, I suspect this is not something we want enabled by default.
>

Yes it will add a lot of logs in case of normal die also.
But it can suggest free and alloc paths which can help in some debugging.

>When is this logging enabled? I assume the kernel doesn't always record
>the alloc/free paths. Is there a boot-time option to control this?
>

if SLUB_DEBUG_ON is enabled at build time then it is enabled from boot,
otherwise it can be enabled by kernel command line parameter of "slub_debug=u"
if SLUB_DEBUG is enabled.

>How many lines does this produce on average?
>

16 traces at max for alloc and 16 for free path.
so in total for slab object 34 lines will be printed,
otherwise one line for each object info of vmalloc.

>>
>> +void __show_regs_alloc_free(struct pt_regs *regs)
>> +{
>> + int i;
>> +
>> + /* check for x0 - x29 only */
>
>Why x29? The AAPCS says that's the frame pointer, so much like the SP it
>shouldn't point to a heap object.

yes x29 can be ignored.

>
>> + for (i = 0; i <= 29; i++) {
>> + pr_alert("Register x%d information:", i);
>> + mem_dump_obj((void *)regs->regs[i]);
>> + }
>> +}
>
>The pr_alert() is unconditional -- can mem_dumpo_obj() never be
>disabled?
>

mem_dump_obj is dependent on CONFIG_PRINTK

>What loglevel does mem_dump_obj() use? Generally we try to keep that
>matched, so I'm surprised it isn't taken as a parameter.
>

loglevel of mem_dump_obj is pr_cont, so it will be with the caller's loglevel

/**
* mem_dump_obj - Print available provenance information
* @object: object for which to find provenance information.
*
* This function uses pr_cont(), so that the caller is expected to have
* printed out whatever preamble is appropriate.

But loglvel will be changed to pr_info when it goes for printing of traces.

void kmem_dump_obj(void *object)
{
....
pr_cont("\n");
for (i = 0; i < ARRAY_SIZE(kp.kp_stack); i++) {
if (!kp.kp_stack[i])
break;
pr_info(" %pS\n", kp.kp_stack[i]);
}

...

>> +
>> void show_regs(struct pt_regs *regs)
>> {
>> __show_regs(regs);
>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>> index a05d34f0e82a..cb4858c6e57b 100644
>> --- a/arch/arm64/kernel/traps.c
>> +++ b/arch/arm64/kernel/traps.c
>> @@ -104,6 +104,7 @@ static int __die(const char *str, int err, struct pt_regs *regs)
>>
>> print_modules();
>> show_regs(regs);
>> + __show_regs_alloc_free(regs);
>
>As above, I'm not sure this is the right place to put this. We can get
>here for reasons other than UAF, and I'm sure we can trigger panics via
>UAF without going via this.
>

Adding call here, because we though in case of use after free __die will be called.
due to unhandled page fault of 0x6b6b6 MAGIC value. thats why picked this place.

Thanks,
Maninder Singh