From nobody Sat Jul 13 10:28:08 2024 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 4WLl6J4M2fz5QTby; Sat, 13 Jul 2024 10:28:08 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WLl6J3ZTZz45BN; Sat, 13 Jul 2024 10:28:08 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1720866488; 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=3Z/uL9gMWQDtevD672io7SFgbVTcC2Xr75taT4Ohrp4=; b=Zl2pUkayxsa7ek926u3NDtPTpB7cIUoKhyCTSI6x7+/xE1oP/PdFd6Yp2Bq3tpZ9y5Kfus ZxljgLS1Uw11cRS6Yv4GzjLJxIQO+zRrC9IKM+AefSOz45L0LmJx17Kjv8ASbj/JEKvCmo Jp1n1r0kr8veEeCys/m1+/h7W4spdxYuEeeov2V2m2LbvRcGMlYf1KsP/cOwxTN42sEKrE P8LyqA3MxzKGoP35HqLt5tSvdXAVKvm3GGXi+RN4mGHdwMJ9//lmQ3n1h5ocF+grRdLK/D S/BVmX6BGokhqHa/Bk2GohcHF58dHszhnoUtfkMkQZgley/NBHBziLFegMmXpg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1720866488; a=rsa-sha256; cv=none; b=EESVQnC2QFCm7piRNAEWg+ejkwS0d7AjDBQX0eIYN2f0CxM2lwIxH9ehnekTy8HK3BTWBT NHgGvo5H+flntdvJuPShD+Fj+KQsEsvs+RRRXstxlux+oLvBViKUAvLAxp07tdQnLA28+f +ELw/R5Vfx/7t/brfCCO9yeknYf0jNGyw7QUaOniOAquCqQ8nOTUPMGkLDxcT/IhOaz3G9 sWLDbsCz7k9XeDqb/eCZScjwC2Hl9SbdEKYv7mE9XITjT3a/OmhGCBSq+Rm9ek+hlkfBwA iXP2nyEuaHMy1pj/fKKqDA9DMH/MMSuD52jBkvSDTcLXjtXa2nNxtxX7e8czDA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1720866488; 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=3Z/uL9gMWQDtevD672io7SFgbVTcC2Xr75taT4Ohrp4=; b=UI/nnKu2mxcsTgMP9fNlFB+bbF4tiCU9g0MjdRr513uoAopkRBWzkAyrYF3tNB73sOPf7m jvaICXW4I0NZdyC5BuwEFFmKyHoXObkO3Nh/EYWF4O4GVp+8FukWypHMXjWA7/cXMdNVE5 5JRWd8Qf5VFHkDe/9kdkdR3maXZMDBR74zb3ssUfkUri0ROJXwfRGPXXoE0Xz0brLkchYs CEq0pm6kqezydJelY145G5YJsu+MQxyxFr9BMVkWg/SjjLDM+mQXQyDx1hFemCirSkY38V 8C5wNx8eMjqM1IWoZLCBbzeUfJv6xI9oCEFhKz7lhDr5ouEKwtS7Aa4WyPM56Q== 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 4WLl6J2pBTzMfR; Sat, 13 Jul 2024 10:28:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 46DAS8e8043813; Sat, 13 Jul 2024 10:28:08 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 46DAS8We043810; Sat, 13 Jul 2024 10:28:08 GMT (envelope-from git) Date: Sat, 13 Jul 2024 10:28:08 GMT Message-Id: <202407131028.46DAS8We043810@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Michael Tuexen Subject: git: 859f0f0d6bf7 - main - tcp: refactor register_tcp_functions_as_names() 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: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: tuexen X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 859f0f0d6bf753c16caa0103a541c69fb6bd5e37 Auto-Submitted: auto-generated The branch main has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=859f0f0d6bf753c16caa0103a541c69fb6bd5e37 commit 859f0f0d6bf753c16caa0103a541c69fb6bd5e37 Author: Michael Tuexen AuthorDate: 2024-07-13 10:22:25 +0000 Commit: Michael Tuexen CommitDate: 2024-07-13 10:22:25 +0000 tcp: refactor register_tcp_functions_as_names() Refactor register_tcp_functions_as_names() such that either all or no (in error cases) registrations happen atomically (while holding the tcp_function_lock write lock). Also ensure that the TCP function block is not already registered. This avoids situations, where some registrations were performed and then they were removed without holding a lock in between or checking ref counts. Reviewed by: cc MFC after: 1 week Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D45947 --- share/man/man9/tcp_functions.9 | 25 +++++++++++--- sys/netinet/tcp_subr.c | 78 ++++++++++++++++++++++-------------------- sys/netinet/tcp_var.h | 3 ++ 3 files changed, 64 insertions(+), 42 deletions(-) diff --git a/share/man/man9/tcp_functions.9 b/share/man/man9/tcp_functions.9 index 1e0616e03a9f..8ba7f21c978c 100644 --- a/share/man/man9/tcp_functions.9 +++ b/share/man/man9/tcp_functions.9 @@ -23,7 +23,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd June 6, 2024 +.Dd July 13, 2024 .Dt TCP_FUNCTIONS 9 .Os .Sh NAME @@ -37,6 +37,7 @@ .Ft int .Fn register_tcp_functions_as_name "struct tcp_function_block *blk" \ "const char *name" "int wait" +.Ft int .Fn register_tcp_functions_as_names "struct tcp_function_block *blk" \ "int wait" "const char *names[]" "int *num_names" .Ft int @@ -112,6 +113,7 @@ argument. The .Fa num_names argument provides a pointer to the number of names. +This number must not exceed TCP_FUNCTION_NAME_NUM_MAX. This function will either succeed in registering all of the names in the array, or none of the names in the array. On failure, the @@ -328,8 +330,11 @@ must be prepared to wait until all connections have stopped using the specified TCP stack. .Sh ERRORS The -.Fn register_tcp_functions -function will fail if: +.Fn register_tcp_functions , +.Fn register_tcp_functions_as_name , +and +.Fn register_tcp_functions_as_names +functions will fail if: .Bl -tag -width Er .It Bq Er EINVAL Any of the members of the @@ -338,7 +343,19 @@ argument are set incorrectly. .It Bq Er ENOMEM The function could not allocate memory for its internal data. .It Bq Er EALREADY -A function block is already registered with the same name. +The +.Fa blk +is already registered or a function block is already registered with the same +name. +.El +Additionally, +.Fn register_tcp_functions_as_names +will fail if: +.Bl -tag -width Er +.It Bq Er E2BIG +The number of names pointed to by the +.Fa num_names +argument is larger than TCP_FUNCTION_NAME_NUM_MAX. .El The .Fn deregister_tcp_functions diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index b4f605534d59..5743d1bbd100 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -1172,9 +1172,9 @@ int register_tcp_functions_as_names(struct tcp_function_block *blk, int wait, const char *names[], int *num_names) { - struct tcp_function *n; + struct tcp_function *f[TCP_FUNCTION_NAME_NUM_MAX]; struct tcp_function_set fs; - int error, i; + int error, i, num_registered; KASSERT(names != NULL, ("%s: Called with NULL name list", __func__)); KASSERT(*num_names > 0, @@ -1182,71 +1182,73 @@ register_tcp_functions_as_names(struct tcp_function_block *blk, int wait, KASSERT(rw_initialized(&tcp_function_lock), ("%s: called too early", __func__)); + if (*num_names > TCP_FUNCTION_NAME_NUM_MAX) { + /* Too many names. */ + *num_names = 0; + return (E2BIG); + } if ((blk->tfb_tcp_output == NULL) || (blk->tfb_tcp_do_segment == NULL) || (blk->tfb_tcp_ctloutput == NULL) || (blk->tfb_tcp_handoff_ok == NULL) || (strlen(blk->tfb_tcp_block_name) == 0)) { - /* - * These functions are required and you - * need a name. - */ + /* These functions are required and a name is needed. */ *num_names = 0; return (EINVAL); } - if (blk->tfb_flags & TCP_FUNC_BEING_REMOVED) { - *num_names = 0; - return (EINVAL); + for (i = 0; i < *num_names; i++) { + f[i] = malloc(sizeof(struct tcp_function), M_TCPFUNCTIONS, wait); + if (f[i] == NULL) { + while (--i >= 0) + free(f[i], M_TCPFUNCTIONS); + *num_names = 0; + return (ENOMEM); + } } + num_registered = 0; + rw_wlock(&tcp_function_lock); + if (find_tcp_fb_locked(blk, NULL) != NULL) { + /* A TCP function block can only be registered once. */ + error = EALREADY; + goto cleanup; + } + if (blk->tfb_flags & TCP_FUNC_BEING_REMOVED) { + error = EINVAL; + goto cleanup; + } refcount_init(&blk->tfb_refcnt, 0); blk->tfb_id = atomic_fetchadd_int(&next_tcp_stack_id, 1); for (i = 0; i < *num_names; i++) { - n = malloc(sizeof(struct tcp_function), M_TCPFUNCTIONS, wait); - if (n == NULL) { - error = ENOMEM; - goto cleanup; - } - n->tf_fb = blk; - (void)strlcpy(fs.function_set_name, names[i], sizeof(fs.function_set_name)); - rw_wlock(&tcp_function_lock); if (find_tcp_functions_locked(&fs) != NULL) { /* Duplicate name space not allowed */ - rw_wunlock(&tcp_function_lock); - free(n, M_TCPFUNCTIONS); error = EALREADY; goto cleanup; } - (void)strlcpy(n->tf_name, names[i], sizeof(n->tf_name)); - TAILQ_INSERT_TAIL(&t_functions, n, tf_next); + f[i]->tf_fb = blk; + (void)strlcpy(f[i]->tf_name, names[i], sizeof(f[i]->tf_name)); + TAILQ_INSERT_TAIL(&t_functions, f[i], tf_next); tcp_fb_cnt++; - rw_wunlock(&tcp_function_lock); + num_registered++; } + rw_wunlock(&tcp_function_lock); return (0); cleanup: - /* - * Deregister the names we just added. Because registration failed - * for names[i], we don't need to deregister that name. - */ - *num_names = i; - rw_wlock(&tcp_function_lock); - while (--i >= 0) { - TAILQ_FOREACH(n, &t_functions, tf_next) { - if (!strncmp(n->tf_name, names[i], - TCP_FUNCTION_NAME_LEN_MAX)) { - TAILQ_REMOVE(&t_functions, n, tf_next); - tcp_fb_cnt--; - n->tf_fb = NULL; - free(n, M_TCPFUNCTIONS); - break; - } + /* Remove the entries just added. */ + for (i = 0; i < *num_names; i++) { + if (i < num_registered) { + TAILQ_REMOVE(&t_functions, f[i], tf_next); + tcp_fb_cnt--; } + f[i]->tf_fb = NULL; + free(f[i], M_TCPFUNCTIONS); } rw_wunlock(&tcp_function_lock); + *num_names = num_registered; return (error); } diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h index 8330966c2c3f..1f48297c2b0a 100644 --- a/sys/netinet/tcp_var.h +++ b/sys/netinet/tcp_var.h @@ -634,6 +634,9 @@ struct tcp_function_block { uint8_t tfb_id; }; +/* Maximum number of names each TCP function block can be registered with. */ +#define TCP_FUNCTION_NAME_NUM_MAX 8 + struct tcp_function { TAILQ_ENTRY(tcp_function) tf_next; char tf_name[TCP_FUNCTION_NAME_LEN_MAX];