| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <PH0PR21MB302584B93A9F44AF5BA016EAD7FC9@PH0PR21MB3025.namprd21.prod.outlook.com>
Date: Fri, 29 Apr 2022 16:27:42 +0000
From: "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To: "Guilherme G. Piccoli" <gpiccoli@...lia.com>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"bhe@...hat.com" <bhe@...hat.com>,
"pmladek@...e.com" <pmladek@...e.com>,
"kexec@...ts.infradead.org" <kexec@...ts.infradead.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"bcm-kernel-feedback-list@...adcom.com"
<bcm-kernel-feedback-list@...adcom.com>,
"coresight@...ts.linaro.org" <coresight@...ts.linaro.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"linux-alpha@...r.kernel.org" <linux-alpha@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
"linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
"linux-parisc@...r.kernel.org" <linux-parisc@...r.kernel.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
"linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"linux-um@...ts.infradead.org" <linux-um@...ts.infradead.org>,
"linux-xtensa@...ux-xtensa.org" <linux-xtensa@...ux-xtensa.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"openipmi-developer@...ts.sourceforge.net"
<openipmi-developer@...ts.sourceforge.net>,
"rcu@...r.kernel.org" <rcu@...r.kernel.org>,
"sparclinux@...r.kernel.org" <sparclinux@...r.kernel.org>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
"x86@...nel.org" <x86@...nel.org>,
"kernel-dev@...lia.com" <kernel-dev@...lia.com>,
"kernel@...ccoli.net" <kernel@...ccoli.net>,
"halves@...onical.com" <halves@...onical.com>,
"fabiomirmar@...il.com" <fabiomirmar@...il.com>,
"alejandro.j.jimenez@...cle.com" <alejandro.j.jimenez@...cle.com>,
"andriy.shevchenko@...ux.intel.com"
<andriy.shevchenko@...ux.intel.com>,
"arnd@...db.de" <arnd@...db.de>, "bp@...en8.de" <bp@...en8.de>,
"corbet@....net" <corbet@....net>,
"d.hatayama@...fujitsu.com" <d.hatayama@...fujitsu.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"dyoung@...hat.com" <dyoung@...hat.com>,
"feng.tang@...el.com" <feng.tang@...el.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"hidehiro.kawai.ez@...achi.com" <hidehiro.kawai.ez@...achi.com>,
"jgross@...e.com" <jgross@...e.com>,
"john.ogness@...utronix.de" <john.ogness@...utronix.de>,
"keescook@...omium.org" <keescook@...omium.org>,
"luto@...nel.org" <luto@...nel.org>,
"mhiramat@...nel.org" <mhiramat@...nel.org>,
"mingo@...hat.com" <mingo@...hat.com>,
"paulmck@...nel.org" <paulmck@...nel.org>,
"peterz@...radead.org" <peterz@...radead.org>,
"rostedt@...dmis.org" <rostedt@...dmis.org>,
"senozhatsky@...omium.org" <senozhatsky@...omium.org>,
"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"vgoyal@...hat.com" <vgoyal@...hat.com>,
vkuznets <vkuznets@...hat.com>,
"will@...nel.org" <will@...nel.org>,
Arjan van de Ven <arjan@...ux.intel.com>,
Cong Wang <xiyou.wangcong@...il.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Valentin Schneider <valentin.schneider@....com>,
Xiaoming Ni <nixiaoming@...wei.com>
Subject: RE: [PATCH 18/30] notifier: Show function names on notifier routines
if DEBUG_NOTIFIERS is set
From: Guilherme G. Piccoli <gpiccoli@...lia.com> Sent: Wednesday, April 27, 2022 3:49 PM
>
> Currently we have a debug infrastructure in the notifiers file, but
> it's very simple/limited. This patch extends it by:
>
> (a) Showing all registered/unregistered notifiers' callback names;
>
> (b) Adding a dynamic debug tuning to allow showing called notifiers'
> function names. Notice that this should be guarded as a tunable since
> it can flood the kernel log buffer.
>
> Cc: Arjan van de Ven <arjan@...ux.intel.com>
> Cc: Cong Wang <xiyou.wangcong@...il.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> Cc: Valentin Schneider <valentin.schneider@....com>
> Cc: Xiaoming Ni <nixiaoming@...wei.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@...lia.com>
> ---
>
> We have some design decisions that worth discussing here:
>
> (a) First of call, using C99 helps a lot to write clear and concise code, but
s/call/all/
> due to commit 4d94f910e79a ("Kbuild: use -Wdeclaration-after-statement") we
> have a warning if mixing variable declarations with code. For this patch though,
> doing that makes the code way clear, so decision was to add the debug code
> inside brackets whenever this warning pops up. We can change that, but that'll
> cause more ifdefs in the same function.
>
> (b) In the symbol lookup helper function, we modify the parameter passed but
> even more, we return it as well! This is unusual and seems unnecessary, but was
> the strategy taken to allow embedding such function in the pr_debug() call.
>
> Not doing that would likely requiring 3 symbol_name variables to avoid
> concurrency (registering notifier A while calling notifier B) - we rely in
> local variables as a serialization mechanism.
>
> We're open for suggestions in case this design is not appropriate;
> thanks in advance!
>
> kernel/notifier.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/notifier.c b/kernel/notifier.c
> index ba005ebf4730..21032ebcde57 100644
> --- a/kernel/notifier.c
> +++ b/kernel/notifier.c
> @@ -7,6 +7,22 @@
> #include <linux/vmalloc.h>
> #include <linux/reboot.h>
>
> +#ifdef CONFIG_DEBUG_NOTIFIERS
> +#include <linux/kallsyms.h>
> +
> +/*
> + * Helper to get symbol names in case DEBUG_NOTIFIERS is set.
> + * Return the modified parameter is a strategy used to achieve
> + * the pr_debug() functionality - with this, function is only
> + * executed if the dynamic debug tuning is effectively set.
> + */
> +static inline char *notifier_name(struct notifier_block *nb, char *sym_name)
> +{
> + lookup_symbol_name((unsigned long)(nb->notifier_call), sym_name);
> + return sym_name;
> +}
> +#endif
> +
> /*
> * Notifier list for kernel code which wants to be called
> * at shutdown. This is used to stop any idling DMA operations
> @@ -34,20 +50,41 @@ static int notifier_chain_register(struct notifier_block **nl,
> }
> n->next = *nl;
> rcu_assign_pointer(*nl, n);
> +
> +#ifdef CONFIG_DEBUG_NOTIFIERS
> + {
> + char sym_name[KSYM_NAME_LEN];
> +
> + pr_info("notifiers: registered %s()\n",
> + notifier_name(n, sym_name));
> + }
> +#endif
> return 0;
> }
>
> static int notifier_chain_unregister(struct notifier_block **nl,
> struct notifier_block *n)
> {
> + int ret = -ENOENT;
> +
> while ((*nl) != NULL) {
> if ((*nl) == n) {
> rcu_assign_pointer(*nl, n->next);
> - return 0;
> + ret = 0;
> + break;
> }
> nl = &((*nl)->next);
> }
> - return -ENOENT;
> +
> +#ifdef CONFIG_DEBUG_NOTIFIERS
> + if (!ret) {
> + char sym_name[KSYM_NAME_LEN];
> +
> + pr_info("notifiers: unregistered %s()\n",
> + notifier_name(n, sym_name));
> + }
> +#endif
> + return ret;
> }
>
> /**
> @@ -80,6 +117,13 @@ static int notifier_call_chain(struct notifier_block **nl,
> nb = next_nb;
> continue;
> }
> +
> + {
> + char sym_name[KSYM_NAME_LEN];
> +
> + pr_debug("notifiers: calling %s()\n",
> + notifier_name(nb, sym_name));
> + }
> #endif
> ret = nb->notifier_call(nb, val, v);
>
> --
> 2.36.0
Powered by blists - more mailing lists