2021-11-08 13:38:55

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 00/42] notifiers: Return an error when callback is already registered

From: Borislav Petkov <[email protected]>

Hi all,

this is a huge patchset for something which is really trivial - it
changes the notifier registration routines to return an error value
if a notifier callback is already present on the respective list of
callbacks. For more details scroll to the last patch.

Everything before it is converting the callers to check the return value
of the registration routines and issue a warning, instead of the WARN()
notifier_chain_register() does now.

Before the last patch has been applied, though, that checking is a
NOP which would make the application of those patches trivial - every
maintainer can pick a patch at her/his discretion - only the last one
enables the build warnings and that one will be queued only after the
preceding patches have all been merged so that there are no build
warnings.

Due to the sheer volume of the patches, I have addressed the respective
patch and the last one, which enables the warning, with addressees for
each maintained area so as not to spam people unnecessarily.

If people prefer I carry some through tip, instead, I'll gladly do so -
your call.

And, if you think the warning messages need to be more precise, feel
free to adjust them before committing.

Thanks!

Borislav Petkov (42):
x86: Check notifier registration return value
xen/x86: Check notifier registration return value
impi: Check notifier registration return value
clk: renesas: Check notifier registration return value
dca: Check notifier registration return value
firmware: Check notifier registration return value
drm/i915: Check notifier registration return value
Drivers: hv: vmbus: Check notifier registration return value
iio: proximity: cros_ec: Check notifier registration return value
leds: trigger: Check notifier registration return value
misc: Check notifier registration return value
ethernet: chelsio: Check notifier registration return value
power: reset: Check notifier registration return value
remoteproc: Check notifier registration return value
scsi: target: Check notifier registration return value
USB: Check notifier registration return value
drivers: video: Check notifier registration return value
drivers/xen: Check notifier registration return value
kernel/hung_task: Check notifier registration return value
rcu: Check notifier registration return value
tracing: Check notifier registration return value
net: fib_notifier: Check notifier registration return value
ASoC: soc-jack: Check notifier registration return value
staging: olpc_dcon: Check notifier registration return value
arch/um: Check notifier registration return value
alpha: Check notifier registration return value
bus: brcmstb_gisb: Check notifier registration return value
soc: bcm: brcmstb: pm: pm-arm: Check notifier registration return
value
arm64: Check notifier registration return value
soc/tegra: Check notifier registration return value
parisc: Check notifier registration return value
macintosh/adb: Check notifier registration return value
mips: Check notifier registration return value
powerpc: Check notifier registration return value
sh: Check notifier registration return value
s390: Check notifier registration return value
sparc: Check notifier registration return value
xtensa: Check notifier registration return value
crypto: ccree - check notifier registration return value
EDAC/altera: Check notifier registration return value
power: supply: ab8500: Check notifier registration return value
notifier: Return an error when callback is already registered

arch/alpha/kernel/setup.c | 5 +--
arch/arm64/kernel/setup.c | 6 ++--
arch/mips/kernel/relocate.c | 6 ++--
arch/mips/sgi-ip22/ip22-reset.c | 4 ++-
arch/mips/sgi-ip32/ip32-reset.c | 4 ++-
arch/parisc/kernel/pdc_chassis.c | 5 +--
arch/powerpc/kernel/setup-common.c | 12 ++++---
arch/s390/kernel/ipl.c | 4 ++-
arch/s390/kvm/kvm-s390.c | 7 ++--
arch/sh/kernel/cpu/sh4a/setup-sh7724.c | 11 +++---
arch/sparc/kernel/sstate.c | 6 ++--
arch/um/drivers/mconsole_kern.c | 6 ++--
arch/um/kernel/um_arch.c | 5 +--
arch/x86/kernel/cpu/mce/core.c | 3 +-
arch/x86/kernel/cpu/mce/dev-mcelog.c | 3 +-
arch/x86/kernel/setup.c | 7 ++--
arch/x86/xen/enlighten.c | 4 ++-
arch/xtensa/platforms/iss/setup.c | 3 +-
drivers/bus/brcmstb_gisb.c | 6 ++--
drivers/char/ipmi/ipmi_msghandler.c | 3 +-
drivers/clk/renesas/clk-div6.c | 4 ++-
drivers/clk/renesas/rcar-cpg-lib.c | 4 ++-
drivers/crypto/ccree/cc_fips.c | 4 ++-
drivers/dca/dca-core.c | 3 +-
drivers/edac/altera_edac.c | 6 ++--
drivers/firmware/arm_scmi/notify.c | 3 +-
drivers/firmware/google/gsmi.c | 6 ++--
drivers/gpu/drm/i915/gvt/scheduler.c | 6 ++--
drivers/hv/vmbus_drv.c | 4 +--
.../iio/proximity/cros_ec_mkbp_proximity.c | 3 +-
drivers/leds/trigger/ledtrig-activity.c | 6 ++--
drivers/leds/trigger/ledtrig-heartbeat.c | 6 ++--
drivers/leds/trigger/ledtrig-panic.c | 4 +--
drivers/macintosh/adbhid.c | 4 +--
drivers/misc/ibmasm/heartbeat.c | 3 +-
drivers/misc/pvpanic/pvpanic.c | 3 +-
.../chelsio/inline_crypto/chtls/chtls_main.c | 5 ++-
drivers/parisc/power.c | 5 +--
drivers/power/reset/ltc2952-poweroff.c | 6 ++--
drivers/power/supply/ab8500_charger.c | 8 ++---
drivers/remoteproc/qcom_common.c | 3 +-
drivers/remoteproc/qcom_sysmon.c | 4 ++-
drivers/remoteproc/remoteproc_core.c | 4 ++-
drivers/s390/char/con3215.c | 5 ++-
drivers/s390/char/con3270.c | 5 ++-
drivers/s390/char/sclp_con.c | 4 ++-
drivers/s390/char/sclp_vt220.c | 4 ++-
drivers/s390/char/zcore.c | 4 ++-
drivers/soc/bcm/brcmstb/pm/pm-arm.c | 5 +--
drivers/soc/tegra/ari-tegra186.c | 7 ++--
drivers/staging/olpc_dcon/olpc_dcon.c | 4 ++-
drivers/target/tcm_fc/tfc_conf.c | 4 ++-
drivers/usb/core/notify.c | 3 +-
drivers/video/console/dummycon.c | 3 +-
drivers/video/fbdev/hyperv_fb.c | 5 +--
drivers/xen/manage.c | 3 +-
drivers/xen/xenbus/xenbus_probe.c | 8 +++--
include/linux/notifier.h | 8 ++---
kernel/hung_task.c | 3 +-
kernel/notifier.c | 36 ++++++++++---------
kernel/rcu/tree_stall.h | 4 ++-
kernel/trace/trace.c | 4 +--
net/core/fib_notifier.c | 4 ++-
sound/soc/soc-jack.c | 3 +-
64 files changed, 222 insertions(+), 118 deletions(-)

--
2.29.2


2021-11-08 13:39:46

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 09/42] iio: proximity: cros_ec: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/iio/proximity/cros_ec_mkbp_proximity.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/proximity/cros_ec_mkbp_proximity.c b/drivers/iio/proximity/cros_ec_mkbp_proximity.c
index 8213b0081713..e9536de8dd8a 100644
--- a/drivers/iio/proximity/cros_ec_mkbp_proximity.c
+++ b/drivers/iio/proximity/cros_ec_mkbp_proximity.c
@@ -234,7 +234,8 @@ static int cros_ec_mkbp_proximity_probe(struct platform_device *pdev)
return ret;

data->notifier.notifier_call = cros_ec_mkbp_proximity_notify;
- blocking_notifier_chain_register(&ec->event_notifier, &data->notifier);
+ if (blocking_notifier_chain_register(&ec->event_notifier, &data->notifier))
+ pr_warn("cros-ec proximity notifier already registered\n");

return 0;
}
--
2.29.2

2021-11-08 13:46:46

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 29/42] arm64: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/arm64/kernel/setup.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index be5f85b0a24d..e66af9de7cd7 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -440,8 +440,10 @@ static struct notifier_block arm64_panic_block = {

static int __init register_arm64_panic_block(void)
{
- atomic_notifier_chain_register(&panic_notifier_list,
- &arm64_panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &arm64_panic_block))
+ pr_warn("Panic notifier already registered\n");
+
return 0;
}
device_initcall(register_arm64_panic_block);
--
2.29.2

2021-11-08 13:46:46

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 30/42] soc/tegra: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/soc/tegra/ari-tegra186.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/tegra/ari-tegra186.c b/drivers/soc/tegra/ari-tegra186.c
index 02577853ec49..a25e1b7497e0 100644
--- a/drivers/soc/tegra/ari-tegra186.c
+++ b/drivers/soc/tegra/ari-tegra186.c
@@ -72,8 +72,11 @@ static struct notifier_block tegra186_ari_panic_nb = {

static int __init tegra186_ari_init(void)
{
- if (of_machine_is_compatible("nvidia,tegra186"))
- atomic_notifier_chain_register(&panic_notifier_list, &tegra186_ari_panic_nb);
+ if (of_machine_is_compatible("nvidia,tegra186")) {
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &tegra186_ari_panic_nb))
+ pr_warn("Panic notifier already registered\n");
+ }

return 0;
}
--
2.29.2

2021-11-08 13:47:34

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 31/42] parisc: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/parisc/kernel/pdc_chassis.c | 5 +++--
drivers/parisc/power.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/parisc/kernel/pdc_chassis.c b/arch/parisc/kernel/pdc_chassis.c
index da154406d368..8bedc9faa791 100644
--- a/arch/parisc/kernel/pdc_chassis.c
+++ b/arch/parisc/kernel/pdc_chassis.c
@@ -135,8 +135,9 @@ void __init parisc_pdc_chassis_init(void)
PDC_CHASSIS_VER);

