Re: [PATCH] staging: rtl8192u: cleanup proc fs entries upon exit

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Sun, Feb 20, 2022 at 03:15:53PM -0800, Tong Zhang wrote:
> proc fs entries need to be removed when module is removed, otherwise
> when we try to insert the module again, kernel will complain
> 
> [  493.068012] proc_dir_entry 'net/ieee80211' already registered
> [  493.271973]  proc_mkdir+0x18/0x20
> [  493.272136]  ieee80211_debug_init+0x28/0xde8 [r8192u_usb]
> [  493.272404]  rtl8192_usb_module_init+0x10/0x161 [r8192u_usb]
> 
> [   13.910616] proc_dir_entry 'net/rtl819xU' already registered
> [   13.918931]  proc_mkdir+0x18/0x20
> [   13.919098]  rtl8192_usb_module_init+0x142/0x16d [r8192u_usb]
> 
> Signed-off-by: Tong Zhang <ztong0001@xxxxxxxxx>
> ---

This is a partial fix but there is a lot wrong with both the init() and
exit() function.  It's not hard to just fix everything and it saves
time.

Here is how to write Free the Last thing style error handling for init()
and when you finish writing the error handling code then the exit()
function is just a matter of cut and paste.

The rules are: 1) Free the last successful allocation.  2) Every
function must have a matching release function. 3) Every function must
clean up after itself.  No partial allocations.  4) Name your labels
with descriptive names to say what the goto does.

	ret = ieee80211_debug_init();
	if (ret) {

Here nothing has been allocated.  Nothing to clean up.  Just return an
error code.

		pr_err("ieee80211_debug_init() failed %d\n", ret);
		return ret;
	}

	ret = ieee80211_crypto_init();
	if (ret) {

The last successful allocation was ieee80211_debug_init().  So we need
to goto debug_exit;
		pr_err("ieee80211_crypto_init() failed %d\n", ret);
		goto debug_exit;
	}

	ret = ieee80211_crypto_tkip_init();
	if (ret) {
The last successful allocation was ieee80211_crypto_init() so we need to
goto crypto_exit;
		pr_err("ieee80211_crypto_tkip_init() failed %d\n", ret);
		goto crypto_exit;
	}

Etc.  At the end of the function it prints the pr_info() even though
usb_register(&rtl8192_usb_driver); can fail.  It needs to do:

	ret = usb_register(&rtl8192_usb_driver);
	if (ret)
		goto crypto_wep_exit;

	pr_info("\nLinux kernel driver for RTL8192 based WLAN cards\n");
	pr_info("Copyright (c) 2007-2008, Realsil Wlan\n");

	return 0;

crypto_wep_exit:
	ieee80211_crypto_wep_exit();
other_stuff:
	free_other_stuff();
crypto_exit:
	ieee80211_crypto_deinit();
debug_exit:
	ieee80211_debug_exit();

	return ret;
}

Now you copy and paste the cleanup block, delete the labels, and add a
usb_deregister() to create the rtl8192_usb_module_exit() function.

static void __exit rtl8192_usb_module_exit(void)
{
	usb_deregister(&rtl8192_usb_driver);
	ieee80211_crypto_wep_exit();
	free_other_stuff();
	ieee80211_crypto_deinit();
	ieee80211_debug_exit();
}

Remember to replace "free other stuff" with a the correct code.  ;)

regards,
dan carpenter