wg: fix a number of issues with module load failure handling
If MOD_LOAD fails, then MOD_UNLOAD will be called to unwind module state, but wg_module_init() will have already deinitialized everything it needs to in a manner that renders it unsafe to call MOD_UNLOAD after (e.g., freed zone not reset to NULL, wg_osd_jail_slot not reset to 0). Let's simply stop trying to handle freeing everything in wg_module_init() to simplify it; let the subsequent MOD_UNLOAD deal with it, and let's make that robust against partially-constructed state. jhb@ notes that MOD_UNLOAD being called if MOD_LOAD fails is kind of an anomaly that doesn't match other paradigms in the kernel; e.g., if device_attach() fails, we don't invoke device_detach(). It's likely that a future commit will revert this and instead stop calling MOD_UNLOAD if MOD_LOAD fails, expecting modules to clean up after themselves in MOD_LOAD upon failure. Some other modules already do this and may see similar problems to the wg module (see: carp). The proper fix is decidedly a bit too invasive to do this close to 14 branching, and it requires auditing all kmods (base + ports) for potential leaks. PR: 272089 Reviewed by: emaste MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D40708
This commit is contained in:
parent
ad9f4e6351
commit
b08ee10c06
@ -2984,39 +2984,27 @@ static inline bool wg_run_selftests(void) { return true; }
|
||||
static int
|
||||
wg_module_init(void)
|
||||
{
|
||||
int ret = ENOMEM;
|
||||
|
||||
int ret;
|
||||
osd_method_t methods[PR_MAXMETHOD] = {
|
||||
[PR_METHOD_REMOVE] = wg_prison_remove,
|
||||
};
|
||||
|
||||
if ((wg_packet_zone = uma_zcreate("wg packet", sizeof(struct wg_packet),
|
||||
NULL, NULL, NULL, NULL, 0, 0)) == NULL)
|
||||
goto free_none;
|
||||
return (ENOMEM);
|
||||
ret = crypto_init();
|
||||
if (ret != 0)
|
||||
goto free_zone;
|
||||
return (ret);
|
||||
ret = cookie_init();
|
||||
if (ret != 0)
|
||||
goto free_crypto;
|
||||
return (ret);
|
||||
|
||||
wg_osd_jail_slot = osd_jail_register(NULL, methods);
|
||||
|
||||
ret = ENOTRECOVERABLE;
|
||||
if (!wg_run_selftests())
|
||||
goto free_all;
|
||||
return (ENOTRECOVERABLE);
|
||||
|
||||
return (0);
|
||||
|
||||
free_all:
|
||||
osd_jail_deregister(wg_osd_jail_slot);
|
||||
cookie_deinit();
|
||||
free_crypto:
|
||||
crypto_deinit();
|
||||
free_zone:
|
||||
uma_zdestroy(wg_packet_zone);
|
||||
free_none:
|
||||
return (ret);
|
||||
}
|
||||
|
||||
static void
|
||||
@ -3034,10 +3022,12 @@ wg_module_deinit(void)
|
||||
VNET_LIST_RUNLOCK();
|
||||
NET_EPOCH_WAIT();
|
||||
MPASS(LIST_EMPTY(&wg_list));
|
||||
osd_jail_deregister(wg_osd_jail_slot);
|
||||
if (wg_osd_jail_slot != 0)
|
||||
osd_jail_deregister(wg_osd_jail_slot);
|
||||
cookie_deinit();
|
||||
crypto_deinit();
|
||||
uma_zdestroy(wg_packet_zone);
|
||||
if (wg_packet_zone != NULL)
|
||||
uma_zdestroy(wg_packet_zone);
|
||||
}
|
||||
|
||||
static int
|
||||
|
@ -55,6 +55,7 @@ struct ratelimit {
|
||||
struct callout rl_gc;
|
||||
LIST_HEAD(, ratelimit_entry) rl_table[RATELIMIT_SIZE];
|
||||
size_t rl_table_num;
|
||||
bool rl_initialized;
|
||||
};
|
||||
|
||||
static void precompute_key(uint8_t *,
|
||||
@ -102,7 +103,8 @@ cookie_deinit(void)
|
||||
#ifdef INET6
|
||||
ratelimit_deinit(&ratelimit_v6);
|
||||
#endif
|
||||
uma_zdestroy(ratelimit_zone);
|
||||
if (ratelimit_zone != NULL)
|
||||
uma_zdestroy(ratelimit_zone);
|
||||
}
|
||||
|
||||
void
|
||||
@ -350,16 +352,21 @@ ratelimit_init(struct ratelimit *rl)
|
||||
for (i = 0; i < RATELIMIT_SIZE; i++)
|
||||
LIST_INIT(&rl->rl_table[i]);
|
||||
rl->rl_table_num = 0;
|
||||
rl->rl_initialized = true;
|
||||
}
|
||||
|
||||
static void
|
||||
ratelimit_deinit(struct ratelimit *rl)
|
||||
{
|
||||
if (!rl->rl_initialized)
|
||||
return;
|
||||
mtx_lock(&rl->rl_mtx);
|
||||
callout_stop(&rl->rl_gc);
|
||||
ratelimit_gc(rl, true);
|
||||
mtx_unlock(&rl->rl_mtx);
|
||||
mtx_destroy(&rl->rl_mtx);
|
||||
|
||||
rl->rl_initialized = false;
|
||||
}
|
||||
|
||||
static void
|
||||
|
Loading…
Reference in New Issue
Block a user