/* initialize panic notifier chain */
- atomic_notifier_chain_register(&panic_notifier_list,
- &pdc_chassis_panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &pdc_chassis_panic_block))
+ printk(KERN_WARNING "Panic notifier already registered\n");

/* initialize reboot notifier chain */
register_reboot_notifier(&pdc_chassis_reboot_block);
diff --git a/drivers/parisc/power.c b/drivers/parisc/power.c
index 456776bd8ee6..26dabaa1e5f8 100644
--- a/drivers/parisc/power.c
+++ b/drivers/parisc/power.c
@@ -230,8 +230,9 @@ static int __init power_init(void)
}

/* Register a call for panic conditions. */
- atomic_notifier_chain_register(&panic_notifier_list,
- &parisc_panic_block);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &parisc_panic_block))
+ pr_warn("Panic notifier already registered\n");

return 0;
}
--
2.29.2

2021-11-08 15:01:42

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 03/42] impi: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/char/ipmi/ipmi_msghandler.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index deed355422f4..9df360b53a90 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -5381,7 +5381,8 @@ static int ipmi_init_msghandler(void)
timer_setup(&ipmi_timer, ipmi_timeout, 0);
mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES);

- atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &panic_block))
+ pr_warn("IPMI panic notifier already registered\n");

initialized = true;

--
2.29.2

2021-11-08 15:01:42

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 01/42] x86: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/x86/kernel/cpu/mce/core.c | 3 ++-
arch/x86/kernel/cpu/mce/dev-mcelog.c | 3 ++-
arch/x86/kernel/setup.c | 7 +++++--
3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 6ed365337a3b..2c4a9ff49384 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -164,7 +164,8 @@ void mce_register_decode_chain(struct notifier_block *nb)
nb->priority > MCE_PRIO_HIGHEST))
return;

- blocking_notifier_chain_register(&x86_mce_decoder_chain, nb);
+ if (blocking_notifier_chain_register(&x86_mce_decoder_chain, nb))
+ pr_warn("MCE decoder chain notifier already registered\n");
}
EXPORT_SYMBOL_GPL(mce_register_decode_chain);

diff --git a/arch/x86/kernel/cpu/mce/dev-mcelog.c b/arch/x86/kernel/cpu/mce/dev-mcelog.c
index 100fbeebdc72..efa503a512dd 100644
--- a/arch/x86/kernel/cpu/mce/dev-mcelog.c
+++ b/arch/x86/kernel/cpu/mce/dev-mcelog.c
@@ -281,7 +281,8 @@ static long mce_chrdev_ioctl(struct file *f, unsigned int cmd,

void mce_register_injector_chain(struct notifier_block *nb)
{
- blocking_notifier_chain_register(&mce_injector_chain, nb);
+ if (blocking_notifier_chain_register(&mce_injector_chain, nb))
+ pr_warn("MCE injector notifier already registered\n");
}
EXPORT_SYMBOL_GPL(mce_register_injector_chain);

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 40ed44ead063..e28cd4dd81c1 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1245,8 +1245,11 @@ static struct notifier_block kernel_offset_notifier = {

static int __init register_kernel_offset_dumper(void)
{
- atomic_notifier_chain_register(&panic_notifier_list,
- &kernel_offset_notifier);
+ int ret = atomic_notifier_chain_register(&panic_notifier_list,
+ &kernel_offset_notifier);
+ if (ret)
+ pr_warn("Kernel offset dumper notifier already registered\n");
+
return 0;
}
__initcall(register_kernel_offset_dumper);
--
2.29.2

2021-11-08 15:01:43

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 05/42] dca: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/dca/dca-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dca/dca-core.c b/drivers/dca/dca-core.c
index c40c2ebfdae9..3e8f52214652 100644
--- a/drivers/dca/dca-core.c
+++ b/drivers/dca/dca-core.c
@@ -428,7 +428,8 @@ EXPORT_SYMBOL_GPL(unregister_dca_provider);
*/
void dca_register_notify(struct notifier_block *nb)
{
- blocking_notifier_chain_register(&dca_provider_chain, nb);
+ if (blocking_notifier_chain_register(&dca_provider_chain, nb))
+ pr_warn("dca provider notifier already registered\n");
}
EXPORT_SYMBOL_GPL(dca_register_notify);

--
2.29.2

2021-11-08 15:01:53

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 02/42] xen/x86: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/x86/xen/enlighten.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 95d970359e17..2264dd6e157f 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -354,7 +354,9 @@ static struct notifier_block xen_panic_block = {

int xen_panic_handler_init(void)
{
- atomic_notifier_chain_register(&panic_notifier_list, &xen_panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &xen_panic_block))
+ pr_warn("Xen panic notifier already registered\n");
+
return 0;
}

--
2.29.2

2021-11-08 15:01:54

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 08/42] Drivers: hv: vmbus: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/hv/vmbus_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 392c1ac4f819..370afd108d2d 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1574,8 +1574,8 @@ static int vmbus_bus_init(void)
* the VMbus channel connection to prevent any VMbus
* activity after the VM panics.
*/
- atomic_notifier_chain_register(&panic_notifier_list,
- &hyperv_panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &hyperv_panic_block))
+ pr_warn("VMBus panic notifier already registered\n");

vmbus_request_offers();

--
2.29.2

2021-11-08 15:02:31

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 11/42] misc: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/misc/ibmasm/heartbeat.c | 3 ++-
drivers/misc/pvpanic/pvpanic.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/ibmasm/heartbeat.c b/drivers/misc/ibmasm/heartbeat.c
index 59c9a0d95659..f7c22d9a768b 100644
--- a/drivers/misc/ibmasm/heartbeat.c
+++ b/drivers/misc/ibmasm/heartbeat.c
@@ -39,7 +39,8 @@ static struct notifier_block panic_notifier = { panic_happened, NULL, 1 };

void ibmasm_register_panic_notifier(void)
{
- atomic_notifier_chain_register(&panic_notifier_list, &panic_notifier);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &panic_notifier))
+ pr_warn("IBM ASM panic notifier already registered\n");
}

void ibmasm_unregister_panic_notifier(void)
diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
index 4b8f1c7d726d..6e89e8b1beaf 100644
--- a/drivers/misc/pvpanic/pvpanic.c
+++ b/drivers/misc/pvpanic/pvpanic.c
@@ -95,7 +95,8 @@ static int pvpanic_init(void)
INIT_LIST_HEAD(&pvpanic_list);
spin_lock_init(&pvpanic_lock);

- atomic_notifier_chain_register(&panic_notifier_list, &pvpanic_panic_nb);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &pvpanic_panic_nb))
+ pr_warn("PV panic notifier already registered\n");

return 0;
}
--
2.29.2

2021-11-08 15:03:04

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 12/42] ethernet: chelsio: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Ayush Sawal <[email protected]>
Cc: Vinay Kumar Yadav <[email protected]>
Cc: Rohit Maheshwari <[email protected]>
---
.../net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c
index 9098b3eed4da..43ee3fb6bc67 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c
@@ -39,7 +39,10 @@ static uint send_page_order = (14 - PAGE_SHIFT < 0) ? 0 : 14 - PAGE_SHIFT;
static void register_listen_notifier(struct notifier_block *nb)
{
mutex_lock(&notify_mutex);
- raw_notifier_chain_register(&listen_notify_list, nb);
+
+ if (raw_notifier_chain_register(&listen_notify_list, nb))
+ pr_warn("chtls listen notifier already registered\n");
+
mutex_unlock(&notify_mutex);
}

--
2.29.2

2021-11-08 15:03:31

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 15/42] scsi: target: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/target/tcm_fc/tfc_conf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/target/tcm_fc/tfc_conf.c b/drivers/target/tcm_fc/tfc_conf.c
index 1a38c98f681b..28960f68de08 100644
--- a/drivers/target/tcm_fc/tfc_conf.c
+++ b/drivers/target/tcm_fc/tfc_conf.c
@@ -465,7 +465,9 @@ static int __init ft_init(void)
if (ret)
goto out_unregister_template;

- blocking_notifier_chain_register(&fc_lport_notifier_head, &ft_notifier);
+ if (blocking_notifier_chain_register(&fc_lport_notifier_head, &ft_notifier))
+ pr_warn("FC lport notifier already registered\n");
+
fc_lport_iterate(ft_lport_add, NULL);
return 0;

--
2.29.2

2021-11-08 15:03:41

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 22/42] net: fib_notifier: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
net/core/fib_notifier.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
index fc96259807b6..fa07d6a81ba0 100644
--- a/net/core/fib_notifier.c
+++ b/net/core/fib_notifier.c
@@ -86,7 +86,9 @@ static bool fib_dump_is_consistent(struct net *net, struct notifier_block *nb,
{
struct fib_notifier_net *fn_net = net_generic(net, fib_notifier_net_id);

- atomic_notifier_chain_register(&fn_net->fib_chain, nb);
+ if (atomic_notifier_chain_register(&fn_net->fib_chain, nb))
+ pr_warn("FIB notifier already registered\n");
+
if (fib_seq == fib_seq_sum(net))
return true;
atomic_notifier_chain_unregister(&fn_net->fib_chain, nb);
--
2.29.2

2021-11-08 15:07:47

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 14/42] remoteproc: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/remoteproc/qcom_common.c | 3 ++-
drivers/remoteproc/qcom_sysmon.c | 4 +++-
drivers/remoteproc/remoteproc_core.c | 4 +++-
3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 4b91e3c9eafa..61a7812fe4df 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -390,7 +390,8 @@ void *qcom_register_ssr_notifier(const char *name, struct notifier_block *nb)
if (IS_ERR(info))
return info;

- srcu_notifier_chain_register(&info->notifier_list, nb);
+ if (srcu_notifier_chain_register(&info->notifier_list, nb))
+ pr_warn("SSR notifier already registered\n");

return &info->notifier_list;
}
diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
index 9fca81492863..ffe7811f8aac 100644
--- a/drivers/remoteproc/qcom_sysmon.c
+++ b/drivers/remoteproc/qcom_sysmon.c
@@ -673,7 +673,9 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
rproc_add_subdev(rproc, &sysmon->subdev);

