linux-kernel - Re: [PATCH 15/30] bus: brcmstb_gisb: Clean-up panic/die notifiers

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YnqEqDnMfUgC4dM6@alley>
Date:   Tue, 10 May 2022 17:28:40 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     "Guilherme G. Piccoli" <gpiccoli@...lia.com>
Cc:     akpm@...ux-foundation.org, bhe@...hat.com,
        kexec@...ts.infradead.org, linux-kernel@...r.kernel.org,
        bcm-kernel-feedback-list@...adcom.com, coresight@...ts.linaro.org,
        linuxppc-dev@...ts.ozlabs.org, linux-alpha@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-edac@...r.kernel.org,
        linux-hyperv@...r.kernel.org, linux-leds@...r.kernel.org,
        linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
        linux-s390@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-um@...ts.infradead.org, linux-xtensa@...ux-xtensa.org,
        netdev@...r.kernel.org, openipmi-developer@...ts.sourceforge.net,
        rcu@...r.kernel.org, sparclinux@...r.kernel.org,
        xen-devel@...ts.xenproject.org, x86@...nel.org,
        kernel-dev@...lia.com, kernel@...ccoli.net, halves@...onical.com,
        fabiomirmar@...il.com, alejandro.j.jimenez@...cle.com,
        andriy.shevchenko@...ux.intel.com, arnd@...db.de, bp@...en8.de,
        corbet@....net, d.hatayama@...fujitsu.com,
        dave.hansen@...ux.intel.com, dyoung@...hat.com,
        feng.tang@...el.com, gregkh@...uxfoundation.org,
        mikelley@...rosoft.com, hidehiro.kawai.ez@...achi.com,
        jgross@...e.com, john.ogness@...utronix.de, keescook@...omium.org,
        luto@...nel.org, mhiramat@...nel.org, mingo@...hat.com,
        paulmck@...nel.org, peterz@...radead.org, rostedt@...dmis.org,
        senozhatsky@...omium.org, stern@...land.harvard.edu,
        tglx@...utronix.de, vgoyal@...hat.com, vkuznets@...hat.com,
        will@...nel.org, Brian Norris <computersforpeace@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH 15/30] bus: brcmstb_gisb: Clean-up panic/die notifiers

On Wed 2022-04-27 19:49:09, Guilherme G. Piccoli wrote:
> This patch improves the panic/die notifiers in this driver by
> making use of a passed "id" instead of comparing pointer
> address; also, it removes an useless prototype declaration
> and unnecessary header inclusion.
> 
> This is part of a panic notifiers refactor - this notifier in
> the future will be moved to a new list, that encompass the
> information notifiers only.
> 
> --- a/drivers/bus/brcmstb_gisb.c
> +++ b/drivers/bus/brcmstb_gisb.c
> @@ -347,25 +346,14 @@ static irqreturn_t brcmstb_gisb_bp_handler(int irq, void *dev_id)
>  /*
>   * Dump out gisb errors on die or panic.
>   */
> -static int dump_gisb_error(struct notifier_block *self, unsigned long v,
> -			   void *p);
> -
> -static struct notifier_block gisb_die_notifier = {
> -	.notifier_call = dump_gisb_error,
> -};
> -
> -static struct notifier_block gisb_panic_notifier = {
> -	.notifier_call = dump_gisb_error,
> -};
> -
>  static int dump_gisb_error(struct notifier_block *self, unsigned long v,
>  			   void *p)
>  {
>  	struct brcmstb_gisb_arb_device *gdev;
> -	const char *reason = "panic";
> +	const char *reason = "die";
>  
> -	if (self == &gisb_die_notifier)
> -		reason = "die";
> +	if (v == PANIC_NOTIFIER)
> +		reason = "panic";

IMHO, the check of the @self parameter was the proper solution.

"gisb_die_notifier" list uses @val from enum die_val.
"gisb_panic_notifier" list uses @val from enum panic_notifier_val.

These are unrelated types. It might easily break when
someone defines the same constant also in enum die_val.

Best Regards,
Petr