From nobody Wed Jul 12 03:10:55 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4R12nD2hpcz4h3bl; Wed, 12 Jul 2023 03:10:56 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4R12nD1HQvz49WC; Wed, 12 Jul 2023 03:10:56 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1689131456; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=MrKXVY76+OvCxvnWWt2v9L2tU/brK4mbYIKARwzXXGc=; b=oJcVhmPAXhundK0ghKGglNJ5U0ypR0x/zc5JWD6nbymD1HwKjttWtuCHEPsprVW1vVP+yF nAyfjAjjjXU55SNs7Y/r//TPylyLhfxPEHY64Lz+NUZTtJHfS457q0fspmSiCId/IcgcTB tcnhQ2cs2x5ShuHiRDcMVOlWXJDG+hbpRp3UKi1afEPqwFD+Y+r2D5BW2AZcd0vuOAb6/y DFKiFmRnAigho/RlYRGwOG6g8qJLxs9f81Nf5blKnQPsCT31v4RcCrvt9gQJDlah2ruHGi Hq+fOX57+Y6zL9yKYdswZPJ3s455aOEhRVMcChFIKthlq6rUQp41CrEtJ/qPPw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1689131456; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=MrKXVY76+OvCxvnWWt2v9L2tU/brK4mbYIKARwzXXGc=; b=CNNpw8uFLoqK+SDPi8ab4YlOx4o5N/FggljRc/d//Jr+cWhs098WGJTf0YSH9jP9sJednR I+1N2l7H5c1COZ0OhIgSZdg8Q4xiVzYoUYIkwbxDa2A9JdX8koYKST/5TXHlRIRzI1LEvt RMjUl0w4LGLVz8F7Jykm+49wRBH6tSlp6RV7aCH7b3r/FNVmiWzDxs/dJQS710eiohtsJv UUYk6IzRM8+vQgkiTmIatk1bjXYsXI6hqWxWWruXMNmLLF7Bgp/KRV7nmhZZoNfwwI1Xsq ebGhUJA5EsC9ndflaCHPk8Cl8g0E85NaY17rY6wfNjkTnXbV+hd4HKxepiKhfw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1689131456; a=rsa-sha256; cv=none; b=wPG01jiiWu/bYCx4N5C/YM1OnTYF0Peu1kNZQDFMnMzK506VG4hkQgcvyKHH5SnG09t9FB 4XGWeZw2na9SM3KIfp9fBdrHPvDdpqmtuZAsGftx+KuCWlLq2531OSa6wmuEXsDa1/ld/4 llmYCIKVoB9A873LU7mMrwF7nNEcPG8IMioU1jszFi90WEK1OPnILHKVQpChvzmU1C3zKo O0jgPP57Z7DRD2KnH03z30d1n9ugPHJjaph2gwvQYbRznN9W8bOEl5AYMk8z7x22Mvp82G iPvh4YyXzvysYB/kY16fFXxaKAg1ioHqYL9VO8zVZTu/QilWrFjH0cthv639gw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4R12nD0NxWzsc1; Wed, 12 Jul 2023 03:10:56 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 36C3At9S010431; Wed, 12 Jul 2023 03:10:55 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 36C3AtY3010430; Wed, 12 Jul 2023 03:10:55 GMT (envelope-from git) Date: Wed, 12 Jul 2023 03:10:55 GMT Message-Id: <202307120310.36C3AtY3010430@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kyle Evans Subject: git: b39545f7bc38 - stable/13 - wg: fix a number of issues with module load failure handling List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kevans X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: b39545f7bc38ec9865ac9cb4e703a6344ad8310b Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=b39545f7bc38ec9865ac9cb4e703a6344ad8310b commit b39545f7bc38ec9865ac9cb4e703a6344ad8310b Author: Kyle Evans AuthorDate: 2023-06-21 18:56:58 +0000 Commit: Kyle Evans CommitDate: 2023-07-11 15:05:45 +0000 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 (cherry picked from commit b08ee10c0646e683cd03c9e28f537d9a7ba306af) --- sys/dev/wg/if_wg.c | 28 +++++++++------------------- sys/dev/wg/wg_cookie.c | 9 ++++++++- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/sys/dev/wg/if_wg.c b/sys/dev/wg/if_wg.c index 8ba1d229da4f..e8a258edc4da 100644 --- a/sys/dev/wg/if_wg.c +++ b/sys/dev/wg/if_wg.c @@ -2986,39 +2986,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 @@ -3036,10 +3024,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 diff --git a/sys/dev/wg/wg_cookie.c b/sys/dev/wg/wg_cookie.c index 16fa5d7fb52d..6ff9325c6613 100644 --- a/sys/dev/wg/wg_cookie.c +++ b/sys/dev/wg/wg_cookie.c @@ -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