[PATCH net] net: mctp i3c: clean up notifier and buses if driver register fails

Myeonghun Pak posted 1 patch 1 week, 4 days ago
There is a newer version of this series
drivers/net/mctp/mctp-i3c.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH net] net: mctp i3c: clean up notifier and buses if driver register fails
Posted by Myeonghun Pak 1 week, 4 days ago
mctp_i3c_mod_init() registers the I3C bus notifier and then walks the
existing buses with i3c_for_each_bus_locked(mctp_i3c_bus_add_new, NULL)
before registering the I3C device driver.  If i3c_driver_register()
fails, the function returns the error directly, leaving the notifier
registered and every mctp_i3c_bus object created for the existing buses
allocated.  The notifier is left pointing into the module that failed to
load and the bus list is leaked.

Mirror the module exit path on this failure: unregister the notifier and
tear down the buses that were added before returning the error.

Fixes: c8755b29b58e ("mctp i3c: MCTP I3C driver")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/net/mctp/mctp-i3c.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mctp/mctp-i3c.c b/drivers/net/mctp/mctp-i3c.c
index 6d2bbae747..677e77e917 100644
--- a/drivers/net/mctp/mctp-i3c.c
+++ b/drivers/net/mctp/mctp-i3c.c
@@ -740,9 +740,14 @@ static __init int mctp_i3c_mod_init(void)

 	rc = i3c_driver_register(&mctp_i3c_driver);
 	if (rc < 0)
-		return rc;
+		goto err_unregister_notifier;

 	return 0;
+
+err_unregister_notifier:
+	i3c_unregister_notifier(&mctp_i3c_notifier);
+	mctp_i3c_bus_remove_all();
+	return rc;
 }

 static __exit void mctp_i3c_mod_exit(void)
--
2.53.0
Re: [PATCH net] net: mctp i3c: clean up notifier and buses if driver register fails
Posted by Jeremy Kerr 1 week, 3 days ago
Hi,

> mctp_i3c_mod_init() registers the I3C bus notifier and then walks the
> existing buses with i3c_for_each_bus_locked(mctp_i3c_bus_add_new, NULL)
> before registering the I3C device driver.  If i3c_driver_register()
> fails, the function returns the error directly, leaving the notifier
> registered and every mctp_i3c_bus object created for the existing buses
> allocated.  The notifier is left pointing into the module that failed to
> load and the bus list is leaked.
> 
> Mirror the module exit path on this failure: unregister the notifier and
> tear down the buses that were added before returning the error.

Looks good, but we probably want to remove the unneeded
i3c_driver_unregister in the notify registration failure path too.

Also, you're missing Horms from the CC; more out of curiosity, but
how did you generate the CC list here?

Cheers,


Jeremy
Re: [PATCH net] net: mctp i3c: clean up notifier and buses if driver register fails
Posted by Myeonghun Pak 1 week, 3 days ago
Hi Jeremy,

Thanks, agreed. I'll remove the unmatched i3c_driver_unregister() from
the notifier registration failure path and send a v2.

I generated the recipient list from my Linux v7.0 checkout using:

  scripts/get_maintainer.pl drivers/net/mctp/mctp-i3c.c

Since I passed the source path rather than the patch, it only picked up
the MAINTAINERS entries and missed Simon from the trailers of the Fixes
commit. For v2, I'll run get_maintainer.pl on the patch from a
full-history kernel tree and add Simon to CC.

Cheers,
Myeonghun