sysmon->nb.notifier_call = sysmon_notify;
- blocking_notifier_chain_register(&sysmon_notifiers, &sysmon->nb);
+
+ if (blocking_notifier_chain_register(&sysmon_notifiers, &sysmon->nb))
+ pr_warn("sysmon notifier already registered\n");

mutex_lock(&sysmon_lock);
list_add(&sysmon->node, &sysmon_list);
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 502b6604b757..18d477e6141a 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -2790,7 +2790,9 @@ static int rproc_panic_handler(struct notifier_block *nb, unsigned long event,
static void __init rproc_init_panic(void)
{
rproc_panic_nb.notifier_call = rproc_panic_handler;
- atomic_notifier_chain_register(&panic_notifier_list, &rproc_panic_nb);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &rproc_panic_nb))
+ pr_warn("Remote Proc notifier already registered\n");
}

static void __exit rproc_exit_panic(void)
--
2.29.2

2021-11-08 15:07:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 16/42] USB: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/usb/core/notify.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/notify.c b/drivers/usb/core/notify.c
index e6143663778f..80d1bfa61887 100644
--- a/drivers/usb/core/notify.c
+++ b/drivers/usb/core/notify.c
@@ -28,7 +28,8 @@ static BLOCKING_NOTIFIER_HEAD(usb_notifier_list);
*/
void usb_register_notify(struct notifier_block *nb)
{
- blocking_notifier_chain_register(&usb_notifier_list, nb);
+ if (blocking_notifier_chain_register(&usb_notifier_list, nb))
+ pr_warn("USB change notifier already registered\n");
}
EXPORT_SYMBOL_GPL(usb_register_notify);

--
2.29.2

2021-11-08 15:07:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 10/42] leds: trigger: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/leds/trigger/ledtrig-activity.c | 6 ++++--
drivers/leds/trigger/ledtrig-heartbeat.c | 6 ++++--
drivers/leds/trigger/ledtrig-panic.c | 4 ++--
3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-activity.c b/drivers/leds/trigger/ledtrig-activity.c
index 30bc9df03636..c2b7a3d322f3 100644
--- a/drivers/leds/trigger/ledtrig-activity.c
+++ b/drivers/leds/trigger/ledtrig-activity.c
@@ -247,8 +247,10 @@ static int __init activity_init(void)
int rc = led_trigger_register(&activity_led_trigger);

if (!rc) {
- atomic_notifier_chain_register(&panic_notifier_list,
- &activity_panic_nb);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &activity_panic_nb))
+ pr_warn("Activity LED trigger notifier already registered\n");
+
register_reboot_notifier(&activity_reboot_nb);
}
return rc;
diff --git a/drivers/leds/trigger/ledtrig-heartbeat.c b/drivers/leds/trigger/ledtrig-heartbeat.c
index 7fe0a05574d2..747c5113d528 100644
--- a/drivers/leds/trigger/ledtrig-heartbeat.c
+++ b/drivers/leds/trigger/ledtrig-heartbeat.c
@@ -190,8 +190,10 @@ static int __init heartbeat_trig_init(void)
int rc = led_trigger_register(&heartbeat_led_trigger);

if (!rc) {
- atomic_notifier_chain_register(&panic_notifier_list,
- &heartbeat_panic_nb);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &heartbeat_panic_nb))
+ pr_warn("Heartbeat LED Trigger notifier already registered\n");
+
register_reboot_notifier(&heartbeat_reboot_nb);
}
return rc;
diff --git a/drivers/leds/trigger/ledtrig-panic.c b/drivers/leds/trigger/ledtrig-panic.c
index 64abf2e91608..c5103f51c3de 100644
--- a/drivers/leds/trigger/ledtrig-panic.c
+++ b/drivers/leds/trigger/ledtrig-panic.c
@@ -64,8 +64,8 @@ static long led_panic_blink(int state)

static int __init ledtrig_panic_init(void)
{
- atomic_notifier_chain_register(&panic_notifier_list,
- &led_trigger_panic_nb);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &led_trigger_panic_nb))
+ pr_warn("LED trigger panic notifier already registered\n");

led_trigger_register_simple("panic", &trigger);
panic_blink = led_panic_blink;
--
2.29.2

2021-11-08 15:07:58

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 13/42] power: reset: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/power/reset/ltc2952-poweroff.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index fbb344353fe4..6ca251e4b1be 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -279,8 +279,10 @@ static int ltc2952_poweroff_probe(struct platform_device *pdev)
pm_power_off = ltc2952_poweroff_kill;

data->panic_notifier.notifier_call = ltc2952_poweroff_notify_panic;
- atomic_notifier_chain_register(&panic_notifier_list,
- &data->panic_notifier);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &data->panic_notifier))
+ pr_warn("LTC2952 panic notifier already registered\n");
+
dev_info(&pdev->dev, "probe successful\n");

return 0;
--
2.29.2

2021-11-08 15:08:25

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 17/42] drivers: video: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/video/console/dummycon.c | 3 ++-
drivers/video/fbdev/hyperv_fb.c | 5 +++--
2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index f1711b2f9ff0..2d0cdd76dfde 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -36,7 +36,8 @@ void dummycon_register_output_notifier(struct notifier_block *nb)
{
WARN_CONSOLE_UNLOCKED();

- raw_notifier_chain_register(&dummycon_output_nh, nb);
+ if (raw_notifier_chain_register(&dummycon_output_nh, nb))
+ pr_warn("dummycon output notifier already registered\n");

if (dummycon_putc_called)
nb->notifier_call(nb, 0, NULL);
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 23999df52739..183bd459e5c2 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -1255,8 +1255,9 @@ static int hvfb_probe(struct hv_device *hdev,

par->synchronous_fb = false;
par->hvfb_panic_nb.notifier_call = hvfb_on_panic;
- atomic_notifier_chain_register(&panic_notifier_list,
- &par->hvfb_panic_nb);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &par->hvfb_panic_nb))
+ pr_warn("Hyper-V FB panic notifier already registered\n");

return 0;

--
2.29.2

2021-11-08 15:08:44

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 24/42] staging: olpc_dcon: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/staging/olpc_dcon/olpc_dcon.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c b/drivers/staging/olpc_dcon/olpc_dcon.c
index 7284cb4ac395..f5204a88bb35 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon.c
@@ -653,7 +653,9 @@ static int dcon_probe(struct i2c_client *client, const struct i2c_device_id *id)
}

register_reboot_notifier(&dcon->reboot_nb);
- atomic_notifier_chain_register(&panic_notifier_list, &dcon_panic_nb);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &dcon_panic_nb))
+ pr_warn("Panic notifier already registered\n");

return 0;

--
2.29.2

2021-11-08 15:08:47

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 36/42] s390: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/s390/kernel/ipl.c | 4 +++-
arch/s390/kvm/kvm-s390.c | 7 +++++--
drivers/s390/char/con3215.c | 5 ++++-
drivers/s390/char/con3270.c | 5 ++++-
drivers/s390/char/sclp_con.c | 4 +++-
drivers/s390/char/sclp_vt220.c | 4 +++-
drivers/s390/char/zcore.c | 4 +++-
7 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
index e2cc35775b99..3b1dceede55c 100644
--- a/arch/s390/kernel/ipl.c
+++ b/arch/s390/kernel/ipl.c
@@ -2069,7 +2069,9 @@ void __init setup_ipl(void)
/* We have no info to copy */
break;
}
- atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb))
+ pr_warn("Panic notifier already registered\n");
}

void s390_reset_system(void)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index c6257f625929..11b325724272 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -325,8 +325,11 @@ int kvm_arch_hardware_setup(void *opaque)
gmap_register_pte_notifier(&gmap_notifier);
vsie_gmap_notifier.notifier_call = kvm_s390_vsie_gmap_notifier;
gmap_register_pte_notifier(&vsie_gmap_notifier);
- atomic_notifier_chain_register(&s390_epoch_delta_notifier,
- &kvm_clock_notifier);
+
+ if (atomic_notifier_chain_register(&s390_epoch_delta_notifier,
+ &kvm_clock_notifier))
+ pr_warn("KVM clock notifier already registered\n");
+
return 0;
}

diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
index f356607835d8..52b441b29fbc 100644
--- a/drivers/s390/char/con3215.c
+++ b/drivers/s390/char/con3215.c
@@ -865,7 +865,10 @@ static int __init con3215_init(void)
raw3215[0] = NULL;
return -ENODEV;
}
- atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb))
+ pr_warn("Panic notifier already registered\n");
+
register_reboot_notifier(&on_reboot_nb);
register_console(&con3215);
return 0;
diff --git a/drivers/s390/char/con3270.c b/drivers/s390/char/con3270.c
index e4592890f20a..0ce25924fb18 100644
--- a/drivers/s390/char/con3270.c
+++ b/drivers/s390/char/con3270.c
@@ -641,7 +641,10 @@ con3270_init(void)
condev->cline->len = 0;
con3270_create_status(condev);
condev->input = alloc_string(&condev->freemem, 80);
- atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb))
+ pr_warn("Panic notifier already registered\n");
+
register_reboot_notifier(&on_reboot_nb);
register_console(&con3270);
return 0;
diff --git a/drivers/s390/char/sclp_con.c b/drivers/s390/char/sclp_con.c
index de028868c6f4..42bb2ddaf791 100644
--- a/drivers/s390/char/sclp_con.c
+++ b/drivers/s390/char/sclp_con.c
@@ -285,7 +285,9 @@ sclp_console_init(void)
timer_setup(&sclp_con_timer, sclp_console_timeout, 0);

