From b7fbe65ff26eb769013b9ad23321cd685e36f04e Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Tue, 3 Dec 2019 18:38:07 +0100 Subject: [PATCH] libc: fork/execve improvements - Close FDs marked with the close-on-execve flag (needed for 'make', which sets the flag for the pipe-in FD of forked children) - Update binary name on execve to use as ROM for subsequent fork - Enable vfork as an alias for fork (needed by make) - Purge line buffers for output streams during execve because they may be allocated at the allocation heap, which does not survive the execve call. - Consider short-lived processes that may exit while the parent still blocks in the fork call. With these changes, the website generator of genodians.org works without the need for the Noux runtime. Issue #3578 --- repos/libports/include/libc-plugin/fd_alloc.h | 8 ++ repos/libports/src/lib/libc/dummies.cc | 1 - repos/libports/src/lib/libc/execve.cc | 45 ++++--- repos/libports/src/lib/libc/fd_alloc.cc | 14 +++ repos/libports/src/lib/libc/fork.cc | 112 ++++++++++-------- .../src/lib/libc/internal/file_operations.h | 3 +- repos/libports/src/lib/libc/internal/init.h | 7 +- repos/libports/src/lib/libc/internal/kernel.h | 7 ++ repos/libports/src/lib/libc/internal/types.h | 2 + repos/libports/src/lib/libc/kernel.cc | 8 +- 10 files changed, 137 insertions(+), 70 deletions(-) diff --git a/repos/libports/include/libc-plugin/fd_alloc.h b/repos/libports/include/libc-plugin/fd_alloc.h index dabf3572b..f046636b5 100644 --- a/repos/libports/include/libc-plugin/fd_alloc.h +++ b/repos/libports/include/libc-plugin/fd_alloc.h @@ -104,6 +104,14 @@ namespace Libc { File_descriptor *find_by_libc_fd(int libc_fd); + /** + * Return any file descriptor with close-on-execve flag set + * + * \return pointer to file descriptor, or + * nullptr is no such file descriptor exists + */ + File_descriptor *any_cloexec_libc_fd(); + void generate_info(Genode::Xml_generator &); }; diff --git a/repos/libports/src/lib/libc/dummies.cc b/repos/libports/src/lib/libc/dummies.cc index 6b461b96d..cb1dbbb8d 100644 --- a/repos/libports/src/lib/libc/dummies.cc +++ b/repos/libports/src/lib/libc/dummies.cc @@ -97,7 +97,6 @@ DUMMY(char *, 0, crypt, (const char *, const char *)) DUMMY(DB * , 0, dbopen, (const char *, int, int, DBTYPE, const void *)) DUMMY(u_int32_t, 0, __default_hash, (const void *, size_t)); DUMMY_SILENT(long , -1, _fpathconf, (int, int)) -DUMMY(pid_t , -1, vfork, (void)) DUMMY(long , -1, fpathconf, (int, int)) DUMMY(int , -1, freebsd7___semctl, (void)) DUMMY(int , -1, getcontext, (ucontext_t *)) diff --git a/repos/libports/src/lib/libc/execve.cc b/repos/libports/src/lib/libc/execve.cc index 044ab1044..2f1d7aaf0 100644 --- a/repos/libports/src/lib/libc/execve.cc +++ b/repos/libports/src/lib/libc/execve.cc @@ -21,7 +21,9 @@ #include #include #include +#include #include +#include /* libc-internal includes */ #include @@ -171,7 +173,7 @@ struct Libc::Interpreter _rom(env, filename), num_args(_count_args() + 2 /* argv0 + filename */) { if (script()) { - args = (char **)malloc(sizeof(char *)*num_args); + args = (char **)calloc(num_args + 1 /* null termination */, sizeof(char *)); unsigned i = 0; @@ -183,8 +185,6 @@ struct Libc::Interpreter /* supply script file name as last argument */ args[i++] = strdup(filename); - - } } @@ -277,9 +277,10 @@ struct Libc::String_array : Noncopyable size_t const n = _num_entries(src_array); for (unsigned i = skip; i < n; i++) { - array[dst_i++] = _buffer->pos_ptr(); + array[dst_i] = _buffer->pos_ptr(); if (!_buffer->try_append(src_array[i])) break; + dst_i++; } }; @@ -291,8 +292,6 @@ struct Libc::String_array : Noncopyable array[dst_i] = nullptr; break; } - - warning("string-array buffer ", size, " exceeded"); size += 1024; } } @@ -304,22 +303,27 @@ struct Libc::String_array : Noncopyable /* pointer to environment, provided by libc */ extern char **environ; -static Env *_env_ptr; -static Allocator *_alloc_ptr; -static void *_user_stack_ptr; -static main_fn_ptr _main_ptr; -static Libc::String_array *_env_vars_ptr; -static Libc::String_array *_args_ptr; -static Libc::Reset_malloc_heap *_reset_malloc_heap_ptr; +static Env *_env_ptr; +static Allocator *_alloc_ptr; +static void *_user_stack_ptr; +static main_fn_ptr _main_ptr; +static Libc::String_array *_env_vars_ptr; +static Libc::String_array *_args_ptr; +static Libc::Reset_malloc_heap *_reset_malloc_heap_ptr; +static Libc::Binary_name *_binary_name_ptr; +static Libc::File_descriptor_allocator *_fd_alloc_ptr; void Libc::init_execve(Env &env, Genode::Allocator &alloc, void *user_stack_ptr, - Reset_malloc_heap &reset_malloc_heap) + Reset_malloc_heap &reset_malloc_heap, Binary_name &binary_name, + File_descriptor_allocator &fd_alloc) { _env_ptr = &env; _alloc_ptr = &alloc; _user_stack_ptr = user_stack_ptr; _reset_malloc_heap_ptr = &reset_malloc_heap; + _binary_name_ptr = &binary_name; + _fd_alloc_ptr = &fd_alloc; Dynamic_linker::keep(env, "libc.lib.so"); Dynamic_linker::keep(env, "libm.lib.so"); @@ -346,6 +350,10 @@ extern "C" int execve(char const *filename, return Libc::Errno(EACCES); } + /* close all file descriptors with the close-on-execve flag enabled */ + while (Libc::File_descriptor *fd = _fd_alloc_ptr->any_cloexec_libc_fd()) + close(fd->libc_fd); + /* capture environment variables and args to libc-internal heap */ Libc::String_array *saved_env_vars = new (*_alloc_ptr) Libc::String_array(*_alloc_ptr, envp); @@ -367,7 +375,7 @@ extern "C" int execve(char const *filename, try { Libc::resolve_symlinks(path.string(), resolved_path); } catch (Libc::Symlink_resolve_error) { - warning("execve: executable binary does not exist"); + warning("execve: executable binary '", filename, "' does not exist"); return Libc::Errno(ENOENT); } @@ -411,6 +419,10 @@ extern "C" int execve(char const *filename, return Libc::Errno(EACCES); } + /* purge line buffers, which may be allocated at the application heap */ + setvbuf(stdout, nullptr, _IONBF, 0); + setvbuf(stderr, nullptr, _IONBF, 0); + /* * Reconstruct malloc heap for application-owned data */ @@ -424,6 +436,9 @@ extern "C" int execve(char const *filename, /* register list of environment variables at libc 'environ' pointer */ environ = _env_vars_ptr->array; + /* remember name of new ROM module, to be used by next call of fork */ + *_binary_name_ptr = Libc::Binary_name(resolved_path.string()); + destroy(_alloc_ptr, saved_env_vars); destroy(_alloc_ptr, saved_args); diff --git a/repos/libports/src/lib/libc/fd_alloc.cc b/repos/libports/src/lib/libc/fd_alloc.cc index b003f75dd..57e0c4c0f 100644 --- a/repos/libports/src/lib/libc/fd_alloc.cc +++ b/repos/libports/src/lib/libc/fd_alloc.cc @@ -111,6 +111,20 @@ File_descriptor *File_descriptor_allocator::find_by_libc_fd(int libc_fd) } +File_descriptor *File_descriptor_allocator::any_cloexec_libc_fd() +{ + Lock::Guard guard(_lock); + + File_descriptor *result = nullptr; + + _id_space.for_each([&] (File_descriptor &fd) { + if (!result && fd.cloexec) + result = &fd; }); + + return result; +} + + void File_descriptor_allocator::generate_info(Xml_generator &xml) { Lock::Guard guard(_lock); diff --git a/repos/libports/src/lib/libc/fork.cc b/repos/libports/src/lib/libc/fork.cc index fb3437c07..ac49e588d 100644 --- a/repos/libports/src/lib/libc/fork.cc +++ b/repos/libports/src/lib/libc/fork.cc @@ -40,6 +40,23 @@ #include #include +namespace Libc { + struct Child_config; + struct Parent_services; + struct Local_rom_service; + struct Local_rom_services; + struct Local_clone_service; + struct Forked_child_policy; + struct Forked_child; + + struct Child_ready : Interface + { + virtual void child_ready() = 0; + }; + + typedef Registry > Forked_children; +} + using namespace Libc; @@ -56,40 +73,9 @@ static void *_user_stack_base_ptr; static size_t _user_stack_size; static int _pid; static int _pid_cnt; -static Config_accessor const *_config_accessor_ptr; - - -void Libc::init_fork(Env &env, Config_accessor const &config_accessor, - Allocator &alloc, Heap &malloc_heap, pid_t pid, - Suspend &suspend, Resume &resume, Signal &signal, - Kernel_routine_scheduler &kernel_routine_scheduler) -{ - _env_ptr = &env; - _alloc_ptr = &alloc; - _suspend_ptr = &suspend; - _resume_ptr = &resume; - _signal_ptr = &signal; - _kernel_routine_scheduler_ptr = &kernel_routine_scheduler; - _malloc_heap_ptr = &malloc_heap; - _config_accessor_ptr = &config_accessor; - _pid = pid; -} - - -namespace Libc { - struct Child_config; - struct Parent_services; - struct Local_rom_service; - struct Local_rom_services; - struct Local_clone_service; - struct Forked_child_policy; - struct Forked_child; - - struct Child_ready : Interface - { - virtual void child_ready() = 0; - }; -} +static Config_accessor const *_config_accessor_ptr; +static Binary_name const *_binary_name_ptr; +static Forked_children *_forked_children_ptr; struct Libc::Child_config @@ -406,6 +392,8 @@ struct Libc::Forked_child : Child_policy, Child_ready { Env &_env; + Binary_name const _binary_name; + Resume &_resume; Signal &_signal; @@ -447,7 +435,7 @@ struct Libc::Forked_child : Child_policy, Child_ready void execute_in_kernel() override { /* keep executing this kernel routine until child is running */ - if (!child.running()) + if (!child.running() && !child.exited()) _kernel_routine_scheduler_ptr->register_kernel_routine(*this); } } wait_fork_ready { *this }; @@ -465,7 +453,17 @@ struct Libc::Forked_child : Child_policy, Child_ready ** Child_ready interface ** ***************************/ - void child_ready() override { _state = State::RUNNING; } + void child_ready() override + { + /* + * Don't modify the state if the child already exited. + * This can happen for short-lived children where the asynchronous + * notification for '_handle_exit' arrives before '_handle_child_ready' + * (while the parent is still blocking in the fork call). + */ + if (_state == State::STARTING_UP) + _state = State::RUNNING; + } /**************************** @@ -474,7 +472,7 @@ struct Libc::Forked_child : Child_policy, Child_ready Name name() const override { return _name; } - Binary_name binary_name() const override { return "binary"; } + Binary_name binary_name() const override { return _binary_name; } Pd_session &ref_pd() override { return _env.pd(); } Pd_session_capability ref_pd_cap() const override { return _env.pd_session_cap(); } @@ -549,6 +547,7 @@ struct Libc::Forked_child : Child_policy, Child_ready Forked_child(Env &env, Entrypoint &fork_ep, Allocator &alloc, + Binary_name const &binary_name, Resume &resume, Signal &signal, pid_t pid, @@ -556,7 +555,8 @@ struct Libc::Forked_child : Child_policy, Child_ready Parent_services &parent_services, Local_rom_services &local_rom_services) : - _env(env), _resume(resume), _signal(signal), _pid(pid), + _env(env), _binary_name(binary_name), + _resume(resume), _signal(signal), _pid(pid), _child_config(env, config_accessor, pid), _parent_services(parent_services), _local_rom_services(local_rom_services), @@ -569,11 +569,6 @@ struct Libc::Forked_child : Child_policy, Child_ready }; -/* initialized on first call of 'fork_kernel_routine' */ -typedef Registry > Forked_children; -static Forked_children *_forked_children_ptr; - - static void fork_kernel_routine() { fork_result = 0; @@ -597,11 +592,9 @@ static void fork_kernel_routine() static Local_rom_services local_rom_services(env, fork_ep, alloc); - static Forked_children forked_children { }; - _forked_children_ptr = &forked_children; - Registered &child = *new (alloc) - Registered(forked_children, env, fork_ep, alloc, resume, + Registered(*_forked_children_ptr, env, fork_ep, alloc, + *_binary_name_ptr, resume, signal, child_pid, *_config_accessor_ptr, parent_services, local_rom_services); @@ -652,7 +645,9 @@ extern "C" pid_t __sys_fork(void) return fork_result; } -pid_t fork(void) __attribute__((weak, alias("__sys_fork"))); + +pid_t fork(void) __attribute__((weak, alias("__sys_fork"))); +pid_t vfork(void) __attribute__((weak, alias("__sys_fork"))); /************ @@ -758,3 +753,24 @@ extern "C" pid_t __sys_wait4(pid_t pid, int *status, int options, rusage *rusage extern "C" pid_t wait4(pid_t, int *, int, rusage *) __attribute__((weak, alias("__sys_wait4"))); + +void Libc::init_fork(Env &env, Config_accessor const &config_accessor, + Allocator &alloc, Heap &malloc_heap, pid_t pid, + Suspend &suspend, Resume &resume, Signal &signal, + Kernel_routine_scheduler &kernel_routine_scheduler, + Binary_name const &binary_name) +{ + _env_ptr = &env; + _alloc_ptr = &alloc; + _suspend_ptr = &suspend; + _resume_ptr = &resume; + _signal_ptr = &signal; + _kernel_routine_scheduler_ptr = &kernel_routine_scheduler; + _malloc_heap_ptr = &malloc_heap; + _config_accessor_ptr = &config_accessor; + _pid = pid; + _binary_name_ptr = &binary_name; + + static Forked_children forked_children { }; + _forked_children_ptr = &forked_children; +} diff --git a/repos/libports/src/lib/libc/internal/file_operations.h b/repos/libports/src/lib/libc/internal/file_operations.h index 1d9366e79..d3aaebbeb 100644 --- a/repos/libports/src/lib/libc/internal/file_operations.h +++ b/repos/libports/src/lib/libc/internal/file_operations.h @@ -19,6 +19,7 @@ /* libc includes */ #include /* for 'PATH_MAX' */ +#include /* libc-internal includes */ #include @@ -27,8 +28,6 @@ namespace Libc { typedef Genode::Path Absolute_path; - class Symlink_resolve_error : Exception { }; - void resolve_symlinks(char const *path, Absolute_path &resolved_path); } diff --git a/repos/libports/src/lib/libc/internal/init.h b/repos/libports/src/lib/libc/internal/init.h index 8889a69ef..1e698ca14 100644 --- a/repos/libports/src/lib/libc/internal/init.h +++ b/repos/libports/src/lib/libc/internal/init.h @@ -36,6 +36,7 @@ namespace Libc { struct Kernel_routine_scheduler; struct Watch; struct Signal; + struct File_descriptor_allocator; /** * Support for shared libraries @@ -118,7 +119,8 @@ namespace Libc { */ void init_fork(Genode::Env &, Config_accessor const &, Genode::Allocator &heap, Heap &malloc_heap, int pid, - Suspend &, Resume &, Signal &, Kernel_routine_scheduler &); + Suspend &, Resume &, Signal &, Kernel_routine_scheduler &, + Binary_name const &); struct Reset_malloc_heap : Interface { @@ -129,7 +131,8 @@ namespace Libc { * Execve mechanism */ void init_execve(Genode::Env &, Genode::Allocator &, void *user_stack, - Reset_malloc_heap &); + Reset_malloc_heap &, Binary_name &, + File_descriptor_allocator &); /** * Signal handling diff --git a/repos/libports/src/lib/libc/internal/kernel.h b/repos/libports/src/lib/libc/internal/kernel.h index e5edab542..e140a1b11 100644 --- a/repos/libports/src/lib/libc/internal/kernel.h +++ b/repos/libports/src/lib/libc/internal/kernel.h @@ -79,6 +79,13 @@ struct Libc::Kernel final : Vfs::Io_response_handler, */ Allocator &_heap; + /** + * Name of the current binary's ROM module + * + * Used by fork, modified by execve. + */ + Binary_name _binary_name { "binary" }; + /** * Allocator for application-owned data * diff --git a/repos/libports/src/lib/libc/internal/types.h b/repos/libports/src/lib/libc/internal/types.h index 1945c066d..39addb350 100644 --- a/repos/libports/src/lib/libc/internal/types.h +++ b/repos/libports/src/lib/libc/internal/types.h @@ -15,12 +15,14 @@ #define _LIBC__INTERNAL__TYPES_H_ #include +#include namespace Libc { using namespace Genode; typedef Genode::uint64_t uint64_t; + typedef String<64> Binary_name; } #endif /* _LIBC__INTERNAL__TYPES_H_ */ diff --git a/repos/libports/src/lib/libc/kernel.cc b/repos/libports/src/lib/libc/kernel.cc index 892614bd2..f04160a5f 100644 --- a/repos/libports/src/lib/libc/kernel.cc +++ b/repos/libports/src/lib/libc/kernel.cc @@ -119,6 +119,8 @@ void Libc::Kernel::_init_file_descriptors() return; } + fd->cloexec = node.attribute_value("cloexec", false); + /* * We need to manually register the path. Normally this is done * by '_open'. But we call the local 'open' function directly @@ -373,8 +375,10 @@ Libc::Kernel::Kernel(Genode::Env &env, Genode::Allocator &heap) init_malloc(*_malloc_heap); } - init_fork(_env, _libc_env, _heap, *_malloc_heap, _pid, *this, *this, _signal, *this); - init_execve(_env, _heap, _user_stack, *this); + init_fork(_env, _libc_env, _heap, *_malloc_heap, _pid, *this, *this, _signal, + *this, _binary_name); + init_execve(_env, _heap, _user_stack, *this, _binary_name, + *file_descriptor_allocator()); init_plugin(*this); init_sleep(*this); init_vfs_plugin(*this);