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
MFC after:		3 days
Sponsored by:		Netflix, Inc.
Differential Revision:	https://reviews.freebsd.org/D52948

1 files changed, 15 insertions, 18 deletions

diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c
index f842a5678fa1..be20fb44a820 100644
--- a/sys/netinet/tcp_syncache.c
+++ b/sys/netinet/tcp_syncache.c
@@ -1046,6 +1046,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,
@@ -1094,12 +1096,14 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 				 */
 				SCH_UNLOCK(sch);
 				TCPSTAT_INC(tcps_sc_spurcookie);
-				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) {
@@ -1109,12 +1113,14 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 				 */
 				SCH_UNLOCK(sch);
 				TCPSTAT_INC(tcps_sc_spurcookie);
-				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);
 		}
@@ -1128,11 +1134,13 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 			TCPSTAT_INC(tcps_sc_recvcookie);
 		} else {
 			TCPSTAT_INC(tcps_sc_failcookie);
-			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. */
@@ -1206,9 +1214,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 */
 		}
 
@@ -1225,7 +1233,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;
 			}
 		}
 
@@ -1312,16 +1319,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 *