/* enable printk-access to this driver */
- atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb))
+ pr_warn("Panic notifier already registered\n");
+
register_reboot_notifier(&on_reboot_nb);
register_console(&sclp_console);
return 0;
diff --git a/drivers/s390/char/sclp_vt220.c b/drivers/s390/char/sclp_vt220.c
index 29a6a0099f83..89cf7a7b0194 100644
--- a/drivers/s390/char/sclp_vt220.c
+++ b/drivers/s390/char/sclp_vt220.c
@@ -836,7 +836,9 @@ sclp_vt220_con_init(void)
if (rc)
return rc;
/* Attach linux console */
- atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb))
+ pr_warn("Panic notifier already registered\n");
+
register_reboot_notifier(&on_reboot_nb);
register_console(&sclp_vt220_console);
return 0;
diff --git a/drivers/s390/char/zcore.c b/drivers/s390/char/zcore.c
index 3ba2d934a3e8..0bf28583a3b9 100644
--- a/drivers/s390/char/zcore.c
+++ b/drivers/s390/char/zcore.c
@@ -323,7 +323,9 @@ static int __init zcore_init(void)
NULL, &zcore_hsa_fops);

register_reboot_notifier(&zcore_reboot_notifier);
- atomic_notifier_chain_register(&panic_notifier_list, &zcore_on_panic_notifier);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &zcore_on_panic_notifier))
+ pr_warn("Panic notifier already registered\n");

return 0;
fail:
--
2.29.2

2021-11-08 15:08:58

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 27/42] bus: brcmstb_gisb: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/bus/brcmstb_gisb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 4c2f7d61cb9b..1c0180ebe6a7 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -490,8 +490,10 @@ static int __init brcmstb_gisb_arb_probe(struct platform_device *pdev)

if (list_is_singular(&brcmstb_gisb_arb_device_list)) {
register_die_notifier(&gisb_die_notifier);
- atomic_notifier_chain_register(&panic_notifier_list,
- &gisb_panic_notifier);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &gisb_panic_notifier))
+ pr_warn("GISB Panic notifier already registered\n");
}

dev_info(&pdev->dev, "registered irqs: %d, %d\n",
--
2.29.2

2021-11-08 15:08:59

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 19/42] kernel/hung_task: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
---
kernel/hung_task.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 9888e2bc8c76..8e3bb95968a4 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -304,7 +304,8 @@ static int watchdog(void *dummy)

static int __init hung_task_init(void)
{
- atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &panic_block))
+ pr_warn("Hung task notifier already registered\n");

/* Disable hung task detector on suspend */
pm_notifier(hungtask_pm_notify, 0);
--
2.29.2

2021-11-08 15:09:01

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 21/42] tracing: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Steven Rostedt <[email protected]>
---
kernel/trace/trace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index f9139dc1262c..87fd6f1fa407 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10064,8 +10064,8 @@ __init static int tracer_alloc_buffers(void)
/* All seems OK, enable tracing */
tracing_disabled = 0;

- atomic_notifier_chain_register(&panic_notifier_list,
- &trace_panic_notifier);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &trace_panic_notifier))
+ pr_warn("Trace panic notifier already registered\n");

register_die_notifier(&trace_die_notifier);

--
2.29.2

2021-11-08 15:10:14

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 40/42] EDAC/altera: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/edac/altera_edac.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 3a6d2416cb0f..49fa7ae77ba3 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -2125,8 +2125,10 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
int dberror, err_addr;

edac->panic_notifier.notifier_call = s10_edac_dberr_handler;
- atomic_notifier_chain_register(&panic_notifier_list,
- &edac->panic_notifier);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &edac->panic_notifier))
+ pr_warn("Panic notifier already registered\n");

/* Printout a message if uncorrectable error previously. */
regmap_read(edac->ecc_mgr_map, S10_SYSMGR_UE_VAL_OFST,
--
2.29.2

2021-11-08 15:10:19

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 25/42] arch/um: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/um/drivers/mconsole_kern.c | 6 ++++--
arch/um/kernel/um_arch.c | 5 +++--
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c
index 6ead1e240457..ceea940c0aa0 100644
--- a/arch/um/drivers/mconsole_kern.c
+++ b/arch/um/drivers/mconsole_kern.c
@@ -856,8 +856,10 @@ static struct notifier_block panic_exit_notifier = {

static int add_notifier(void)
{
- atomic_notifier_chain_register(&panic_notifier_list,
- &panic_exit_notifier);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &panic_exit_notifier))
+ pr_warn("UM console panic notifier already registered\n");
+
return 0;
}

diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 54447690de11..6c0e562ff0b7 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -255,8 +255,9 @@ static struct notifier_block panic_exit_notifier = {

void uml_finishsetup(void)
{
- atomic_notifier_chain_register(&panic_notifier_list,
- &panic_exit_notifier);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &panic_exit_notifier))
+ pr_warn("UM panic notifier already registered\n");

uml_postsetup();

--
2.29.2

2021-11-08 15:22:58

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 32/42] macintosh/adb: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/macintosh/adbhid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/macintosh/adbhid.c b/drivers/macintosh/adbhid.c
index 994ba5cb3678..c8cbf8588186 100644
--- a/drivers/macintosh/adbhid.c
+++ b/drivers/macintosh/adbhid.c
@@ -1262,8 +1262,8 @@ static int __init adbhid_init(void)

adbhid_probe();

- blocking_notifier_chain_register(&adb_client_list,
- &adbhid_adb_notifier);
+ if (blocking_notifier_chain_register(&adb_client_list, &adbhid_adb_notifier))
+ pr_warn("ADB message notifier already registered\n");

return 0;
}
--
2.29.2

2021-11-08 15:23:53

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 26/42] alpha: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/alpha/kernel/setup.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index b4fbbba30aa2..520c08031137 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -465,8 +465,9 @@ setup_arch(char **cmdline_p)
}

/* Register a call for panic conditions. */
- atomic_notifier_chain_register(&panic_notifier_list,
- &alpha_panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &alpha_panic_block))
+ pr_warn("Panic notifier already registered\n");

#ifndef alpha_using_srm
/* Assume that we've booted from SRM if we haven't booted from MILO.
--
2.29.2

2021-11-08 15:23:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 41/42] power: supply: ab8500: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/power/supply/ab8500_charger.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c
index 15eadaf46f14..de99f070cbc2 100644
--- a/drivers/power/supply/ab8500_charger.c
+++ b/drivers/power/supply/ab8500_charger.c
@@ -3644,10 +3644,10 @@ static int ab8500_charger_probe(struct platform_device *pdev)
}

/* Notifier for external charger enabling */
- if (!di->ac_chg.enabled)
- blocking_notifier_chain_register(
- &charger_notifier_list, &charger_nb);
-
+ if (!di->ac_chg.enabled) {
+ if (blocking_notifier_chain_register(&charger_notifier_list, &charger_nb))
+ pr_warn("Charger notifier already registered\n");
+ }

di->usb_phy = usb_get_phy(USB_PHY_TYPE_USB2);
if (IS_ERR_OR_NULL(di->usb_phy)) {
--
2.29.2

2021-11-08 15:23:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 28/42] soc: bcm: brcmstb: pm: pm-arm: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/soc/bcm/brcmstb/pm/pm-arm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/bcm/brcmstb/pm/pm-arm.c b/drivers/soc/bcm/brcmstb/pm/pm-arm.c
index 3cbb165d6e30..f80068ec57cb 100644
--- a/drivers/soc/bcm/brcmstb/pm/pm-arm.c
+++ b/drivers/soc/bcm/brcmstb/pm/pm-arm.c
@@ -802,8 +802,9 @@ static int brcmstb_pm_probe(struct platform_device *pdev)
goto out;
}

- atomic_notifier_chain_register(&panic_notifier_list,
- &brcmstb_pm_panic_nb);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &brcmstb_pm_panic_nb))
+ pr_warn("Panic notifier already registered\n");

pm_power_off = brcmstb_pm_poweroff;
suspend_set_ops(&brcmstb_pm_ops);
--
2.29.2

2021-11-08 15:27:32

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 37/42] sparc: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/sparc/kernel/sstate.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/sstate.c b/arch/sparc/kernel/sstate.c
index 3bcc4ddc6911..4e29e8977609 100644
--- a/arch/sparc/kernel/sstate.c
+++ b/arch/sparc/kernel/sstate.c
@@ -106,8 +106,10 @@ static int __init sstate_init(void)

do_set_sstate(HV_SOFT_STATE_TRANSITION, booting_msg);

- atomic_notifier_chain_register(&panic_notifier_list,
- &sstate_panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &sstate_panic_block))
+ pr_warn("Soft state panic notifier already registered\n");
+
register_reboot_notifier(&sstate_reboot_notifier);

return 0;
--
2.29.2

2021-11-08 15:27:32

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 35/42] sh: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/sh/kernel/cpu/sh4a/setup-sh7724.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
index 0d990ab1ba2a..8dfbb8149f66 100644
--- a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
+++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
@@ -1277,11 +1277,14 @@ static struct notifier_block sh7724_post_sleep_notifier = {

static int __init sh7724_sleep_setup(void)
{
- atomic_notifier_chain_register(&sh_mobile_pre_sleep_notifier_list,
- &sh7724_pre_sleep_notifier);
+ if (atomic_notifier_chain_register(&sh_mobile_pre_sleep_notifier_list,
+ &sh7724_pre_sleep_notifier))
+ pr_warn("SH7724 pre-sleep notifier already registered\n");
+
+ if (atomic_notifier_chain_register(&sh_mobile_post_sleep_notifier_list,
+ &sh7724_post_sleep_notifier))
+ pr_warn("SH7724 pre-sleep notifier already registered\n");

- atomic_notifier_chain_register(&sh_mobile_post_sleep_notifier_list,
- &sh7724_post_sleep_notifier);
return 0;
}
arch_initcall(sh7724_sleep_setup);
--
2.29.2

2021-11-08 15:27:43

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 33/42] mips: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/mips/kernel/relocate.c | 6 ++++--
arch/mips/sgi-ip22/ip22-reset.c | 4 +++-
arch/mips/sgi-ip32/ip32-reset.c | 4 +++-
3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/mips/kernel/relocate.c b/arch/mips/kernel/relocate.c
index 56b51de2dc51..d577654242da 100644
--- a/arch/mips/kernel/relocate.c
+++ b/arch/mips/kernel/relocate.c
@@ -459,8 +459,10 @@ static struct notifier_block kernel_location_notifier = {

static int __init register_kernel_offset_dumper(void)
{
- atomic_notifier_chain_register(&panic_notifier_list,
- &kernel_location_notifier);
+ if (atomic_notifier_chain_register(&panic_notifier_list,
+ &kernel_location_notifier))
+ pr_warn("Kernel location notifier already registered\n");
+
return 0;
}
__initcall(register_kernel_offset_dumper);
diff --git a/arch/mips/sgi-ip22/ip22-reset.c b/arch/mips/sgi-ip22/ip22-reset.c
index 9028dbbb45dd..841fd31cac03 100644
--- a/arch/mips/sgi-ip22/ip22-reset.c
+++ b/arch/mips/sgi-ip22/ip22-reset.c
@@ -196,7 +196,9 @@ static int __init reboot_setup(void)
}

