From 66f51cb3b93873ca47d821b6cb82db3f004f63ea Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Fri, 3 Jan 2025 12:16:53 -0500 Subject: [PATCH] kernel/team: Do not inherit anything from the kernel's IO context. We already didn't inherit FDs, which meant that the only thing we did meaningfully inherit was the table size. That meant that basically no applications actually had a table size of the default 256, but all were at the kernel's 4096 (except Tracker and anything started by it, as Tracker resets it to 512), and also that basically all applications had FD tables allocated with the raw allocator instead of the block allocator, which isn't very efficient. Since this reduces the default FD table size, some applications might encounter problems. However, build systems and other such tools should already increase this by default as needed, and it's easy enough to patch in calls to setrlimit() if it turns out some applications needed a higher default after all. Also remove a redundant call to vfs_exec_io_context. Calling vfs_new_io_context with the second argument set to "true" already skips cloning CLOEXEC FDs. --- headers/private/kernel/vfs.h | 1 - src/system/kernel/fs/vfs.cpp | 44 +++++++++++++++++------------------- src/system/kernel/team.cpp | 19 ++++++++-------- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/headers/private/kernel/vfs.h b/headers/private/kernel/vfs.h index 9065d4f712..1e2696a69d 100644 --- a/headers/private/kernel/vfs.h +++ b/headers/private/kernel/vfs.h @@ -59,7 +59,6 @@ typedef struct io_context { struct list node_monitors; uint32 num_monitors; uint32 max_monitors; - bool inherit_fds; } io_context; diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp index 066a99f774..675a1827be 100644 --- a/src/system/kernel/fs/vfs.cpp +++ b/src/system/kernel/fs/vfs.cpp @@ -4968,35 +4968,34 @@ vfs_new_io_context(const io_context* parentContext, bool purgeCloseOnExec) if (parentContext != NULL) { mutex_lock(&sIOContextRootLock); context->root = parentContext->root; - if (context->root) + if (context->root != NULL) inc_vnode_ref_count(context->root); mutex_unlock(&sIOContextRootLock); context->cwd = parentContext->cwd; - if (context->cwd) + if (context->cwd != NULL) inc_vnode_ref_count(context->cwd); - if (parentContext->inherit_fds) { - for (size_t i = 0; i < tableSize; i++) { - struct file_descriptor* descriptor = parentContext->fds[i]; - - if (descriptor != NULL - && (descriptor->open_mode & O_DISCONNECTED) == 0) { - const bool closeOnExec = fd_close_on_exec(parentContext, i); - if (closeOnExec && purgeCloseOnExec) - continue; - - TFD(InheritFD(context, i, descriptor, parentContext)); - - context->fds[i] = descriptor; - context->num_used_fds++; - atomic_add(&descriptor->ref_count, 1); - atomic_add(&descriptor->open_count, 1); - - if (closeOnExec) - fd_set_close_on_exec(context, i, true); - } + for (size_t i = 0; i < tableSize; i++) { + struct file_descriptor* descriptor = parentContext->fds[i]; + if (descriptor == NULL + || (descriptor->open_mode & O_DISCONNECTED) != 0) { + continue; } + + const bool closeOnExec = fd_close_on_exec(parentContext, i); + if (closeOnExec && purgeCloseOnExec) + continue; + + TFD(InheritFD(context, i, descriptor, parentContext)); + + context->fds[i] = descriptor; + context->num_used_fds++; + atomic_add(&descriptor->ref_count, 1); + atomic_add(&descriptor->open_count, 1); + + if (closeOnExec) + fd_set_close_on_exec(context, i, true); } parentLocker.Unlock(); @@ -5012,7 +5011,6 @@ vfs_new_io_context(const io_context* parentContext, bool purgeCloseOnExec) } context->table_size = tableSize; - context->inherit_fds = parentContext != NULL; list_init(&context->node_monitors); context->max_monitors = DEFAULT_NODE_MONITORS; diff --git a/src/system/kernel/team.cpp b/src/system/kernel/team.cpp index b7d4e25ae5..b934792563 100644 --- a/src/system/kernel/team.cpp +++ b/src/system/kernel/team.cpp @@ -1788,8 +1788,9 @@ load_image_internal(char**& _flatArgs, size_t flatArgsSize, int32 argCount, inherit_parent_user_and_group(team, parent); // get a reference to the parent's I/O context -- we need it to create ours - parentIOContext = parent->io_context; - vfs_get_io_context(parentIOContext); + parentIOContext = (parent->id == B_SYSTEM_TEAM) ? NULL : parent->io_context; + if (parentIOContext != NULL) + vfs_get_io_context(parentIOContext); team->Unlock(); parent->UnlockTeamAndProcessGroup(); @@ -1808,18 +1809,18 @@ load_image_internal(char**& _flatArgs, size_t flatArgsSize, int32 argCount, team->SetArgs(path, teamArgs->flat_args + 1, argCount - 1); // create a new io_context for this team + // remove any fds that have the CLOEXEC flag set (emulating BeOS behaviour) team->io_context = vfs_new_io_context(parentIOContext, true); - if (!team->io_context) { + if (team->io_context == NULL) { status = B_NO_MEMORY; goto err2; } - // We don't need the parent's I/O context any longer. - vfs_put_io_context(parentIOContext); - parentIOContext = NULL; - - // remove any fds that have the CLOEXEC flag set (emulating BeOS behaviour) - vfs_exec_io_context(team->io_context); + if (parentIOContext != NULL) { + // We don't need the parent's I/O context any longer. + vfs_put_io_context(parentIOContext); + parentIOContext = NULL; + } // create an address space for this team status = VMAddressSpace::Create(team->id, USER_BASE, USER_SIZE, false,