From nobody Sat Aug 3 22:55:23 2024 X-Original-To: dev-commits-src-branches@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 4Wbyhq27QVz5RVXr; Sat, 03 Aug 2024 22:55:23 +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 4Wbyhq1MMZz4N5s; Sat, 3 Aug 2024 22:55:23 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1722725723; 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=7FFLo18a8we/nPxyLOWy5M2/fZHGBS4xdDwHr3t7Rx8=; b=A7SLpmy7QRvpOHafR4i93Op+0PmaGgOUWuIEtSg6scQuvSFbrwqMLAs5DEHkwYvi6SKEBN B3V4rfd466geh3XvKb+8oxZUWqRC3/APF27dsQ85PaKYM3SpMTnzgTIf/cUh5qM6A+qGBs rWydlXKBXpjUTZTEhvchbTy1C7CuYKdeWgpO/p2SqjkPdfQA6+IzAKmSMyrt+8EZSeUNpq yRasWR9nLAQq/QSbFc0JoA7dPCZYEBNAn/oBcDL/BPeWn1jcs5E3UAa0Hga6X9qN5itKVc Idfy92GwOScyHMUFS+3gIYAg9uw2NbA6j6xVo7u4r/67vATWBIz3lyCf8/OdWA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1722725723; a=rsa-sha256; cv=none; b=RtM0oke+r92BgBiiXPTtshjc6QhysodgAkaCQ9UsAs8DRX7EkxdcC0PHD4247luUKLiSNA 0AqENXcM6x0720MKH2YPaS0K6uGrWh+4HHWEkSIhoIPDCs9JB2FxQybLtMzTZmnQxrw29N 2yfwAM07bkSPWJAmGCPn+1wtIxnmeKnqSYUhUrmNAVzcDUva7ac9EiW8Lev923xK0L/F9z buqOQ3Qd0fxAdVeRu7xaHZvB+yl5M40mrTypn+pi6P1PlnIMJO0A3njpdEW+J10EvtO29b J9Rt3qEDG4EDt5vdI0r9Jx15x/mzMRX1DRB0uXbhoSJp89PaS6rOcqGsxJVEjg== 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=1722725723; 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=7FFLo18a8we/nPxyLOWy5M2/fZHGBS4xdDwHr3t7Rx8=; b=MAN9zps4M8IiA1KJuOjvd4SDvXkpZfp6kkQCKOvUWY5rfAuRfstQzcWrjyqecVvyX9WpUH SkzGxmyqTB3mMYJzPRU41gqubmEC+zabE8NPkGwJXUYH1+ahh1hA2y2iJ+p42pt19BHbKH qlyDePbysksgvUniklJL5LOf2LBYSw/2h42JOIyqyrbffyPliRR2+CA8d7gXUD0zuyI+sg lO0WmHA85nXY/ET7jF1obYRWfCXAqD3jHieSixh+LfcGjPZoQh3BtCFq05UeJqSkoSSK7e zJYcm8irYjpT9k0VN/lIOIhkkIvP4FNu+C+0g3qSJbibzNp1dyfVAzoBbMJIog== 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 4Wbyhq0yjvzWS9; Sat, 3 Aug 2024 22:55:23 +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 473MtNSZ078265; Sat, 3 Aug 2024 22:55:23 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 473MtNsW078262; Sat, 3 Aug 2024 22:55:23 GMT (envelope-from git) Date: Sat, 3 Aug 2024 22:55:23 GMT Message-Id: <202408032255.473MtNsW078262@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Michael Tuexen Subject: git: f87aebe64c96 - stable/14 - tcp: refactor register_tcp_functions_as_names() List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-branches@freebsd.org Sender: owner-dev-commits-src-branches@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/stable/14 X-Git-Reftype: branch X-Git-Commit: f87aebe64c9678916b4c22edb99793adbfd6fcea Auto-Submitted: auto-generated The branch stable/14 has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=f87aebe64c9678916b4c22edb99793adbfd6fcea commit f87aebe64c9678916b4c22edb99793adbfd6fcea Author: Michael Tuexen AuthorDate: 2024-07-13 10:22:25 +0000 Commit: Michael Tuexen CommitDate: 2024-08-03 22:54:44 +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 Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D45947 (cherry picked from commit 859f0f0d6bf753c16caa0103a541c69fb6bd5e37) --- 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 8ae9018f1e5d..6feb1916bb35 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -1215,9 +1215,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, @@ -1225,71 +1225,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 b8278bbf2644..a8621563e190 100644 --- a/sys/netinet/tcp_var.h +++ b/sys/netinet/tcp_var.h @@ -638,6 +638,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];