From: Maninder Singh <maninder1.s@samsung.com> To: Mark Rutland <mark.rutland@arm.com> Cc: "catalin.marinas@arm.com" <catalin.marinas@arm.com>, "will@kernel.org" <will@kernel.org>, "pcc@google.com" <pcc@google.com>, "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "0x7f454c46@gmail.com" <0x7f454c46@gmail.com>, "amit.kachhap@arm.com" <amit.kachhap@arm.com>, "james.morse@arm.com" <james.morse@arm.com>, "vincenzo.frascino@arm.com" <vincenzo.frascino@arm.com>, "nsaenzjulienne@suse.de" <nsaenzjulienne@suse.de>, "keescook@chromium.org" <keescook@chromium.org>, "samitolvanen@google.com" <samitolvanen@google.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, AMIT SAHRAWAT <a.sahrawat@samsung.com>, Vaneet Narang <v.narang@samsung.com> Subject: RE: [PATCH 2/2] arm64: print alloc free paths for address in registers Date: Wed, 24 Mar 2021 20:10:31 +0530 [thread overview] Message-ID: <20210324144031epcms5p30f848873ba3a4e20f4eb4e5d13d381f7@epcms5p3> (raw) In-Reply-To: <20210324115113.GA19135@C02TD0UTHF1T.local> 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
WARNING: multiple messages have this Message-ID (diff)
From: Maninder Singh <maninder1.s@samsung.com> To: Mark Rutland <mark.rutland@arm.com> Cc: "catalin.marinas@arm.com" <catalin.marinas@arm.com>, "will@kernel.org" <will@kernel.org>, "pcc@google.com" <pcc@google.com>, "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "0x7f454c46@gmail.com" <0x7f454c46@gmail.com>, "amit.kachhap@arm.com" <amit.kachhap@arm.com>, "james.morse@arm.com" <james.morse@arm.com>, "vincenzo.frascino@arm.com" <vincenzo.frascino@arm.com>, "nsaenzjulienne@suse.de" <nsaenzjulienne@suse.de>, "keescook@chromium.org" <keescook@chromium.org>, "samitolvanen@google.com" <samitolvanen@google.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, AMIT SAHRAWAT <a.sahrawat@samsung.com>, Vaneet Narang <v.narang@samsung.com> Subject: RE: [PATCH 2/2] arm64: print alloc free paths for address in registers Date: Wed, 24 Mar 2021 20:10:31 +0530 [thread overview] Message-ID: <20210324144031epcms5p30f848873ba3a4e20f4eb4e5d13d381f7@epcms5p3> (raw) In-Reply-To: <20210324115113.GA19135@C02TD0UTHF1T.local> 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-03-24 16:12 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20210324065511epcas5p1cb74119660e2d98a381ae69d01b29275@epcas5p1.samsung.com> 2021-03-24 6:54 ` [PATCH 1/2] arm64/process.c: fix Wmissing-prototypes build warnings Maninder Singh 2021-03-24 6:54 ` Maninder Singh [not found] ` <CGME20210324065516epcas5p450958011b69f5941e8f2bc993b82b904@epcas5p4.samsung.com> 2021-03-24 6:54 ` [PATCH 2/2] arm64: print alloc free paths for address in registers Maninder Singh 2021-03-24 6:54 ` Maninder Singh 2021-03-24 11:51 ` Mark Rutland 2021-03-24 11:51 ` Mark Rutland [not found] ` <CGME20210324065516epcas5p450958011b69f5941e8f2bc993b82b904@epcms5p3> 2021-03-24 14:40 ` Maninder Singh [this message] 2021-03-24 14:40 ` Maninder Singh 2021-03-25 11:19 ` (subset) [PATCH 1/2] arm64/process.c: fix Wmissing-prototypes build warnings Will Deacon 2021-03-25 11:19 ` Will Deacon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210324144031epcms5p30f848873ba3a4e20f4eb4e5d13d381f7@epcms5p3 \ --to=maninder1.s@samsung.com \ --cc=0x7f454c46@gmail.com \ --cc=a.sahrawat@samsung.com \ --cc=akpm@linux-foundation.org \ --cc=amit.kachhap@arm.com \ --cc=catalin.marinas@arm.com \ --cc=james.morse@arm.com \ --cc=keescook@chromium.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=nsaenzjulienne@suse.de \ --cc=pcc@google.com \ --cc=samitolvanen@google.com \ --cc=v.narang@samsung.com \ --cc=vincenzo.frascino@arm.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.