From nobody Tue Jan 10 02:05:04 2023 X-Original-To: jail@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 4NrYzk6Y4hz2p1ZS for ; Tue, 10 Jan 2023 02:05:06 +0000 (UTC) (envelope-from glebius@freebsd.org) Received: from glebi.us (glebi.us [162.251.186.162]) by mx1.freebsd.org (Postfix) with ESMTP id 4NrYzj4JXWz3jx0; Tue, 10 Jan 2023 02:05:05 +0000 (UTC) (envelope-from glebius@freebsd.org) Authentication-Results: mx1.freebsd.org; dkim=none; spf=softfail (mx1.freebsd.org: 162.251.186.162 is neither permitted nor denied by domain of glebius@freebsd.org) smtp.mailfrom=glebius@freebsd.org; dmarc=none Received: by glebi.us (Postfix, from userid 1000) id 0BE96C075; Mon, 9 Jan 2023 18:05:05 -0800 (PST) Date: Mon, 9 Jan 2023 18:05:04 -0800 From: Gleb Smirnoff To: "Bjoern A. Zeeb" Cc: Andrew Gallatin , "pjd@FreeBSD.org" , James Gritton , jail@freebsd.org Subject: Re: prison_flag() check in hot path of in_pcblookup() Message-ID: References: <6on81os3-501-s5n2-8nos-p85n8op23232@serrofq.bet> <6r10qop4-7p83-qs6s-q3r0-64756n243rp5@serrofq.bet> List-Id: Discussion about FreeBSD jail(8) List-Archive: https://lists.freebsd.org/archives/freebsd-jail List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-jail@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6r10qop4-7p83-qs6s-q3r0-64756n243rp5@serrofq.bet> X-Spamd-Result: default: False [0.51 / 15.00]; VIOLATED_DIRECT_SPF(3.50)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.99)[-0.989]; MIME_GOOD(-0.10)[text/plain]; RCVD_NO_TLS_LAST(0.10)[]; MLMMJ_DEST(0.00)[jail@freebsd.org]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:27348, ipnet:162.251.186.0/24, country:US]; RCVD_COUNT_TWO(0.00)[2]; R_DKIM_NA(0.00)[]; ARC_NA(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCPT_COUNT_FIVE(0.00)[5]; FREEFALL_USER(0.00)[glebius]; TO_DN_EQ_ADDR_SOME(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; DMARC_NA(0.00)[freebsd.org]; TO_DN_SOME(0.00)[]; R_SPF_SOFTFAIL(0.00)[~all:c]; FREEMAIL_CC(0.00)[gmail.com,freebsd.org] X-Rspamd-Queue-Id: 4NrYzj4JXWz3jx0 X-Spamd-Bar: / X-ThisMailContainsUnwantedMimeParts: N On Tue, Dec 13, 2022 at 11:54:17PM +0000, Bjoern A. Zeeb wrote: B> On Tue, 13 Dec 2022, Andrew Gallatin wrote: B> B> > [ I added pjd, since the original patch came from him ] B> > B> > Just to make sure I understand, I have a simple yes/no question: B> > B> > Can jails and the host ever share the same (local) port and the same IP? B> B> Can they currently (I tested only for TCP)? B> B> - local binds can overlap like they can with just the base system. B> so bind(... {AF_INET, laddr, lport} ... ) works fine (REUSEPORT). B> B> - tcp connect of a 2nd socket to the same {faddr, fport} from the above B> bind will fail with 'Address already in use' [currently] B> [I believe that would mean your patch could go in? Where does the error come from [%]?] [*] My reading of code confirms your tests. The official way to insert a tuple is in_pcbconnect_setup() which branches into either its own check with in_pcblookup_hash_locked() and returns EADDRINUSE or runs in_pcb_lport_dest() which would also use in_pcblookup_hash_locked() to check for existing entries. Thus, with official KPI it is impossible to insert two completely identical 4-tuples. Alas, we have in_pcbinshash() exposed outside, as it is expected to be called after in_pcblookup_hash_locked(), so there is no 100% source code protection from having identical tuples. ... and I'm lost in researching all possible scenarios that edit the database. We need to tighten this KPI. Anyway, given tests that Bjoern did, given lack of official tests in src/tests/sys and overall agreement on this code looking strange, I'd suggest to go forward with Drew's suggestion: https://reviews.freebsd.org/D38015 What's your opinion? Given that code comes from Drew I'd suggest him to either commandeer the revision and commit himself, or let me commit & push with --author. Or I can commit with my name and be fully resposible for the fallout :) -- Gleb Smirnoff