[PATCH] pasta, util: Align stack area for clones to maximum natural alignment

[PATCH] pasta, util: Align stack area for clones to maximum natural alignment

[Stefano Brivio]

Given that we use this stack pointer as a location to store arbitrary
data types from the cloned process, we need to guarantee that its
alignment matches any of those possible data types.

runsisi reports that pasta gets a SIGBUS in pasta_open_ns() on
aarch64, where the alignment requirement for stack pointers is a
16 bytes (same as the size of a long double), and similar requirements
actually apply to most architectures we run on.

Reported-by: runsisi <runsisi@hust.edu.cn>
Link: https://bugs.passt.top/show_bug.cgi?id=85
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 pasta.c | 3 ++-
 util.h  | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/pasta.c b/pasta.c
index e73b5af..31e1e00 100644
--- a/pasta.c
+++ b/pasta.c
@@ -211,12 +211,13 @@ static int pasta_spawn_cmd(void *arg)
 void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
 		    int argc, char *argv[])
 {
+	char ns_fn_stack[NS_FN_STACK_SIZE]
+	__attribute__ ((aligned(__alignof__(max_align_t))));
 	struct pasta_spawn_cmd_arg arg = {
 		.exe = argv[0],
 		.argv = argv,
 	};
 	char uidmap[BUFSIZ], gidmap[BUFSIZ];
-	char ns_fn_stack[NS_FN_STACK_SIZE];
 	char *sh_argv[] = { NULL, NULL };
 	char sh_arg0[PATH_MAX + 1];
 	sigset_t set;
diff --git a/util.h b/util.h
index 7c261d7..8c37729 100644
--- a/util.h
+++ b/util.h
@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <stdbool.h>
+#include <stddef.h>
 #include <string.h>
 #include <signal.h>
 
@@ -116,7 +117,8 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
 	     void *arg);
 #define NS_CALL(fn, arg)						\
 	do {								\
-		char ns_fn_stack[NS_FN_STACK_SIZE];			\
+		char ns_fn_stack[NS_FN_STACK_SIZE]			\
+		__attribute__ ((aligned(__alignof__(max_align_t))));	\
 									\
 		do_clone((fn), ns_fn_stack, sizeof(ns_fn_stack),	\
 			 CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD,\
-- 
@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <stdbool.h>
+#include <stddef.h>
 #include <string.h>
 #include <signal.h>
 
@@ -116,7 +117,8 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
 	     void *arg);
 #define NS_CALL(fn, arg)						\
 	do {								\
-		char ns_fn_stack[NS_FN_STACK_SIZE];			\
+		char ns_fn_stack[NS_FN_STACK_SIZE]			\
+		__attribute__ ((aligned(__alignof__(max_align_t))));	\
 									\
 		do_clone((fn), ns_fn_stack, sizeof(ns_fn_stack),	\
 			 CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD,\
-- 
2.43.0

Re: [PATCH] pasta, util: Align stack area for clones to maximum natural alignment

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2396 bytes --]

On Fri, Apr 12, 2024 at 12:18:00AM +0200, Stefano Brivio wrote:
> Given that we use this stack pointer as a location to store arbitrary
> data types from the cloned process, we need to guarantee that its
> alignment matches any of those possible data types.
> 
> runsisi reports that pasta gets a SIGBUS in pasta_open_ns() on
> aarch64, where the alignment requirement for stack pointers is a
> 16 bytes (same as the size of a long double), and similar requirements
> actually apply to most architectures we run on.
> 
> Reported-by: runsisi <runsisi@hust.edu.cn>
> Link: https://bugs.passt.top/show_bug.cgi?id=85
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  pasta.c | 3 ++-
>  util.h  | 4 +++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/pasta.c b/pasta.c
> index e73b5af..31e1e00 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -211,12 +211,13 @@ static int pasta_spawn_cmd(void *arg)
>  void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
>  		    int argc, char *argv[])
>  {
> +	char ns_fn_stack[NS_FN_STACK_SIZE]
> +	__attribute__ ((aligned(__alignof__(max_align_t))));
>  	struct pasta_spawn_cmd_arg arg = {
>  		.exe = argv[0],
>  		.argv = argv,
>  	};
>  	char uidmap[BUFSIZ], gidmap[BUFSIZ];
> -	char ns_fn_stack[NS_FN_STACK_SIZE];
>  	char *sh_argv[] = { NULL, NULL };
>  	char sh_arg0[PATH_MAX + 1];
>  	sigset_t set;
> diff --git a/util.h b/util.h
> index 7c261d7..8c37729 100644
> --- a/util.h
> +++ b/util.h
> @@ -9,6 +9,7 @@
>  #include <stdlib.h>
>  #include <stdarg.h>
>  #include <stdbool.h>
> +#include <stddef.h>
>  #include <string.h>
>  #include <signal.h>
>  
> @@ -116,7 +117,8 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
>  	     void *arg);
>  #define NS_CALL(fn, arg)						\
>  	do {								\
> -		char ns_fn_stack[NS_FN_STACK_SIZE];			\
> +		char ns_fn_stack[NS_FN_STACK_SIZE]			\
> +		__attribute__ ((aligned(__alignof__(max_align_t))));	\
>  									\
>  		do_clone((fn), ns_fn_stack, sizeof(ns_fn_stack),	\
>  			 CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD,\

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]