timer_setup(&blink_timer, blink_timeout, 0);
- atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &panic_block))
+ pr_warn("Panic notifier already registered\n");

return 0;
}
diff --git a/arch/mips/sgi-ip32/ip32-reset.c b/arch/mips/sgi-ip32/ip32-reset.c
index 18d1c115cd53..bdad9213f81b 100644
--- a/arch/mips/sgi-ip32/ip32-reset.c
+++ b/arch/mips/sgi-ip32/ip32-reset.c
@@ -145,7 +145,9 @@ static __init int ip32_reboot_setup(void)
pm_power_off = ip32_machine_halt;

timer_setup(&blink_timer, blink_timeout, 0);
- atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
+
+ if (atomic_notifier_chain_register(&panic_notifier_list, &panic_block))
+ pr_warn("Panic notifier already registered\n");

return 0;
}
--
2.29.2

2021-11-08 15:27:43

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 23/42] ASoC: soc-jack: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
sound/soc/soc-jack.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index d798765d168c..0875f5022066 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -181,7 +181,8 @@ EXPORT_SYMBOL_GPL(snd_soc_jack_add_pins);
void snd_soc_jack_notifier_register(struct snd_soc_jack *jack,
struct notifier_block *nb)
{
- blocking_notifier_chain_register(&jack->notifier, nb);
+ if (blocking_notifier_chain_register(&jack->notifier, nb))
+ pr_warn("Jack status notifier already registered\n");
}
EXPORT_SYMBOL_GPL(snd_soc_jack_notifier_register);

--
2.29.2

2021-11-08 15:28:08

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 38/42] xtensa: Check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/xtensa/platforms/iss/setup.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/xtensa/platforms/iss/setup.c b/arch/xtensa/platforms/iss/setup.c
index d3433e1bb94e..00b847e3cb76 100644
--- a/arch/xtensa/platforms/iss/setup.c
+++ b/arch/xtensa/platforms/iss/setup.c
@@ -81,5 +81,6 @@ void __init platform_setup(char **p_cmdline)
}
}

- atomic_notifier_chain_register(&panic_notifier_list, &iss_panic_block);
+ if (atomic_notifier_chain_register(&panic_notifier_list, &iss_panic_block))
+ pr_warn("Panic notifier already registered\n");
}
--
2.29.2

2021-11-08 15:44:46

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v0 08/42] Drivers: hv: vmbus: Check notifier registration return value

On Mon, Nov 08, 2021 at 11:11:23AM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Avoid homegrown notifier registration checks.
>
> No functional changes.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: [email protected]

Acked-by: Wei Liu <[email protected]>

> ---
> drivers/hv/vmbus_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 392c1ac4f819..370afd108d2d 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1574,8 +1574,8 @@ static int vmbus_bus_init(void)
> * the VMbus channel connection to prevent any VMbus
> * activity after the VM panics.
> */
> - atomic_notifier_chain_register(&panic_notifier_list,
> - &hyperv_panic_block);
> + if (atomic_notifier_chain_register(&panic_notifier_list, &hyperv_panic_block))
> + pr_warn("VMBus panic notifier already registered\n");
>
> vmbus_request_offers();
>
> --
> 2.29.2
>

2021-11-08 15:55:09

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 08/42] Drivers: hv: vmbus: Check notifier registration return value

On Mon, Nov 08, 2021 at 11:16:37AM +0000, Wei Liu wrote:
> On Mon, Nov 08, 2021 at 11:11:23AM +0100, Borislav Petkov wrote:
> > From: Borislav Petkov <[email protected]>
> >
> > Avoid homegrown notifier registration checks.
> >
> > No functional changes.
> >
> > Signed-off-by: Borislav Petkov <[email protected]>
> > Cc: [email protected]
>
> Acked-by: Wei Liu <[email protected]>

Thanks.

I assume your ack means, I can take the two through tip.

Alternatively, you can take 'em too, if you prefer, as there's no
dependency, see here:

https://lore.kernel.org/r/[email protected]

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-08 16:02:51

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 08/42] Drivers: hv: vmbus: Check notifier registration return value

On Mon, Nov 08, 2021 at 11:45:49AM +0000, Wei Liu wrote:
> Yes please take them through tip. I should've been clearer on this.
> Sorry.

No worries - I'll do so.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-08 16:36:46

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 35/42] sh: Check notifier registration return value

On Mon, Nov 08, 2021 at 02:31:41PM +0100, Geert Uytterhoeven wrote:
> Do you think these can actually fail?

Hmm, maybe you missed the 0th message. Does this explain it:

https://lore.kernel.org/r/[email protected]

?

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-08 16:47:23

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v0 35/42] sh: Check notifier registration return value

Hi Borislav,

On Mon, Nov 8, 2021 at 2:49 PM Borislav Petkov <[email protected]> wrote:
> On Mon, Nov 08, 2021 at 02:31:41PM +0100, Geert Uytterhoeven wrote:
> > Do you think these can actually fail?
>
> Hmm, maybe you missed the 0th message. Does this explain it:
>
> https://lore.kernel.org/r/[email protected]
>
> ?

Thanks, but that still doesn't explain why we need to add the check,
for something that IMHO cannot fail, in a caller that cannot do
anything in the very unlikely event that he call would ever start
to fail?

The clue is the addition of __must_check in "[PATCH v0 42/42] notifier:
Return an error when callback is already registered"
(https://lore.kernel.org/all/[email protected]/).

I'll reply to that one...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-08 16:47:25

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v0 16/42] USB: Check notifier registration return value

On Mon, Nov 08, 2021 at 11:11:31AM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Avoid homegrown notifier registration checks.

This is a rather misleading description. The patch does exactly the
opposite: It _adds_ a homegrown notifier registration check. (Homegrown
in the sense that the check is made by the individual caller rather than
being centralized in the routine being called.)

Alan Stern

> No functional changes.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: [email protected]
> ---
> drivers/usb/core/notify.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/notify.c b/drivers/usb/core/notify.c
> index e6143663778f..80d1bfa61887 100644
> --- a/drivers/usb/core/notify.c
> +++ b/drivers/usb/core/notify.c
> @@ -28,7 +28,8 @@ static BLOCKING_NOTIFIER_HEAD(usb_notifier_list);
> */
> void usb_register_notify(struct notifier_block *nb)
> {
> - blocking_notifier_chain_register(&usb_notifier_list, nb);
> + if (blocking_notifier_chain_register(&usb_notifier_list, nb))
> + pr_warn("USB change notifier already registered\n");
> }
> EXPORT_SYMBOL_GPL(usb_register_notify);
>
> --
> 2.29.2
>

2021-11-08 18:07:00

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v0 08/42] Drivers: hv: vmbus: Check notifier registration return value

On Mon, Nov 08, 2021 at 12:39:14PM +0100, Borislav Petkov wrote:
> On Mon, Nov 08, 2021 at 11:16:37AM +0000, Wei Liu wrote:
> > On Mon, Nov 08, 2021 at 11:11:23AM +0100, Borislav Petkov wrote:
> > > From: Borislav Petkov <[email protected]>
> > >
> > > Avoid homegrown notifier registration checks.
> > >
> > > No functional changes.
> > >
> > > Signed-off-by: Borislav Petkov <[email protected]>
> > > Cc: [email protected]
> >
> > Acked-by: Wei Liu <[email protected]>
>
> Thanks.
>
> I assume your ack means, I can take the two through tip.

Yes please take them through tip. I should've been clearer on this.
Sorry.

Wei.

2021-11-08 18:58:37

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v0 35/42] sh: Check notifier registration return value

Hi Borislav,

On Mon, Nov 8, 2021 at 1:50 PM Borislav Petkov <[email protected]> wrote:
> From: Borislav Petkov <[email protected]>
>
> Avoid homegrown notifier registration checks.
>
> No functional changes.
>
> Signed-off-by: Borislav Petkov <[email protected]>

Thanks for your patch!

> --- a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
> +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
> @@ -1277,11 +1277,14 @@ static struct notifier_block sh7724_post_sleep_notifier = {
>
> static int __init sh7724_sleep_setup(void)
> {
> - atomic_notifier_chain_register(&sh_mobile_pre_sleep_notifier_list,
> - &sh7724_pre_sleep_notifier);
> + if (atomic_notifier_chain_register(&sh_mobile_pre_sleep_notifier_list,
> + &sh7724_pre_sleep_notifier))
> + pr_warn("SH7724 pre-sleep notifier already registered\n");
> +
> + if (atomic_notifier_chain_register(&sh_mobile_post_sleep_notifier_list,
> + &sh7724_post_sleep_notifier))
> + pr_warn("SH7724 pre-sleep notifier already registered\n");

Do you think these can actually fail?

>
> - atomic_notifier_chain_register(&sh_mobile_post_sleep_notifier_list,
> - &sh7724_post_sleep_notifier);
> return 0;
> }
> arch_initcall(sh7724_sleep_setup);

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-08 19:38:27

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 16/42] USB: Check notifier registration return value

