From nobody Mon Nov 29 12:49:20 2021 X-Original-To: dev-commits-src-main@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 5D5B718CA3BD; Mon, 29 Nov 2021 12:49:22 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4J2lXw29Tqz4fD4; Mon, 29 Nov 2021 12:49:20 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 2A2EA22AC5; Mon, 29 Nov 2021 12:49:20 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1ATCnKrv025206; Mon, 29 Nov 2021 12:49:20 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1ATCnKZA025205; Mon, 29 Nov 2021 12:49:20 GMT (envelope-from git) Date: Mon, 29 Nov 2021 12:49:20 GMT Message-Id: <202111291249.1ATCnKZA025205@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: 147bf5e930b7 - main - tcp: Don't try to upgrade a read lock just for logging List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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: 147bf5e930b70638a73059e19c97d3f3a9a227fd Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1638190160; 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=MUZ2+IM65nzTmReYFcIgeaEZqf1jXI6c5gReEboWvpo=; b=sucQH2WdroHLV5KkK1tqhBS3VnTCG9+tXyKddScYW/s4AxIWHvnJkX5qOH+A2zj/HNt3xI J1+xThGflik0CjcYa/s9xM7J0axZ50b6lteueENNdgvGHKgBa7TVeA09BArO+bB4NBUcHC jU5lxKs109D2og5CvPhADg3LJtPPir320Xn/YJDOnNIkMmb7X2dVVF/bUlYRODtgKJvCrg o7rXg94iBRxrBTpCjSFhl//5Xo5GyPuQAIG6/U/xdhvtmF1dztGzBlI14vdse5fVWYohhw 8izl+jS4WjJ8BpdjA33pGIG6KmF01hUVOxUYM1+pz+V2qoIHkAagasYjJOlECQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1638190160; a=rsa-sha256; cv=none; b=CMK0XfisKxSqTvmECZI8ESNOA1KTwBA4xNUnqFUYu5nYb8+9zEGGy0b+8l+HiuT33CKCcj UDHbVJ6mlwAKKTvn2UhyvMB1DzTW3DQX1KCqJLv/zr4B8VCzgVI3g04PmZa0IKe73aV6TG HCgBbB2wuuOMfhEC4GahOiEl0XdAGC6ToL2hfd2xIKIJ26P6OMJpEemLGdjVy2aOUNN+OR EV+XnT2UKc3/x6VnF2TYQealQM5Ydv53hfl4Gu0VH/dubAzXxBuLve+dPVdmFZAUwUJQJC 8c789hIvtY4+dxx+uQCf++4rc20mgmGjG99j2b8b036HF+ZVgw7u1trPXByYNw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=147bf5e930b70638a73059e19c97d3f3a9a227fd commit 147bf5e930b70638a73059e19c97d3f3a9a227fd Author: Michael Tuexen AuthorDate: 2021-11-26 10:53:00 +0000 Commit: Michael Tuexen CommitDate: 2021-11-29 12:48:40 +0000 tcp: Don't try to upgrade a read lock just for logging Reviewed by: glebius, lstewart, rrs Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D33098 --- sys/netinet/tcp_subr.c | 54 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index 5fa16f43f28b..34cc291dc274 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -101,6 +101,9 @@ __FBSDID("$FreeBSD$"); #endif #include +#ifdef INVARIANTS +#define TCPSTATES +#endif #include #include #include @@ -1755,9 +1758,12 @@ tcp_respond(struct tcpcb *tp, void *ipgen, struct tcphdr *th, struct mbuf *m, int isipv6; #endif /* INET6 */ int optlen, tlen, win, ulen; - bool incl_opts, lock_upgraded; + bool incl_opts; uint16_t port; int output_ret; +#ifdef INVARIANTS + int thflags = th->th_flags; +#endif KASSERT(tp != NULL || m != NULL, ("tcp_respond: tp and m both NULL")); NET_EPOCH_ASSERT(); @@ -2088,18 +2094,12 @@ tcp_respond(struct tcpcb *tp, void *ipgen, struct tcphdr *th, struct mbuf *m, TCP_PROBE3(debug__output, tp, th, m); if (flags & TH_RST) TCP_PROBE5(accept__refused, NULL, NULL, m, tp, nth); - lock_upgraded = false; lgb = NULL; if ((tp != NULL) && (tp->t_logstate != TCP_LOG_STATE_OFF)) { - union tcp_log_stackspecific log; - struct timeval tv; - - lock_upgraded = !INP_WLOCKED(inp) && INP_TRY_UPGRADE(inp); - /* - *`If we don't already own the write lock and can't upgrade, - * just don't log the event, but still send the response. - */ if (INP_WLOCKED(inp)) { + union tcp_log_stackspecific log; + struct timeval tv; + memset(&log.u_bbr, 0, sizeof(log.u_bbr)); log.u_bbr.inhpts = tp->t_inpcb->inp_in_hpts; log.u_bbr.ininput = tp->t_inpcb->inp_in_input; @@ -2107,8 +2107,36 @@ tcp_respond(struct tcpcb *tp, void *ipgen, struct tcphdr *th, struct mbuf *m, log.u_bbr.pkts_out = tp->t_maxseg; log.u_bbr.timeStamp = tcp_get_usecs(&tv); log.u_bbr.delivered = 0; - lgb = tcp_log_event_(tp, nth, NULL, NULL, TCP_LOG_OUT, ERRNO_UNK, - 0, &log, false, NULL, NULL, 0, &tv); + lgb = tcp_log_event_(tp, nth, NULL, NULL, TCP_LOG_OUT, + ERRNO_UNK, 0, &log, false, NULL, NULL, 0, &tv); + } else { + /* + * We can not log the packet, since we only own the + * read lock, but a write lock is needed. The read lock + * is not upgraded to a write lock, since only getting + * the read lock was done intentionally to improve the + * handling of SYN flooding attacks. + * This happens only for pure SYN segments received in + * the initial CLOSED state, or received in a more + * advanced state than listen and the UDP encapsulation + * port is unexpected. + * The incoming SYN segments do not really belong to + * the TCP connection and the handling does not change + * the state of the TCP connection. Therefore, the + * sending of the RST segments is not logged. Please + * note that also the incoming SYN segments are not + * logged. + * + * The following code ensures that the above description + * is and stays correct. + */ + KASSERT((thflags & (TH_ACK|TH_SYN)) == TH_SYN && + (tp->t_state == TCPS_CLOSED || + (tp->t_state > TCPS_LISTEN && tp->t_port != port)), + ("%s: Logging of TCP segment with flags 0x%b and " + "UDP encapsulation port %u skipped in state %s", + __func__, thflags, PRINT_TH_FLAGS, + ntohs(port), tcpstates[tp->t_state])); } } @@ -2129,8 +2157,6 @@ tcp_respond(struct tcpcb *tp, void *ipgen, struct tcphdr *th, struct mbuf *m, #endif if (lgb != NULL) lgb->tlb_errno = output_ret; - if (lock_upgraded) - INP_DOWNGRADE(inp); } /*