From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id EFF175A0082 for ; Mon, 24 Oct 2022 01:43:17 +0200 (CEST) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4MwZX00Zcvz4xGS; Mon, 24 Oct 2022 10:43:12 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1666568592; bh=QvINtwHXiJ91oXQz1vDMDTDG9PQL0UN/yKEHiTUo5fY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=gbACDWuExZFKoF3k3pcr7+u2mofzyz26q0sdfdm5lZrZ77AOjUG4CFY+EkNGnfNaz YmTc73CSNQfyy/CtbsBHc6KNnQQZ+APLHx0hLqudP65gTXaai/vuQ8wqMMXx6z0+nU Og0nCLAbdEt2ttXUncHRfEHYbGxkKlghUPguFXJk= Date: Mon, 24 Oct 2022 10:36:19 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] util: Set NS_FN_STACK_SIZE to one eighth of ulimit-reported maximum stack size Message-ID: References: <20221022064503.386563-1-sbrivio@redhat.com> <20221022101535.4ceda8d6@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0ZhRBq7ZYQLp7jft" Content-Disposition: inline In-Reply-To: <20221022101535.4ceda8d6@elisabeth> Message-ID-Hash: VDFTIRP74XM53OLF7WLPWNCPLTGTK6IX X-Message-ID-Hash: VDFTIRP74XM53OLF7WLPWNCPLTGTK6IX X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Andrea Bolognani X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: <> Archived-At: List-Archive: <> List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --0ZhRBq7ZYQLp7jft Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Oct 22, 2022 at 10:15:35AM +0200, Stefano Brivio wrote: > On Sat, 22 Oct 2022 08:45:03 +0200 > Stefano Brivio wrote: >=20 > > ...instead of one fourth. On the main() -> conf() -> nl_sock_init() > > call path, LTO from gcc 12 on (at least) x86_64 decides to inline... > > everything: nl_sock_init() is effectively part of main(), after > > commit 3e2eb4337bc0 ("conf: Bind inbound ports with > > CAP_NET_BIND_SERVICE before isolate_user()"). > >=20 > > This means we exceed the maximum stack size, and we get SIGSEGV, > > under any condition, at start time, as reported by Andrea on a recent > > build for CentOS Stream 9. > >=20 > > The calculation of NS_FN_STACK_SIZE, which is the stack size we > > reserve for clones, was previously obtained by dividing the maximum > > stack size by two, to avoid an explicit check on architecture (on > > PA-RISC, also known as hppa, the stack grows up, so we point the > > clone to the middle of this area), and then further divided by two > > to allow for any additional usage in the caller. > >=20 > > Well, if there are essentially no function calls anymore, this is > > not enough. Divide it by eight, which is anyway much more than > > possibly needed by any clone()d callee. > >=20 > > I think this is robust, so it's a fix in some sense. Strictly > > speaking, though, we have no formal guarantees that this isn't > > either too little or too much. > >=20 > > What we should do, eventually: check cloned() callees, there are just > > thirteen of them at the moment. Note down any stack usage (they are > > mostly small helpers), bonus points for an automated way at build > > time, quadruple that or so, to allow for extreme clumsiness, and use > > as NS_FN_STACK_SIZE. Perhaps introduce a specific condition for hppa. > >=20 > > Reported-by: Andrea Bolognani > > Fixes: 3e2eb4337bc0 ("conf: Bind inbound ports with CAP_NET_BIND_SERVIC= E before isolate_user()") > > Signed-off-by: Stefano Brivio > > --- >=20 > I posted this in any case for (later) review, but I'm actually applying > it right away, given that some builds are completely unusable otherwise. That's sensible, however, this patch confuses me. I don't really understand how reducing the stack size is avoiding a SEGV, regardless of what LTO does. The fact that we're basing the runtime stack size on a limit that's from build time also doesn't really make sense to me. > I guess it would make sense to extend build ("distro") tests with some > compiler flags, common and less common. >=20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --0ZhRBq7ZYQLp7jft Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmNVz+kACgkQgypY4gEw YSJwWA//UhCa4RS2QRKLd3buBLBroxLuVKmhoYS/5PIr0mKyO9Ebem8Wl15Dg9Qg l2Azdj3alC7BUew+DKE7dq80QXNL3nGTmrY1l27Am4E1q0Ls4rnew45Nc4Xsj+Tj 0B44WidoJRePq1ih6HOX2xr2noJtkkJKgpmwhIR9m6tJb3xsletYlYVLHpZ2t3/z WZC9VvI6nAYALSXGcWYwcmyPOwGjTYvPwyW47oQtwhiF913oB38ZMto8zG616Df1 nzzv0ORTpf1lUDiSIaid9K9qjEKBf7Y48WK6lGKUJxx7qmaY6+7tYMXVP20f+gGV eJn1KzJJRVdWw2dMnJyPT8y32QA0ow55VujduoTibFRiR8QBskJ0xEmpWFrKn047 k606ypFWrdgFeda7DFrpIIRJ2Aov1m4bgL2SphBiSrs5HuRXbU2Hqur8DUU50pWH JNeaVuhHgojn23HZ+b6JTV9RpCAXtg4kT57P+fAfdjkR+UAoWNw4NbMHZvNu6Sid N7Hkh4cuiGxRG8s++MDqZhnXGKC3f7QMUUS/6rs4MO0Ya7uieEFLvpbmG8riIgnE mkbxYqi1qtGYKIbUKPaikYI0OPiXWySEf6a4hhd/AdCelXYfb6AUe2BAJwaOxvMi xxB76HvdL6QZmbMnkP8H4h3BngGmyZrmDWUknR3JOWBnTPqtlMs= =AHwT -----END PGP SIGNATURE----- --0ZhRBq7ZYQLp7jft--