public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] util: Set NS_FN_STACK_SIZE to one eighth of ulimit-reported maximum stack size
@ 2022-10-22  6:45 Stefano Brivio
  2022-10-22  8:15 ` Stefano Brivio
  0 siblings, 1 reply; 5+ messages in thread
From: Stefano Brivio @ 2022-10-22  6:45 UTC (permalink / raw)
  To: passt-dev; +Cc: Andrea Bolognani

...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()").

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.

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.

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.

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.

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.

Reported-by: Andrea Bolognani <abologna@redhat.com>
Fixes: 3e2eb4337bc0 ("conf: Bind inbound ports with CAP_NET_BIND_SERVICE before isolate_user()")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 util.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util.h b/util.h
index 27829b1..c498a80 100644
--- a/util.h
+++ b/util.h
@@ -72,7 +72,7 @@
 #define IPV4_IS_LOOPBACK(addr)						\
 	((addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET)
 
-#define NS_FN_STACK_SIZE	(RLIMIT_STACK_VAL * 1024 / 4)
+#define NS_FN_STACK_SIZE	(RLIMIT_STACK_VAL * 1024 / 8)
 #define NS_CALL(fn, arg)						\
 	do {								\
 		char ns_fn_stack[NS_FN_STACK_SIZE];			\
-- 
@@ -72,7 +72,7 @@
 #define IPV4_IS_LOOPBACK(addr)						\
 	((addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET)
 
-#define NS_FN_STACK_SIZE	(RLIMIT_STACK_VAL * 1024 / 4)
+#define NS_FN_STACK_SIZE	(RLIMIT_STACK_VAL * 1024 / 8)
 #define NS_CALL(fn, arg)						\
 	do {								\
 		char ns_fn_stack[NS_FN_STACK_SIZE];			\
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-24  0:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-22  6:45 [PATCH] util: Set NS_FN_STACK_SIZE to one eighth of ulimit-reported maximum stack size Stefano Brivio
2022-10-22  8:15 ` Stefano Brivio
2022-10-23 23:36   ` David Gibson
2022-10-23 23:52     ` David Gibson
2022-10-24  0:36       ` Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).