From nobody Thu Dec 7 22:42:14 2023 X-Original-To: dev-commits-src-all@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 4SmTmQ65hxz53kGg; Thu, 7 Dec 2023 22:42:14 +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 4SmTmQ5Q5Xz3PyJ; Thu, 7 Dec 2023 22:42:14 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1701988934; 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=MV0coqcPxR864R/0fBHUaTJV9kfVXqz5geHup3p3ewU=; b=gZ2Oyi6esTnhFNotTTpUksF8+vvcL/BCytwYB57xdvrZxnV/tfasQFyIRpNlgfI6l/p6ET Mz3L8adBkV7liUeCeGSeVHpy9a9GsVLZ0t4ucfBr2Yf/RF3NiyhBsCvpgKlMv7lArD69eb e1EeriL3w2f8t2TGaxn+MgBMXY8OFOeZXn0PdxfuZ9T1i683CLlebXrPEsXexOmFtMyEPp xYu0mZLcxyAi/u2YBWlUaNdcXI/doRYliCG3uz1wqzkshiy1mnG8knHRY/6oPF6uOKT2z+ FHOF66l0gJhQ8y3322guJByuMal9hy1JUujWMzxUisS6uObbE3ePytIFzhJ7IA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1701988934; a=rsa-sha256; cv=none; b=U+YD/MjqDnPRd+F5yxwI/9L43jAJf2EX2XinFnCwfm3qcj0muQrlRsMu8OB0lPgcAZ9sfW 6YO07LWozDyJmyXB5qIR9cAmIb5UV1vFAY9PykoLL3yd6fv6fWvwn0261d/1NH36JcXS+3 7tMmJy9UQ+2hsyOCI7wrEo7YiUJIwhsfJhrsnIzm0pQW7m/coaflY31CPjArnrjnUvPbqz +J95epRy09ynHfnkNsITdsUzSVRI9qcwcpkkZf9t89ipZbab3Z3svMx4wm6+FTBLpl41/A 9lPzTcqXtWKJy7cSKP6iPYQwZLA5okDKFLpbAEMgNde7Kth43nbn13bNn+FkBQ== 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=1701988934; 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=MV0coqcPxR864R/0fBHUaTJV9kfVXqz5geHup3p3ewU=; b=DdV+E6TqqJoxM9nN+1pUCD6Yk0TB+AQKb1/efUuOS29AT+6cHBOBjJWwgnLvjfzh+2SdKp DMG0avT3/Q2P3YcOORLcCywslpz5CTzVMAortFM9Zz5xAJAudWZStSwJbpysi0EYnWTCCr UK39ekgdcFHatquRLjQLi9miTR4fselu8bLDQiZ4Ds1zE2G0mhMTaWItj39UynMiXaRIbM 8j/gWlzU7tmI1WkiGXi25RTEiMOwmVb2pfDvgeAU7bATTefEp62vv0L0tE3YgiDz0+sZrA RSDuFU8pYDb1eO7RFsErtjwcgppH8x/tjX0yuraWha54uIENTcu7a2DpqXJ6vw== 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 4SmTmQ44wXzw9G; Thu, 7 Dec 2023 22:42:14 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 3B7MgE6o074708; Thu, 7 Dec 2023 22:42:14 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3B7MgEix074697; Thu, 7 Dec 2023 22:42:14 GMT (envelope-from git) Date: Thu, 7 Dec 2023 22:42:14 GMT Message-Id: <202312072242.3B7MgEix074697@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Gleb Smirnoff Subject: git: 3f46be6acadd - main - tcp_hpts: let tcp_hpts_init() set a random CPU only once List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: glebius X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 3f46be6acadd5d660acde67d9d4c80137f424b70 Auto-Submitted: auto-generated The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=3f46be6acadd5d660acde67d9d4c80137f424b70 commit 3f46be6acadd5d660acde67d9d4c80137f424b70 Author: Gleb Smirnoff AuthorDate: 2023-12-07 22:41:43 +0000 Commit: Gleb Smirnoff CommitDate: 2023-12-07 22:41:43 +0000 tcp_hpts: let tcp_hpts_init() set a random CPU only once After d2ef52ef3dee the tcp_hpts_init() function can be called multiple times on a tcpcb if it is switched there and back between two TCP stacks. First, this makes existing assertion in tcp_hpts_init() incorrect. Second, it creates possibility to change a randomly set t_hpts_cpu to a different random value, while a tcpcb is already in the HPTS wheel, triggering other assertions later in tcp_hptsi(). The best approach here would be to work on the stacks to really clear a tcpcb out of HPTS wheel in tfb_tcp_fb_fini, draining the IHPTS_MOVING state. But that's pretty intrusive change, so let's just get back to the old logic (pre d2ef52ef3dee) where t_hpts_cpu was set to a random value only once in a CPU lifetime and a newly switched stack inherits t_hpts_cpu from the previous stack. Reviewed by: rrs, tuexen Differential Revision: https://reviews.freebsd.org/D42946 Reported-by: syzbot+fab29fe1ab089c52998d@syzkaller.appspotmail.com Reported-by: syzbot+ca5f2aa0fda15dcfe6d7@syzkaller.appspotmail.com Fixes: 2b3a77467dd3d74a7170f279fb25f9736b46ef8a --- sys/netinet/tcp_hpts.c | 16 ++++++++++++---- sys/netinet/tcp_subr.c | 3 +++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/sys/netinet/tcp_hpts.c b/sys/netinet/tcp_hpts.c index e88a3f24dfcc..f1b729c249c6 100644 --- a/sys/netinet/tcp_hpts.c +++ b/sys/netinet/tcp_hpts.c @@ -542,15 +542,23 @@ tcp_hpts_release(struct tcpcb *tp) } /* - * Initialize newborn tcpcb to get ready for use with HPTS. + * Initialize tcpcb to get ready for use with HPTS. We will know which CPU + * is preferred on the first incoming packet. Before that avoid crowding + * a single CPU with newborn connections and use a random one. + * This initialization is normally called on a newborn tcpcb, but potentially + * can be called once again if stack is switched. In that case we inherit CPU + * that the previous stack has set, be it random or not. In extreme cases, + * e.g. syzkaller fuzzing, a tcpcb can already be in HPTS in IHPTS_MOVING state + * and has never received a first packet. */ void tcp_hpts_init(struct tcpcb *tp) { - tp->t_hpts_cpu = hpts_random_cpu(); - tp->t_lro_cpu = HPTS_CPU_NONE; - MPASS(!(tp->t_flags2 & TF2_HPTS_CPU_SET)); + if (__predict_true(tp->t_hpts_cpu == HPTS_CPU_NONE)) { + tp->t_hpts_cpu = hpts_random_cpu(); + MPASS(!(tp->t_flags2 & TF2_HPTS_CPU_SET)); + } } /* diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index c79cadd04944..be38280aef0a 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -2274,6 +2274,9 @@ tcp_newtcpcb(struct inpcb *inp) /* All mbuf queue/ack compress flags should be off */ tcp_lro_features_off(tp); + tp->t_hpts_cpu = HPTS_CPU_NONE; + tp->t_lro_cpu = HPTS_CPU_NONE; + callout_init_rw(&tp->t_callout, &inp->inp_lock, CALLOUT_RETURNUNLOCKED); for (int i = 0; i < TT_N; i++) tp->t_timers[i] = SBT_MAX;