On Mon, Nov 08, 2021 at 09:05:53AM -0500, Alan Stern wrote:
> This is a rather misleading description. The patch does exactly the
> opposite: It _adds_ a homegrown notifier registration check. (Homegrown
> in the sense that the check is made by the individual caller rather than
> being centralized in the routine being called.)

See the 0th message - there is a link to another example of what I mean
with "homegrown" but I see your point.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-08 20:45:50

by Sergey Shtylyov

[permalink] [raw]
Subject: Re: [PATCH v0 35/42] sh: Check notifier registration return value

On 08.11.2021 13:11, Borislav Petkov wrote:

> From: Borislav Petkov <[email protected]>
>
> Avoid homegrown notifier registration checks.
>
> No functional changes.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: [email protected]
> ---
> arch/sh/kernel/cpu/sh4a/setup-sh7724.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
> index 0d990ab1ba2a..8dfbb8149f66 100644
> --- a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
> +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
> @@ -1277,11 +1277,14 @@ static struct notifier_block sh7724_post_sleep_notifier = {
>
> static int __init sh7724_sleep_setup(void)
> {
> - atomic_notifier_chain_register(&sh_mobile_pre_sleep_notifier_list,
> - &sh7724_pre_sleep_notifier);
> + if (atomic_notifier_chain_register(&sh_mobile_pre_sleep_notifier_list,
> + &sh7724_pre_sleep_notifier))
> + pr_warn("SH7724 pre-sleep notifier already registered\n");
> +
> + if (atomic_notifier_chain_register(&sh_mobile_post_sleep_notifier_list,
> + &sh7724_post_sleep_notifier))
> + pr_warn("SH7724 pre-sleep notifier already registered\n");

s/pre/post/? :-)

>
> - atomic_notifier_chain_register(&sh_mobile_post_sleep_notifier_list,
> - &sh7724_post_sleep_notifier);
> return 0;
> }
> arch_initcall(sh7724_sleep_setup);

MBR, Sergey

2021-11-09 01:03:32

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v0 37/42] sparc: Check notifier registration return value

From: Borislav Petkov <[email protected]>
Date: Mon, 8 Nov 2021 11:11:52 +0100

> From: Borislav Petkov <[email protected]>
>
> Avoid homegrown notifier registration checks.
>
> No functional changes.
>
> Signed-off-by: Borislav Petkov <[email protected]>

Acked-by: David S. Miller <[email protected]>

2021-11-08 10:14:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 39/42] crypto: ccree - check notifier registration return value

From: Borislav Petkov <[email protected]>

Avoid homegrown notifier registration checks.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: [email protected]
---
drivers/crypto/ccree/cc_fips.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/ccree/cc_fips.c b/drivers/crypto/ccree/cc_fips.c
index 702aefc21447..de842da7d51c 100644
--- a/drivers/crypto/ccree/cc_fips.c
+++ b/drivers/crypto/ccree/cc_fips.c
@@ -146,7 +146,9 @@ int cc_fips_init(struct cc_drvdata *p_drvdata)
tasklet_init(&fips_h->tasklet, fips_dsr, (unsigned long)p_drvdata);
fips_h->drvdata = p_drvdata;
fips_h->nb.notifier_call = cc_ree_fips_failure;
- atomic_notifier_chain_register(&fips_fail_notif_chain, &fips_h->nb);
+
+ if (atomic_notifier_chain_register(&fips_fail_notif_chain, &fips_h->nb))
+ pr_warn("Failure notifier already registered\n");

cc_tee_handle_fips_error(p_drvdata);

--
2.29.2


2021-11-08 10:16:57

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 42/42] notifier: Return an error when callback is already registered

From: Borislav Petkov <[email protected]>

The notifier registration routine doesn't return a proper error value
when a callback has already been registered, leading people to track
whether that registration has happened at the call site:

https://lore.kernel.org/amd-gfx/[email protected]/

Which is unnecessary.

Return -EEXIST to signal that case so that callers can act accordingly.
Enforce callers to check the return value, leading to loud screaming
during build:

arch/x86/kernel/cpu/mce/core.c: In function ‘mce_register_decode_chain’:
arch/x86/kernel/cpu/mce/core.c:167:2: error: ignoring return value of \
‘blocking_notifier_chain_register’, declared with attribute warn_unused_result [-Werror=unused-result]
blocking_notifier_chain_register(&x86_mce_decoder_chain, nb);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Drop the WARN too, while at it.

Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Ayush Sawal <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Rohit Maheshwari <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Vinay Kumar Yadav <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
include/linux/notifier.h | 8 ++++----
kernel/notifier.c | 36 +++++++++++++++++++-----------------
2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 87069b8459af..45cc5a8d0fd8 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -141,13 +141,13 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);

#ifdef __KERNEL__

-extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
+extern int __must_check atomic_notifier_chain_register(struct atomic_notifier_head *nh,
struct notifier_block *nb);
-extern int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
+extern int __must_check blocking_notifier_chain_register(struct blocking_notifier_head *nh,
struct notifier_block *nb);
-extern int raw_notifier_chain_register(struct raw_notifier_head *nh,
+extern int __must_check raw_notifier_chain_register(struct raw_notifier_head *nh,
struct notifier_block *nb);
-extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
+extern int __must_check srcu_notifier_chain_register(struct srcu_notifier_head *nh,
struct notifier_block *nb);

extern int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
diff --git a/kernel/notifier.c b/kernel/notifier.c
index b8251dc0bc0f..451ef3f73ad2 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -20,13 +20,11 @@ BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
*/

static int notifier_chain_register(struct notifier_block **nl,
- struct notifier_block *n)
+ struct notifier_block *n)
{
while ((*nl) != NULL) {
- if (unlikely((*nl) == n)) {
- WARN(1, "double register detected");
- return 0;
- }
+ if (unlikely((*nl) == n))
+ return -EEXIST;
if (n->priority > (*nl)->priority)
break;
nl = &((*nl)->next);
@@ -134,10 +132,11 @@ static int notifier_call_chain_robust(struct notifier_block **nl,
*
* Adds a notifier to an atomic notifier chain.
*
- * Currently always returns zero.
+ * Returns 0 on success, %-EEXIST on error.
*/
-int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
- struct notifier_block *n)
+int __must_check
+atomic_notifier_chain_register(struct atomic_notifier_head *nh,
+ struct notifier_block *n)
{
unsigned long flags;
int ret;
@@ -216,10 +215,11 @@ NOKPROBE_SYMBOL(atomic_notifier_call_chain);
* Adds a notifier to a blocking notifier chain.
* Must be called in process context.
*
- * Currently always returns zero.
+ * Returns 0 on success, %-EEXIST on error.
*/
-int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
- struct notifier_block *n)
+int __must_check
+blocking_notifier_chain_register(struct blocking_notifier_head *nh,
+ struct notifier_block *n)
{
int ret;

@@ -335,10 +335,11 @@ EXPORT_SYMBOL_GPL(blocking_notifier_call_chain);
* Adds a notifier to a raw notifier chain.
* All locking must be provided by the caller.
*
- * Currently always returns zero.
+ * Returns 0 on success, %-EEXIST on error.
*/
-int raw_notifier_chain_register(struct raw_notifier_head *nh,
- struct notifier_block *n)
+int __must_check
+raw_notifier_chain_register(struct raw_notifier_head *nh,
+ struct notifier_block *n)
{
return notifier_chain_register(&nh->head, n);
}
@@ -406,10 +407,11 @@ EXPORT_SYMBOL_GPL(raw_notifier_call_chain);
* Adds a notifier to an SRCU notifier chain.
* Must be called in process context.
*
- * Currently always returns zero.
+ * Returns 0 on success, %-EEXIST on error.
*/
-int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
- struct notifier_block *n)
+int __must_check
+srcu_notifier_chain_register(struct srcu_notifier_head *nh,
+ struct notifier_block *n)
{
int ret;

--
2.29.2


2021-11-08 10:20:10

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH v0 00/42] notifiers: Return an error when callback is already registered

From: Borislav Petkov <[email protected]>

Hi all,

this is a huge patchset for something which is really trivial - it
changes the notifier registration routines to return an error value
if a notifier callback is already present on the respective list of
callbacks. For more details scroll to the last patch.

Everything before it is converting the callers to check the return value
of the registration routines and issue a warning, instead of the WARN()
notifier_chain_register() does now.

Before the last patch has been applied, though, that checking is a
NOP which would make the application of those patches trivial - every
maintainer can pick a patch at her/his discretion - only the last one
enables the build warnings and that one will be queued only after the
preceding patches have all been merged so that there are no build
warnings.

Due to the sheer volume of the patches, I have addressed the respective
patch and the last one, which enables the warning, with addressees for
each maintained area so as not to spam people unnecessarily.

If people prefer I carry some through tip, instead, I'll gladly do so -
your call.

And, if you think the warning messages need to be more precise, feel
free to adjust them before committing.

Thanks!

Cc: Arnd Bergmann <[email protected]>
Cc: Ayush Sawal <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Rohit Maheshwari <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Vinay Kumar Yadav <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Borislav Petkov (42):
x86: Check notifier registration return value
xen/x86: Check notifier registration return value
impi: Check notifier registration return value
clk: renesas: Check notifier registration return value
dca: Check notifier registration return value
firmware: Check notifier registration return value
drm/i915: Check notifier registration return value
Drivers: hv: vmbus: Check notifier registration return value
iio: proximity: cros_ec: Check notifier registration return value
leds: trigger: Check notifier registration return value
misc: Check notifier registration return value
ethernet: chelsio: Check notifier registration return value
power: reset: Check notifier registration return value
remoteproc: Check notifier registration return value
scsi: target: Check notifier registration return value
USB: Check notifier registration return value
drivers: video: Check notifier registration return value
drivers/xen: Check notifier registration return value
kernel/hung_task: Check notifier registration return value
rcu: Check notifier registration return value
tracing: Check notifier registration return value
net: fib_notifier: Check notifier registration return value
ASoC: soc-jack: Check notifier registration return value
staging: olpc_dcon: Check notifier registration return value
arch/um: Check notifier registration return value
alpha: Check notifier registration return value
bus: brcmstb_gisb: Check notifier registration return value
soc: bcm: brcmstb: pm: pm-arm: Check notifier registration return
value
arm64: Check notifier registration return value
soc/tegra: Check notifier registration return value
parisc: Check notifier registration return value
macintosh/adb: Check notifier registration return value
mips: Check notifier registration return value
powerpc: Check notifier registration return value
sh: Check notifier registration return value
s390: Check notifier registration return value
sparc: Check notifier registration return value
xtensa: Check notifier registration return value
crypto: ccree - check notifier registration return value
EDAC/altera: Check notifier registration return value
power: supply: ab8500: Check notifier registration return value
notifier: Return an error when callback is already registered

arch/alpha/kernel/setup.c | 5 +--
arch/arm64/kernel/setup.c | 6 ++--
arch/mips/kernel/relocate.c | 6 ++--
arch/mips/sgi-ip22/ip22-reset.c | 4 ++-
arch/mips/sgi-ip32/ip32-reset.c | 4 ++-
arch/parisc/kernel/pdc_chassis.c | 5 +--
arch/powerpc/kernel/setup-common.c | 12 ++++---
arch/s390/kernel/ipl.c | 4 ++-
arch/s390/kvm/kvm-s390.c | 7 ++--
arch/sh/kernel/cpu/sh4a/setup-sh7724.c | 11 +++---
arch/sparc/kernel/sstate.c | 6 ++--
arch/um/drivers/mconsole_kern.c | 6 ++--
arch/um/kernel/um_arch.c | 5 +--
arch/x86/kernel/cpu/mce/core.c | 3 +-
arch/x86/kernel/cpu/mce/dev-mcelog.c | 3 +-
arch/x86/kernel/setup.c | 7 ++--
arch/x86/xen/enlighten.c | 4 ++-
arch/xtensa/platforms/iss/setup.c | 3 +-
drivers/bus/brcmstb_gisb.c | 6 ++--
drivers/char/ipmi/ipmi_msghandler.c | 3 +-
drivers/clk/renesas/clk-div6.c | 4 ++-
drivers/clk/renesas/rcar-cpg-lib.c | 4 ++-
drivers/crypto/ccree/cc_fips.c | 4 ++-
drivers/dca/dca-core.c | 3 +-
drivers/edac/altera_edac.c | 6 ++--
drivers/firmware/arm_scmi/notify.c | 3 +-
drivers/firmware/google/gsmi.c | 6 ++--
drivers/gpu/drm/i915/gvt/scheduler.c | 6 ++--
drivers/hv/vmbus_drv.c | 4 +--
.../iio/proximity/cros_ec_mkbp_proximity.c | 3 +-
drivers/leds/trigger/ledtrig-activity.c | 6 ++--
drivers/leds/trigger/ledtrig-heartbeat.c | 6 ++--
drivers/leds/trigger/ledtrig-panic.c | 4 +--
drivers/macintosh/adbhid.c | 4 +--
drivers/misc/ibmasm/heartbeat.c | 3 +-
drivers/misc/pvpanic/pvpanic.c | 3 +-
.../chelsio/inline_crypto/chtls/chtls_main.c | 5 ++-
drivers/parisc/power.c | 5 +--
drivers/power/reset/ltc2952-poweroff.c | 6 ++--
drivers/power/supply/ab8500_charger.c | 8 ++---
drivers/remoteproc/qcom_common.c | 3 +-
drivers/remoteproc/qcom_sysmon.c | 4 ++-
drivers/remoteproc/remoteproc_core.c | 4 ++-
drivers/s390/char/con3215.c | 5 ++-
drivers/s390/char/con3270.c | 5 ++-
drivers/s390/char/sclp_con.c | 4 ++-
drivers/s390/char/sclp_vt220.c | 4 ++-
drivers/s390/char/zcore.c | 4 ++-
drivers/soc/bcm/brcmstb/pm/pm-arm.c | 5 +--
drivers/soc/tegra/ari-tegra186.c | 7 ++--
drivers/staging/olpc_dcon/olpc_dcon.c | 4 ++-
drivers/target/tcm_fc/tfc_conf.c | 4 ++-
drivers/usb/core/notify.c | 3 +-
drivers/video/console/dummycon.c | 3 +-
drivers/video/fbdev/hyperv_fb.c | 5 +--
drivers/xen/manage.c | 3 +-
drivers/xen/xenbus/xenbus_probe.c | 8 +++--
include/linux/notifier.h | 8 ++---
kernel/hung_task.c | 3 +-
kernel/notifier.c | 36 ++++++++++---------
kernel/rcu/tree_stall.h | 4 ++-
kernel/trace/trace.c | 4 +--
net/core/fib_notifier.c | 4 ++-
sound/soc/soc-jack.c | 3 +-
64 files changed, 222 insertions(+), 118 deletions(-)

--
2.29.2


2021-11-08 14:07:25

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered

Hi Borislav,

On Mon, Nov 8, 2021 at 11:13 AM Borislav Petkov <[email protected]> wrote:
> From: Borislav Petkov <[email protected]>
>
> The notifier registration routine doesn't return a proper error value
> when a callback has already been registered, leading people to track
> whether that registration has happened at the call site:
>
> https://lore.kernel.org/amd-gfx/[email protected]/
>
> Which is unnecessary.
>
> Return -EEXIST to signal that case so that callers can act accordingly.
> Enforce callers to check the return value, leading to loud screaming
> during build:
>
> arch/x86/kernel/cpu/mce/core.c: In function ‘mce_register_decode_chain’:
> arch/x86/kernel/cpu/mce/core.c:167:2: error: ignoring return value of \
> ‘blocking_notifier_chain_register’, declared with attribute warn_unused_result [-Werror=unused-result]
> blocking_notifier_chain_register(&x86_mce_decoder_chain, nb);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Drop the WARN too, while at it.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>

Thanks for your patch!

> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -141,13 +141,13 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
>
> #ifdef __KERNEL__
>
> -extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
> +extern int __must_check atomic_notifier_chain_register(struct atomic_notifier_head *nh,
> struct notifier_block *nb);
> -extern int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
> +extern int __must_check blocking_notifier_chain_register(struct blocking_notifier_head *nh,
> struct notifier_block *nb);
> -extern int raw_notifier_chain_register(struct raw_notifier_head *nh,
> +extern int __must_check raw_notifier_chain_register(struct raw_notifier_head *nh,
> struct notifier_block *nb);
> -extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
> +extern int __must_check srcu_notifier_chain_register(struct srcu_notifier_head *nh,
> struct notifier_block *nb);

I think the addition of __must_check is overkill, leading to the
addition of useless error checks and message printing. Many callers
call this where it cannot fail, and where nothing can be done in the
very unlikely event that the call would ever start to fail.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-08 14:17:08

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v0 00/42] notifiers: Return an error when callback is already registered

On Mon, Nov 08, 2021 at 11:19:24AM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Hi all,
>
> this is a huge patchset for something which is really trivial - it
> changes the notifier registration routines to return an error value
> if a notifier callback is already present on the respective list of
> callbacks. For more details scroll to the last patch.
>
> Everything before it is converting the callers to check the return value
> of the registration routines and issue a warning, instead of the WARN()
> notifier_chain_register() does now.

What reason is there for moving the check into the callers? It seems
like pointless churn. Why not add the error return code, change the
WARN to pr_warn, and leave the callers as they are? Wouldn't that end
up having exactly the same effect?

For that matter, what sort of remedial action can a caller take if the
return code is -EEXIST? Is there any point in forcing callers to check
the return code if they can't do anything about it?

> Before the last patch has been applied, though, that checking is a
> NOP which would make the application of those patches trivial - every
> maintainer can pick a patch at her/his discretion - only the last one
> enables the build warnings and that one will be queued only after the
> preceding patches have all been merged so that there are no build
> warnings.

Why should there be _any_ build warnings? The real problem occurs when
a notifier callback is added twice, not when a caller fails to check the
return code. Double-registration is not the sort of thing that can be
detected at build time.

Alan Stern

> Due to the sheer volume of the patches, I have addressed the respective
> patch and the last one, which enables the warning, with addressees for
> each maintained area so as not to spam people unnecessarily.
>
> If people prefer I carry some through tip, instead, I'll gladly do so -
> your call.
>
> And, if you think the warning messages need to be more precise, feel
> free to adjust them before committing.
>
> Thanks!

2021-11-08 14:21:31

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered

On Mon, Nov 08, 2021 at 03:07:03PM +0100, Geert Uytterhoeven wrote:
> I think the addition of __must_check is overkill, leading to the
> addition of useless error checks and message printing.

See the WARN in notifier_chain_register() - it will already do "message
printing".

> Many callers call this where it cannot fail, and where nothing can
> be done in the very unlikely event that the call would ever start to
> fail.

This is an attempt to remove this WARN() hack in
notifier_chain_register() and have the function return a proper error
value instead of this "Currently always returns zero." which is bad
design.

Some of the registration functions around the tree check that retval and
some don't. So if "it cannot fail" those registration either should not
return a value or callers should check that return value - what we have
now doesn't make a whole lot of sense.

Oh, and then fixing this should avoid stuff like:

+ if (notifier_registered == false) {
+ mce_register_decode_chain(&amdgpu_bad_page_nb);
+ notifier_registered = true;
+ }

from propagating in the code.

The other idea I have is to add another indirection in
notifier_chain_register() which will check the *proper* return value of
a lower level __notifier_chain_register() and then issue that warning.
That would definitely avoid touch so many call sites.

Bottom line is: what we have now needs cleaning up.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-08 14:24:50

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 00/42] notifiers: Return an error when callback is already registered

On Mon, Nov 08, 2021 at 09:17:03AM -0500, Alan Stern wrote:
> What reason is there for moving the check into the callers? It seems
> like pointless churn. Why not add the error return code, change the
> WARN to pr_warn, and leave the callers as they are? Wouldn't that end
> up having exactly the same effect?
>
> For that matter, what sort of remedial action can a caller take if the
> return code is -EEXIST? Is there any point in forcing callers to check
> the return code if they can't do anything about it?

See my reply to Geert from just now:

https://lore.kernel.org/r/[email protected]

I guess I can add another indirection to notifier_chain_register() and
avoid touching all the call sites.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-08 14:36:01

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 00/42] notifiers: Return an error when callback is already registered

