[PATCH net] can: isotp: check register_netdevice_notifier() error in module init

[PATCH net] can: isotp: check register_netdevice_notifier() error in module init

[Minhong He]

Check the return value of register_netdevice_notifier() after a successful
can_proto_register(). If notifier registration fails, unregister the CAN
protocol before returning the error.

Fixes: 8d0caedb7596 ("can: bcm/raw/isotp: use per module netdevice notifier")

Signed-off-by: Minhong He <heminhong@kylinos.cn>
---
 net/can/isotp.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 54becaf6898f..65e890cafd7a 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1908,12 +1908,18 @@ static __init int isotp_module_init(void)
 	pr_info("can: isotp protocol (max_pdu_size %d)\n", max_pdu_size);
 
 	err = can_proto_register(&isotp_can_proto);
-	if (err < 0)
+	if (err < 0) {
 		pr_err("can: registration of isotp protocol failed %pe\n", ERR_PTR(err));
-	else
-		register_netdevice_notifier(&canisotp_notifier);
+		return err;
+	}
 
-	return err;
+	err = register_netdevice_notifier(&canisotp_notifier);
+	if (err) {
+		can_proto_unregister(&isotp_can_proto);
+		return err;
+	}
+
+	return 0;
 }
 
 static __exit void isotp_module_exit(void)
-- 
2.25.1

Re: [PATCH net] can: isotp: check register_netdevice_notifier() error in module init

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Registering the CAN protocol before completing all fallible initialization steps creates a race window leading to a Use-After-Free crash if initialization fails.
--

--- Patch [1]: [PATCH net] can: isotp: check register_netdevice_notifier() error in module init ---
commit 5f2f7a58360e9a2c18b5d6a9a724f468f79227d1
Author: Minhong He <heminhong@kylinos.cn>

can: isotp: check register_netdevice_notifier() error in module init

This commit adds a check for the return value of register_netdevice_notifier()
in the isotp module initialization. If the notifier registration fails, it
now unregisters the CAN protocol and returns the error.

> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 54becaf6898f1..65e890cafd7a9 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -1908,12 +1908,18 @@ static __init int isotp_module_init(void)
>  	pr_info("can: isotp protocol (max_pdu_size %d)\n", max_pdu_size);
>  
>  	err = can_proto_register(&isotp_can_proto);
> -	if (err < 0)
> +	if (err < 0) {
>  		pr_err("can: registration of isotp protocol failed %pe\n", ERR_PTR(err));
> -	else
> -		register_netdevice_notifier(&canisotp_notifier);
> +		return err;
> +	}
>  
> -	return err;
> +	err = register_netdevice_notifier(&canisotp_notifier);
> +	if (err) {
> +		can_proto_unregister(&isotp_can_proto);
> +		return err;
> +	}

[Severity: High]
Does this error path introduce a use-after-free risk if userspace creates a
socket before the module initialization completes?

If can_proto_register() succeeds, the protocol is immediately exposed to
userspace. If a concurrent thread creates an ISOTP socket before
register_netdevice_notifier() fails, the socket will hold pointers to the
module's protocol operations.

When the module initialization aborts and returns an error, the kernel will
free the module memory. While can_proto_unregister() removes the protocol
from the registry, it does not revoke already-created sockets. As a result,
any subsequent operations on the existing socket will dereference freed
module memory.

Would it be safer to move the fallible register_netdevice_notifier() before
can_proto_register() to avoid exposing the protocol before initialization
is guaranteed to succeed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728031038.76524-1-heminhong@kylinos.cn?part=1