Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 23 Oct 2025 11:18:18 GMT
From:      Michael Tuexen <tuexen@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 6626f4e1a8f7 - stable/14 - tcp: cleanup of syncache_expand()
Message-ID:  <202510231118.59NBIIt6031795@gitrepo.freebsd.org>
index | next in thread | raw e-mail 

The branch stable/14 has been updated by tuexen:

URL: https://cgit.FreeBSD.org/src/commit/?id=6626f4e1a8f779cf454cea899df1220a08c833d6

commit 6626f4e1a8f779cf454cea899df1220a08c833d6
Author:     Michael Tuexen <tuexen@FreeBSD.org>
AuthorDate: 2025-10-17 05:40:02 +0000
Commit:     Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2025-10-23 11:17:51 +0000

    tcp: cleanup of syncache_expand()
    
    * Consistently free the string after unlocking the sch, if possible.
    * Remove the failure handling in case of sc != NULL, since this is
      not possible anymore.
    * Remove the use of goto and instead return 0 in the three cases.
    The only change in behavior is that in three out of the four cases,
    where 0 is returned, *lsop is not set to NULL anymore. So the behavior
    is now consistent and also documented in a comment. The current in
    tree callers only look at *lsop, if and only if syncache_expand()
    returns 1.
    
    Reviewed by:            Peter Lei
    Sponsored by:           Netflix, Inc.
    Differential Revision:  https://reviews.freebsd.org/D52948
    
    (cherry picked from commit aafdbf83b926519cb47de8f16a1a40c1ef3c84b5)
---
 sys/netinet/tcp_syncache.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c
index 1c585378dc5b..077c3a5f6ef3 100644
--- a/sys/netinet/tcp_syncache.c
+++ b/sys/netinet/tcp_syncache.c
@@ -1056,6 +1056,8 @@ abort:
  *
  * On syncache_socket() success the newly created socket
  * has its underlying inp locked.
+ *
+ * *lsop is updated, if and only if 1 is returned.
  */
 int
 syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
@@ -1103,12 +1105,14 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 				 * a valid syncookie.
 				 */
 				SCH_UNLOCK(sch);
-				if ((s = tcp_log_addrs(inc, th, NULL, NULL)))
+				if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
 					log(LOG_DEBUG, "%s; %s: Spurious ACK, "
 					    "segment rejected "
 					    "(syncookies disabled)\n",
 					    s, __func__);
-				goto failed;
+					free(s, M_TCPLOG);
+				}
+				return (0);
 			}
 			if (sch->sch_last_overflow <
 			    time_uptime - SYNCOOKIE_LIFETIME) {
@@ -1117,12 +1121,14 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 				 * don't even check for a valid syncookie.
 				 */
 				SCH_UNLOCK(sch);
-				if ((s = tcp_log_addrs(inc, th, NULL, NULL)))
+				if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
 					log(LOG_DEBUG, "%s; %s: Spurious ACK, "
 					    "segment rejected "
 					    "(no syncache entry)\n",
 					    s, __func__);
-				goto failed;
+					free(s, M_TCPLOG);
+				}
+				return (0);
 			}
 			SCH_UNLOCK(sch);
 		}
@@ -1135,11 +1141,13 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 			sc = &scs;
 			TCPSTAT_INC(tcps_sc_recvcookie);
 		} else {
-			if ((s = tcp_log_addrs(inc, th, NULL, NULL)))
+			if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
 				log(LOG_DEBUG, "%s; %s: Segment failed "
 				    "SYNCOOKIE authentication, segment rejected "
 				    "(probably spoofed)\n", s, __func__);
-			goto failed;
+				free(s, M_TCPLOG);
+			}
+			return (0);
 		}
 #if defined(IPSEC_SUPPORT) || defined(TCP_SIGNATURE)
 		/* If received ACK has MD5 signature, check it. */
@@ -1213,9 +1221,9 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 				    "%s; %s: SEG.TSval %u < TS.Recent %u, "
 				    "segment dropped\n", s, __func__,
 				    to->to_tsval, sc->sc_tsreflect);
-				free(s, M_TCPLOG);
 			}
 			SCH_UNLOCK(sch);
+			free(s, M_TCPLOG);
 			return (-1);  /* Do not send RST */
 		}
 
@@ -1232,7 +1240,6 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 				    "expected, segment processed normally\n",
 				    s, __func__);
 				free(s, M_TCPLOG);
-				s = NULL;
 			}
 		}
 
@@ -1319,16 +1326,6 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 	if (sc != &scs)
 		syncache_free(sc);
 	return (1);
-failed:
-	if (sc != NULL) {
-		TCPSTATES_DEC(TCPS_SYN_RECEIVED);
-		if (sc != &scs)
-			syncache_free(sc);
-	}
-	if (s != NULL)
-		free(s, M_TCPLOG);
-	*lsop = NULL;
-	return (0);
 }
 
 static struct socket *
help

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202510231118.59NBIIt6031795>