From 5ecc4b37752beba35afeb7ef446f07c3c6d01971 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Mon, 2 Mar 2009 00:26:22 +0000 Subject: [PATCH] * Introduced ref-counting for the I/O contexts. * The I/O context related vfs_*() functions have io_context* instead of void* parameters/return values, now. * vfs_new_io_context(): Lock the parent I/O context before getting its table size. Otherwise the table size could change until we do. * vfs_resize_fd_table(): Fixed use of MutexLocker. We created only a temporary object, not one with function scope. * Renamed load_image_etc() to load_image_internal() and added a parameter for specifying the parent team of the one to create. * Introduced a kernel private load_image_etc() with a few more arguments than load_image(). git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@29375 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/private/kernel/team.h | 5 +- headers/private/kernel/thread_types.h | 3 +- headers/private/kernel/usergroup.h | 2 + headers/private/kernel/vfs.h | 8 ++- src/system/kernel/fs/vfs.cpp | 40 +++++++----- src/system/kernel/team.cpp | 94 ++++++++++++++++++--------- src/system/kernel/usergroup.cpp | 12 +++- 7 files changed, 109 insertions(+), 55 deletions(-) diff --git a/headers/private/kernel/team.h b/headers/private/kernel/team.h index 8c78a1eae8..9238f3840c 100644 --- a/headers/private/kernel/team.h +++ b/headers/private/kernel/team.h @@ -14,8 +14,6 @@ extern "C" { #endif status_t team_init(struct kernel_args *args); -team_id team_create_team(const char *path, const char *name, char **args, - int argc, char **envp, int envc, int priority); status_t wait_for_team(team_id id, status_t *returnCode); void team_remove_team(struct team *team); void team_delete_team(struct team *team); @@ -36,6 +34,9 @@ struct team *team_get_team_struct_locked(team_id id); int32 team_max_teams(void); int32 team_used_teams(void); +thread_id load_image_etc(int32 argCount, const char* const* args, + const char* const* env, int32 priority, team_id parentID, uint32 flags); + void team_set_job_control_state(struct team* team, job_control_state newState, int signal, bool threadsLocked); void team_set_controlling_tty(int32 index); diff --git a/headers/private/kernel/thread_types.h b/headers/private/kernel/thread_types.h index f557f3d30e..aa8bc0524f 100644 --- a/headers/private/kernel/thread_types.h +++ b/headers/private/kernel/thread_types.h @@ -54,6 +54,7 @@ typedef enum job_control_state { struct image; // defined in image.c +struct io_context; struct realtime_sem_context; // defined in realtime_sem.cpp struct select_info; struct user_thread; // defined in libroot/user_thread.h @@ -172,7 +173,7 @@ struct team { int num_threads; // number of threads in this team int state; // current team state, see above int32 flags; - void *io_context; + struct io_context *io_context; struct realtime_sem_context *realtime_sem_context; struct xsi_sem_context *xsi_sem_context; sem_id death_sem; // semaphore to wait on for dying threads diff --git a/headers/private/kernel/usergroup.h b/headers/private/kernel/usergroup.h index a166cc19cc..5a6152b050 100644 --- a/headers/private/kernel/usergroup.h +++ b/headers/private/kernel/usergroup.h @@ -21,6 +21,8 @@ extern "C" { void inherit_parent_user_and_group(struct team* team, struct team* parent); +void inherit_parent_user_and_group_locked(struct team* team, + struct team* parent); status_t update_set_id_user_and_group(struct team* team, const char* file); // syscalls diff --git a/headers/private/kernel/vfs.h b/headers/private/kernel/vfs.h index eea8a79f47..caca6728e5 100644 --- a/headers/private/kernel/vfs.h +++ b/headers/private/kernel/vfs.h @@ -46,6 +46,7 @@ typedef struct io_context { struct vnode *root; struct vnode *cwd; mutex io_mutex; + int32 ref_count; uint32 table_size; uint32 num_used_fds; struct file_descriptor **fds; @@ -69,9 +70,10 @@ extern "C" { status_t vfs_init(struct kernel_args *args); status_t vfs_bootstrap_file_systems(void); void vfs_mount_boot_file_system(struct kernel_args *args); -void vfs_exec_io_context(void *context); -void *vfs_new_io_context(void *parentContext); -status_t vfs_free_io_context(void *context); +void vfs_exec_io_context(io_context *context); +io_context *vfs_new_io_context(io_context *parentContext); +void vfs_get_io_context(io_context *context); +void vfs_put_io_context(io_context *context); struct rlimit; int vfs_getrlimit(int resource, struct rlimit * rlp); diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp index e74464d1d7..2b39ef8d29 100644 --- a/src/system/kernel/fs/vfs.cpp +++ b/src/system/kernel/fs/vfs.cpp @@ -4595,9 +4595,8 @@ vfs_unlock_vnode_if_locked(struct file_descriptor* descriptor) have the O_CLOEXEC flag set. */ void -vfs_exec_io_context(void* _context) +vfs_exec_io_context(io_context* context) { - struct io_context* context = (struct io_context*)_context; uint32 i; for (i = 0; i < context->table_size; i++) { @@ -4626,23 +4625,24 @@ vfs_exec_io_context(void* _context) /*! Sets up a new io_control structure, and inherits the properties of the parent io_control if it is given. */ -void* -vfs_new_io_context(void* _parentContext) +io_context* +vfs_new_io_context(io_context* parentContext) { size_t tableSize; struct io_context* context; - struct io_context* parentContext; context = (io_context*)malloc(sizeof(struct io_context)); if (context == NULL) return NULL; memset(context, 0, sizeof(struct io_context)); + context->ref_count = 1; - parentContext = (struct io_context*)_parentContext; - if (parentContext) + MutexLocker parentLocker; + if (parentContext) { + parentLocker.SetTo(parentContext->io_mutex, false); tableSize = parentContext->table_size; - else + } else tableSize = DEFAULT_FD_TABLE_SIZE; // allocate space for FDs and their close-on-exec flag @@ -4669,8 +4669,6 @@ vfs_new_io_context(void* _parentContext) if (parentContext) { size_t i; - mutex_lock(&parentContext->io_mutex); - mutex_lock(&sIOContextRootLock); context->root = parentContext->root; if (context->root) @@ -4695,7 +4693,7 @@ vfs_new_io_context(void* _parentContext) } } - mutex_unlock(&parentContext->io_mutex); + parentLocker.Unlock(); } else { context->root = sRoot; context->cwd = sRoot; @@ -4716,10 +4714,9 @@ vfs_new_io_context(void* _parentContext) } -status_t -vfs_free_io_context(void* _ioContext) +static status_t +vfs_free_io_context(io_context* context) { - struct io_context* context = (struct io_context*)_ioContext; uint32 i; if (context->root) @@ -4747,13 +4744,26 @@ vfs_free_io_context(void* _ioContext) } +void vfs_get_io_context(io_context *context) +{ + atomic_add(&context->ref_count, 1); +} + + +void vfs_put_io_context(io_context *context) +{ + if (atomic_add(&context->ref_count, -1) == 1) + vfs_free_io_context(context); +} + + static status_t vfs_resize_fd_table(struct io_context* context, const int newSize) { if (newSize <= 0 || newSize > MAX_FD_TABLE_SIZE) return EINVAL; - MutexLocker(context->io_mutex); + MutexLocker _(context->io_mutex); int oldSize = context->table_size; int oldCloseOnExitBitmapSize = (oldSize + 7) / 8; diff --git a/src/system/kernel/team.cpp b/src/system/kernel/team.cpp index 08e37ca875..f1acc96e01 100644 --- a/src/system/kernel/team.cpp +++ b/src/system/kernel/team.cpp @@ -1085,37 +1085,33 @@ team_create_thread_start(void *args) } -/*! The BeOS kernel exports a function with this name, but most probably with - different parameters; we should not make it public. -*/ static thread_id -load_image_etc(char**& _flatArgs, size_t flatArgsSize, int32 argCount, - int32 envCount, int32 priority, uint32 flags, port_id errorPort, - uint32 errorToken) +load_image_internal(char**& _flatArgs, size_t flatArgsSize, int32 argCount, + int32 envCount, int32 priority, team_id parentID, uint32 flags, + port_id errorPort, uint32 errorToken) { char** flatArgs = _flatArgs; - struct team *team, *parent; + struct team *team; const char *threadName; thread_id thread; status_t status; cpu_status state; struct team_arg *teamArgs; struct team_loading_info loadingInfo; + io_context* parentIOContext = NULL; if (flatArgs == NULL || argCount == 0) return B_BAD_VALUE; const char* path = flatArgs[0]; - TRACE(("load_image_etc: name '%s', args = %p, argCount = %ld\n", + TRACE(("load_image_internal: name '%s', args = %p, argCount = %ld\n", path, flatArgs, argCount)); team = create_team_struct(path, false); if (team == NULL) return B_NO_MEMORY; - parent = thread_get_current_thread()->team; - if (flags & B_WAIT_TILL_LOADED) { loadingInfo.thread = thread_get_current_thread(); loadingInfo.result = B_ERROR; @@ -1123,21 +1119,38 @@ load_image_etc(char**& _flatArgs, size_t flatArgsSize, int32 argCount, team->loading_info = &loadingInfo; } - // Inherit the parent's user/group, but also check the executable's - // set-user/group-id permission - inherit_parent_user_and_group(team, parent); - update_set_id_user_and_group(team, path); + InterruptsSpinLocker teamLocker(gTeamSpinlock); - state = disable_interrupts(); - GRAB_TEAM_LOCK(); + // get the parent team + struct team* parent; + + if (parentID == B_CURRENT_TEAM) + parent = thread_get_current_thread()->team; + else + parent = team_get_team_struct_locked(parentID); + + if (parent == NULL) { + teamLocker.Unlock(); + status = B_BAD_TEAM_ID; + goto err0; + } + + // inherit the parent's user/group + inherit_parent_user_and_group_locked(team, parent); hash_insert(sTeamHash, team); insert_team_into_parent(parent, team); insert_team_into_group(parent->group, team); sUsedTeams++; - RELEASE_TEAM_LOCK(); - restore_interrupts(state); + // 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); + + teamLocker.Unlock(); + + // check the executable's set-user/group-id permission + update_set_id_user_and_group(team, path); status = create_team_arg(&teamArgs, path, flatArgs, flatArgsSize, argCount, envCount, errorPort, errorToken); @@ -1149,12 +1162,16 @@ load_image_etc(char**& _flatArgs, size_t flatArgsSize, int32 argCount, // args are owned by the team_arg structure now // create a new io_context for this team - team->io_context = vfs_new_io_context(parent->io_context); + team->io_context = vfs_new_io_context(parentIOContext); if (!team->io_context) { 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); @@ -1229,21 +1246,25 @@ err5: err4: vm_put_address_space(team->address_space); err3: - vfs_free_io_context(team->io_context); + vfs_put_io_context(team->io_context); err2: free_team_arg(teamArgs); err1: + if (parentIOContext != NULL) + vfs_put_io_context(parentIOContext); + // remove the team structure from the team hash table and delete the team structure state = disable_interrupts(); GRAB_TEAM_LOCK(); remove_team_from_group(team); - remove_team_from_parent(parent, team); + remove_team_from_parent(team->parent, team); hash_remove(sTeamHash, team); RELEASE_TEAM_LOCK(); restore_interrupts(state); +err0: delete_team_struct(team); return status; @@ -1431,7 +1452,8 @@ fork_team(void) return B_NOT_ALLOWED; // create a new team - // ToDo: this is very similar to team_create_team() - maybe we can do something about it :) + // TODO: this is very similar to load_image_internal() - maybe we can do + // something about it :) team = create_team_struct(parentTeam->name, false); if (team == NULL) @@ -1562,7 +1584,7 @@ err4: err3: delete_realtime_sem_context(team->realtime_sem_context); err25: - vfs_free_io_context(team->io_context); + vfs_put_io_context(team->io_context); err2: free(forkArgs); err1: @@ -2354,7 +2376,7 @@ team_delete_team(struct team *team) // free team resources - vfs_free_io_context(team->io_context); + vfs_put_io_context(team->io_context); delete_realtime_sem_context(team->realtime_sem_context); xsi_sem_undo(team); delete_owned_ports(teamID); @@ -2638,6 +2660,15 @@ team_free_user_thread(struct thread* thread) thread_id load_image(int32 argCount, const char **args, const char **env) +{ + return load_image_etc(argCount, args, env, B_NORMAL_PRIORITY, + B_CURRENT_TEAM, B_WAIT_TILL_LOADED); +} + + +thread_id +load_image_etc(int32 argCount, const char* const* args, + const char* const* env, int32 priority, team_id parentID, uint32 flags) { // we need to flatten the args and environment @@ -2685,11 +2716,11 @@ load_image(int32 argCount, const char **args, const char **env) *slot++ = NULL; - thread_id thread = load_image_etc(flatArgs, size, argCount, envCount, - B_NORMAL_PRIORITY, B_WAIT_TILL_LOADED, -1, 0); + thread_id thread = load_image_internal(flatArgs, size, argCount, envCount, + B_NORMAL_PRIORITY, parentID, B_WAIT_TILL_LOADED, -1, 0); free(flatArgs); - // load_image_etc() unset our variable if it took over ownership + // load_image_internal() unset our variable if it took over ownership return thread; } @@ -3277,7 +3308,7 @@ _user_load_image(const char* const* userFlatArgs, size_t flatArgsSize, int32 argCount, int32 envCount, int32 priority, uint32 flags, port_id errorPort, uint32 errorToken) { - TRACE(("_user_load_image_etc: argc = %ld\n", argCount)); + TRACE(("_user_load_image: argc = %ld\n", argCount)); if (argCount < 1) return B_BAD_VALUE; @@ -3289,11 +3320,12 @@ _user_load_image(const char* const* userFlatArgs, size_t flatArgsSize, if (error != B_OK) return error; - thread_id thread = load_image_etc(flatArgs, _ALIGN(flatArgsSize), argCount, - envCount, priority, flags, errorPort, errorToken); + thread_id thread = load_image_internal(flatArgs, _ALIGN(flatArgsSize), + argCount, envCount, priority, B_CURRENT_TEAM, flags, errorPort, + errorToken); free(flatArgs); - // load_image_etc() unset our variable if it took over ownership + // load_image_internal() unset our variable if it took over ownership return thread; } diff --git a/src/system/kernel/usergroup.cpp b/src/system/kernel/usergroup.cpp index 0ca121dbb1..f13411d738 100644 --- a/src/system/kernel/usergroup.cpp +++ b/src/system/kernel/usergroup.cpp @@ -243,10 +243,8 @@ common_setgroups(int groupCount, const gid_t* groupList, bool kernel) void -inherit_parent_user_and_group(struct team* team, struct team* parent) +inherit_parent_user_and_group_locked(struct team* team, struct team* parent) { - InterruptsSpinLocker _(gTeamSpinlock); - team->saved_set_uid = parent->saved_set_uid; team->real_uid = parent->real_uid; team->effective_uid = parent->effective_uid; @@ -260,6 +258,14 @@ inherit_parent_user_and_group(struct team* team, struct team* parent) } +void +inherit_parent_user_and_group(struct team* team, struct team* parent) +{ + InterruptsSpinLocker _(gTeamSpinlock); + inherit_parent_user_and_group_locked(team, parent); +} + + status_t update_set_id_user_and_group(struct team* team, const char* file) {