git: f87aebe64c96 - stable/14 - tcp: refactor register_tcp_functions_as_names()
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 03 Aug 2024 22:55:23 UTC
The branch stable/14 has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=f87aebe64c9678916b4c22edb99793adbfd6fcea commit f87aebe64c9678916b4c22edb99793adbfd6fcea Author: Michael Tuexen <tuexen@FreeBSD.org> AuthorDate: 2024-07-13 10:22:25 +0000 Commit: Michael Tuexen <tuexen@FreeBSD.org> 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];