On Mon, Nov 08, 2021 at 03:24:39PM +0100, Borislav Petkov wrote:
> I guess I can add another indirection to notifier_chain_register() and
> avoid touching all the call sites.

IOW, something like this below.

This way I won't have to touch all the callsites and the registration
routines would still return a proper value instead of returning 0
unconditionally.

---
diff --git a/kernel/notifier.c b/kernel/notifier.c
index b8251dc0bc0f..04f08b2ef17f 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -19,14 +19,12 @@ BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
* are layered on top of these, with appropriate locking added.
*/

-static int notifier_chain_register(struct notifier_block **nl,
- struct notifier_block *n)
+static int __notifier_chain_register(struct notifier_block **nl,
+ struct notifier_block *n)
{
while ((*nl) != NULL) {
- if (unlikely((*nl) == n)) {
- WARN(1, "double register detected");
- return 0;
- }
+ if (unlikely((*nl) == n))
+ return -EEXIST;
if (n->priority > (*nl)->priority)
break;
nl = &((*nl)->next);
@@ -36,6 +34,18 @@ static int notifier_chain_register(struct notifier_block **nl,
return 0;
}

+static int notifier_chain_register(struct notifier_block **nl,
+ struct notifier_block *n)
+{
+ int ret = __notifier_chain_register(nl, n);
+
+ if (ret == -EEXIST)
+ WARN(1, "double register of notifier callback %ps detected",
+ n->notifier_call);
+
+ return ret;
+}
+
static int notifier_chain_unregister(struct notifier_block **nl,
struct notifier_block *n)
{


--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-08 15:33:24

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered

Hi Borislav,

On Mon, Nov 8, 2021 at 3:21 PM Borislav Petkov <[email protected]> wrote:
> On Mon, Nov 08, 2021 at 03:07:03PM +0100, Geert Uytterhoeven wrote:
> > I think the addition of __must_check is overkill, leading to the
> > addition of useless error checks and message printing.
>
> See the WARN in notifier_chain_register() - it will already do "message
> printing".

I mean the addition of useless error checks and message printing _to
the callers_.

> > Many callers call this where it cannot fail, and where nothing can
> > be done in the very unlikely event that the call would ever start to
> > fail.
>
> This is an attempt to remove this WARN() hack in
> notifier_chain_register() and have the function return a proper error
> value instead of this "Currently always returns zero." which is bad
> design.
>
> Some of the registration functions around the tree check that retval and
> some don't. So if "it cannot fail" those registration either should not
> return a value or callers should check that return value - what we have
> now doesn't make a whole lot of sense.

With __must_check callers are required to check, even if they know
it cannot fail.

> Oh, and then fixing this should avoid stuff like:
>
> + if (notifier_registered == false) {
> + mce_register_decode_chain(&amdgpu_bad_page_nb);
> + notifier_registered = true;
> + }
>
> from propagating in the code.

That's unrelated to the addition of __must_check.

I'm not against returning proper errors codes. I'm against forcing
callers to check things that cannot fail and to add individual error
printing to each and every caller.

Note that in other areas, we are moving in the other
direction, to a centralized printing of error messages,
cfr. e.g. commit 7723f4c5ecdb8d83 ("driver core: platform: Add an
error message to platform_get_irq*()").

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-08 15:59:15

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered

On Mon, Nov 08, 2021 at 04:25:47PM +0100, Geert Uytterhoeven wrote:
> I'm not against returning proper errors codes. I'm against forcing
> callers to check things that cannot fail and to add individual error
> printing to each and every caller.

If you're against checking things at the callers, then the registration
function should be void. IOW, those APIs are not optimally designed atm.

> Note that in other areas, we are moving in the other direction,
> to a centralized printing of error messages, cfr. e.g. commit
> 7723f4c5ecdb8d83 ("driver core: platform: Add an error message to
> platform_get_irq*()").

Yes, thus my other idea to add a lower level __notifier_chain_register()
to do the checking.

I'll see if I can convert those notifier registration functions to
return void, in the process. But let's see what the others think first.

Thanks for taking the time.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-08 16:12:38

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered

Hi Borislav,

On Mon, Nov 8, 2021 at 4:59 PM Borislav Petkov <[email protected]> wrote:
> On Mon, Nov 08, 2021 at 04:25:47PM +0100, Geert Uytterhoeven wrote:
> > I'm not against returning proper errors codes. I'm against forcing
> > callers to check things that cannot fail and to add individual error
> > printing to each and every caller.
>
> If you're against checking things at the callers, then the registration
> function should be void. IOW, those APIs are not optimally designed atm.

Returning void is the other extreme ;-)

There are 3 levels (ignoring BUG_ON()/panic () inside the callee):
1. Return void: no one can check success or failure,
2. Return an error code: up to the caller to decide,
3. Return a __must_check error code: every caller must check.

I'm in favor of 2, as there are several places where it cannot fail.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-08 16:23:26

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v0 00/42] notifiers: Return an error when callback is already registered

On Mon, 8 Nov 2021 15:35:50 +0100
Borislav Petkov <[email protected]> wrote:

> On Mon, Nov 08, 2021 at 03:24:39PM +0100, Borislav Petkov wrote:
> > I guess I can add another indirection to notifier_chain_register() and
> > avoid touching all the call sites.
>
> IOW, something like this below.
>
> This way I won't have to touch all the callsites and the registration
> routines would still return a proper value instead of returning 0
> unconditionally.

I prefer this method.

Question, how often does this warning trigger? Is it common to see in
development?

-- Steve

2021-11-08 16:30:07

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 00/42] notifiers: Return an error when callback is already registered

On Mon, Nov 08, 2021 at 11:23:13AM -0500, Steven Rostedt wrote:
> Question, how often does this warning trigger? Is it common to see in
> development?

Yeah, haven't seen it myself yet.

But we hashed it out over IRC. :-)

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-12 18:32:39

by Cristian Marussi

[permalink] [raw]
Subject: Re: [PATCH v0 39/42] crypto: ccree - check notifier registration return value

On Mon, Nov 08, 2021 at 11:11:54AM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Avoid homegrown notifier registration checks.
>
> No functional changes.
>

Hi Borislav,

> Signed-off-by: Borislav Petkov <[email protected]>
> Cc: [email protected]
> ---
> drivers/crypto/ccree/cc_fips.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/ccree/cc_fips.c b/drivers/crypto/ccree/cc_fips.c
> index 702aefc21447..de842da7d51c 100644
> --- a/drivers/crypto/ccree/cc_fips.c
> +++ b/drivers/crypto/ccree/cc_fips.c
> @@ -146,7 +146,9 @@ int cc_fips_init(struct cc_drvdata *p_drvdata)
> tasklet_init(&fips_h->tasklet, fips_dsr, (unsigned long)p_drvdata);
> fips_h->drvdata = p_drvdata;
> fips_h->nb.notifier_call = cc_ree_fips_failure;
> - atomic_notifier_chain_register(&fips_fail_notif_chain, &fips_h->nb);
> +
> + if (atomic_notifier_chain_register(&fips_fail_notif_chain, &fips_h->nb))
> + pr_warn("Failure notifier already registered\n");
>

Looking at the implementation of atomic_notifier_chain_register() and
its internal helper down below, I can see that atomic_notifier_chain_register()
ALWAYS return 0 (O_o) and anyway there is a WARN() in the notifiier core already
to alert of double registrations.

What is the aim of this patch ?
It's not clear from the commit message what are you trying to achieve.
Am I missing something ?
(Is it to circumvent some static checker to avoid this false positive ?
just guessing...)

Thanks,
Cristian

-----8<----------
static int notifier_chain_register(struct notifier_block **nl,
struct notifier_block *n)
{
while ((*nl) != NULL) {
if (unlikely((*nl) == n)) {
WARN(1, "double register detected");
return 0;
}
if (n->priority > (*nl)->priority)
break;
nl = &((*nl)->next);
}
n->next = *nl;
rcu_assign_pointer(*nl, n);
return 0;
}

/**
* atomic_notifier_chain_register - Add notifier to an atomic notifier chain
* @nh: Pointer to head of the atomic notifier chain
* @n: New entry in notifier chain
*
* Adds a notifier to an atomic notifier chain.
*
* Currently always returns zero.
*/
int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
struct notifier_block *n)
{
unsigned long flags;
int ret;

spin_lock_irqsave(&nh->lock, flags);
ret = notifier_chain_register(&nh->head, n);
spin_unlock_irqrestore(&nh->lock, flags);
return ret;
}
EXPORT_SYMBOL_GPL(atomic_notifier_chain_register);
------

2021-11-12 18:48:29

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v0 39/42] crypto: ccree - check notifier registration return value

On Fri, Nov 12, 2021 at 06:32:33PM +0000, Cristian Marussi wrote:
> Looking at the implementation of atomic_notifier_chain_register() and
> its internal helper down below, I can see that atomic_notifier_chain_register()
> ALWAYS return 0 (O_o) and anyway there is a WARN() in the notifiier core already
> to alert of double registrations.

You can ignore it - we agreed (at least no one has complained against it
yet :)) on doing this instead:

https://lore.kernel.org/r/[email protected]

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette