Spotted in Debian's buildd logs: on ia64, clone(2) is not available:
the glibc wrapper is named __clone2() and it takes, additionally,
the size of the stack area passed by the caller.
Add a do_clone() wrapper handling the different cases, and also
taking care of pointing the child's stack in the middle of the
allocated area: on PA-RISC (hppa), handled by clone(), the stack
grows up, and on ia64 the stack grows down, but the register backing
store grows up -- and I think it might be actually used here.
Suggested-by: David Gibson <david(a)gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
v3: Don't call it __clone(), it conflicts with glibc on static builds
v2: Add a wrapper instead of two __ia64__ ifdefs
pasta.c | 10 +++++-----
util.c | 21 +++++++++++++++++++++
util.h | 8 +++++---
3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/pasta.c b/pasta.c
index db86317..528f02a 100644
--- a/pasta.c
+++ b/pasta.c
@@ -226,11 +226,11 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
arg.argv = sh_argv;
}
- pasta_child_pid = clone(pasta_spawn_cmd,
- ns_fn_stack + sizeof(ns_fn_stack) / 2,
- CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET |
- CLONE_NEWUTS,
- (void *)&arg);
+ pasta_child_pid = do_clone(pasta_spawn_cmd, ns_fn_stack,
+ sizeof(ns_fn_stack),
+ CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET |
+ CLONE_NEWUTS,
+ (void *)&arg);
if (pasta_child_pid == -1) {
perror("clone");
diff --git a/util.c b/util.c
index be102e3..2125a67 100644
--- a/util.c
+++ b/util.c
@@ -482,3 +482,24 @@ int write_file(const char *path, const char *buf)
close(fd);
return len == 0 ? 0 : -1;
}
+
+/**
+ * do_clone() - Wrapper of __clone2() for ia64, clone() for other architectures
+ * @fn: Entry point for child
+ * @stack_area: Stack area for child: we'll point callees to the middle of it
+ * @stack_size: Total size of stack area, passed to callee divided by two
+ * @flags: clone() system call flags
+ * @arg: Argument to @fn
+ *
+ * Return: thread ID of child, -1 on failure
+ */
+int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
+ void *arg)
+{
+#ifdef __ia64__
+ return __clone2(fn, stack_area + stack_size / 2, stack_size / 2,
+ flags, arg);
+#else
+ return clone(fn, stack_area + stack_size / 2, flags, arg);
+#endif
+}
diff --git a/util.h b/util.h
index 2d4e1ff..02b55a9 100644
--- a/util.h
+++ b/util.h
@@ -81,13 +81,15 @@
(((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr)
#define NS_FN_STACK_SIZE (RLIMIT_STACK_VAL * 1024 / 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]; \
\
- clone((fn), ns_fn_stack + sizeof(ns_fn_stack) / 2, \
- CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD, \
- (void *)(arg)); \
+ do_clone((fn), ns_fn_stack, sizeof(ns_fn_stack), \
+ CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD,\
+ (void *)(arg)); \
} while (0)
#if __BYTE_ORDER == __BIG_ENDIAN
--
2.35.1
On Wed, Nov 16, 2022 at 05:30:57PM +0100, Stefano Brivio wrote:Spotted in Debian's buildd logs: on ia64, clone(2) is not available: the glibc wrapper is named __clone2() and it takes, additionally, the size of the stack area passed by the caller. Add a do_clone() wrapper handling the different cases, and also taking care of pointing the child's stack in the middle of the allocated area: on PA-RISC (hppa), handled by clone(), the stack grows up, and on ia64 the stack grows down, but the register backing store grows up -- and I think it might be actually used here. Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Heh, my only comment on v2 was going to be that it's not a good idea to name identifiers starting with _ - that's reserved for the system library. Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>--- v3: Don't call it __clone(), it conflicts with glibc on static builds v2: Add a wrapper instead of two __ia64__ ifdefs pasta.c | 10 +++++----- util.c | 21 +++++++++++++++++++++ util.h | 8 +++++--- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/pasta.c b/pasta.c index db86317..528f02a 100644 --- a/pasta.c +++ b/pasta.c @@ -226,11 +226,11 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, arg.argv = sh_argv; } - pasta_child_pid = clone(pasta_spawn_cmd, - ns_fn_stack + sizeof(ns_fn_stack) / 2, - CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET | - CLONE_NEWUTS, - (void *)&arg); + pasta_child_pid = do_clone(pasta_spawn_cmd, ns_fn_stack, + sizeof(ns_fn_stack), + CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET | + CLONE_NEWUTS, + (void *)&arg); if (pasta_child_pid == -1) { perror("clone"); diff --git a/util.c b/util.c index be102e3..2125a67 100644 --- a/util.c +++ b/util.c @@ -482,3 +482,24 @@ int write_file(const char *path, const char *buf) close(fd); return len == 0 ? 0 : -1; } + +/** + * do_clone() - Wrapper of __clone2() for ia64, clone() for other architectures + * @fn: Entry point for child + * @stack_area: Stack area for child: we'll point callees to the middle of it + * @stack_size: Total size of stack area, passed to callee divided by two + * @flags: clone() system call flags + * @arg: Argument to @fn + * + * Return: thread ID of child, -1 on failure + */ +int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, + void *arg) +{ +#ifdef __ia64__ + return __clone2(fn, stack_area + stack_size / 2, stack_size / 2, + flags, arg); +#else + return clone(fn, stack_area + stack_size / 2, flags, arg); +#endif +} diff --git a/util.h b/util.h index 2d4e1ff..02b55a9 100644 --- a/util.h +++ b/util.h @@ -81,13 +81,15 @@ (((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr) #define NS_FN_STACK_SIZE (RLIMIT_STACK_VAL * 1024 / 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]; \ \ - clone((fn), ns_fn_stack + sizeof(ns_fn_stack) / 2, \ - CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD, \ - (void *)(arg)); \ + do_clone((fn), ns_fn_stack, sizeof(ns_fn_stack), \ + CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD,\ + (void *)(arg)); \ } while (0) #if __BYTE_ORDER == __BIG_ENDIAN-- 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
On Thu, 17 Nov 2022 10:00:32 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:On Wed, Nov 16, 2022 at 05:30:57PM +0100, Stefano Brivio wrote:Oops, yes, I keep forgetting about that -- and we already have other offenders in util.c, I'll try to switch them to other names at some point. -- StefanoSpotted in Debian's buildd logs: on ia64, clone(2) is not available: the glibc wrapper is named __clone2() and it takes, additionally, the size of the stack area passed by the caller. Add a do_clone() wrapper handling the different cases, and also taking care of pointing the child's stack in the middle of the allocated area: on PA-RISC (hppa), handled by clone(), the stack grows up, and on ia64 the stack grows down, but the register backing store grows up -- and I think it might be actually used here. Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Heh, my only comment on v2 was going to be that it's not a good idea to name identifiers starting with _ - that's reserved for the system library.