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
| ||
|
Date: Wed, 29 Mar 2017 11:13:58 +0100 From: Mark Rutland <mark.rutland@....com> To: Doug Berger <opendmb@...il.com> Cc: robh+dt@...nel.org, catalin.marinas@....com, will.deacon@....com, computersforpeace@...il.com, gregory.0xf0@...il.com, f.fainelli@...il.com, bcm-kernel-feedback-list@...adcom.com, wangkefeng.wang@...wei.com, james.morse@....com, mingo@...nel.org, sandeepa.s.prabhu@...il.com, shijie.huang@....com, linus.walleij@...aro.org, treding@...dia.com, jonathanh@...dia.com, olof@...om.net, mirza.krak@...il.com, suzuki.poulose@....com, bgolaszewski@...libre.com, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org Subject: Re: [PATCH v2 7/8] bus: brcmstb_gisb: add notifier handling Hi Doug, On Tue, Mar 28, 2017 at 02:34:30PM -0700, Doug Berger wrote: > Check for GISB arbitration errors through a chained notifier > when a process dies or a kernel panic occurs. This allows a > meaningful diagnostic message to occur along with other > diagnostic information. > > Notably writes to a bad GISB address on an ARM64 architecture > kernel cause unrecoverable SError aborts and this allows the > cause of the abort to be seen. > > Signed-off-by: Doug Berger <opendmb@...il.com> Thanks for changing this. > --- > drivers/bus/brcmstb_gisb.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c > index 500b6bb5c739..774729002b8c 100644 > --- a/drivers/bus/brcmstb_gisb.c > +++ b/drivers/bus/brcmstb_gisb.c > @@ -24,6 +24,8 @@ > #include <linux/of.h> > #include <linux/bitops.h> > #include <linux/pm.h> > +#include <linux/kernel.h> > +#include <linux/kdebug.h> > > #ifdef CONFIG_ARM > #include <asm/bug.h> > @@ -290,6 +292,25 @@ static irqreturn_t brcmstb_gisb_tea_handler(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +/* > + * Dump out gisb errors on die or panic. > + */ > +static int dump_gisb_error(struct notifier_block *self, unsigned long v, > + void *p) > +{ > + struct brcmstb_gisb_arb_device *gdev; > + > + /* iterate over each GISB arb registered handlers */ > + list_for_each_entry(gdev, &brcmstb_gisb_arb_device_list, next) > + brcmstb_gisb_arb_decode_addr(gdev, "async abort"); > + > + return 0; I think this should be NOTIFY_OK. > +} > + > +static struct notifier_block gisb_error_notifier = { > + .notifier_call = dump_gisb_error, > +}; > + > static DEVICE_ATTR(gisb_arb_timeout, S_IWUSR | S_IRUGO, > gisb_arb_get_timeout, gisb_arb_set_timeout); > > @@ -408,6 +429,12 @@ static int __init brcmstb_gisb_arb_probe(struct platform_device *pdev) > board_be_handler = brcmstb_bus_error_handler; > #endif > > + if (list_is_singular(&brcmstb_gisb_arb_device_list)) { > + register_die_notifier(&gisb_error_notifier); > + atomic_notifier_chain_register(&panic_notifier_list, > + &gisb_error_notifier); I don't think this is quite right. A notifier_block can only be registered to one notifier chain at a time, and this has the potential to corrupt both chains. I also think you only need to register the panic notifier. An SError should always result in a panic. Thanks, Mark.
Powered by blists - more mailing lists