diff --git a/binfmt/CMakeLists.txt b/binfmt/CMakeLists.txt index 9af12e1a3b..20a780fbea 100644 --- a/binfmt/CMakeLists.txt +++ b/binfmt/CMakeLists.txt @@ -36,6 +36,7 @@ list( binfmt_execmodule.c binfmt_exec.c binfmt_copyargv.c + binfmt_copyactions.c binfmt_dumpmodule.c binfmt_coredump.c) diff --git a/binfmt/Makefile b/binfmt/Makefile index 004ef03a8a..f4356319f4 100644 --- a/binfmt/Makefile +++ b/binfmt/Makefile @@ -24,7 +24,7 @@ include $(TOPDIR)/Make.defs CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c -CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c +CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_copyactions.c binfmt_dumpmodule.c CSRCS += binfmt_coredump.c ifeq ($(CONFIG_BINFMT_LOADABLE),y) diff --git a/binfmt/binfmt.h b/binfmt/binfmt.h index a769ad93a5..d4ae019803 100644 --- a/binfmt/binfmt.h +++ b/binfmt/binfmt.h @@ -165,6 +165,51 @@ void binfmt_freeargv(FAR char * const *argv); # define binfmt_freeenv(envp) #endif +/**************************************************************************** + * Name: binfmt_copyactions + * + * Description: + * In the kernel build, the file actions will likely lie in the caller's + * address environment and, hence, be inaccessible when we switch to the + * address environment of the new process address environment. So we + * do not have any real option other than to copy the callers action list. + * + * Input Parameters: + * copy - Pointer of file actions + * + * Returned Value: + * A non-zero copy is returned on success. + * + ****************************************************************************/ + +#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL) +int binfmt_copyactions(FAR const posix_spawn_file_actions_t **copy, + FAR const posix_spawn_file_actions_t *actions); +#else +# define binfmt_copyactions(copy, actp) \ + (*(copy) = (FAR posix_spawn_file_actions_t *)(actp), 0) +#endif + +/**************************************************************************** + * Name: binfmt_freeactions + * + * Description: + * Release the copied file action list. + * + * Input Parameters: + * copy - Pointer of file actions + * + * Returned Value: + * None + * + ****************************************************************************/ + +#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL) +void binfmt_freeactions(FAR const posix_spawn_file_actions_t *copy); +#else +# define binfmt_freeactions(copy) +#endif + #ifdef CONFIG_BUILTIN /**************************************************************************** * Name: builtin_initialize diff --git a/binfmt/binfmt_copyactions.c b/binfmt/binfmt_copyactions.c new file mode 100644 index 0000000000..cb941ea09e --- /dev/null +++ b/binfmt/binfmt_copyactions.c @@ -0,0 +1,194 @@ +/**************************************************************************** + * binfmt/binfmt_copyactions.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include + +#include +#include +#include + +#include +#include + +#include "binfmt.h" + +#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL) && !defined(CONFIG_BINFMT_DISABLE) + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define ROUNDUP(x, y) (((x) + (y) - 1) / (y) * (y)) + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: binfmt_copyactions + * + * Description: + * In the kernel build, the file actions will likely lie in the caller's + * address environment and, hence, be inaccessible when we switch to the + * address environment of the new process address environment. So we + * do not have any real option other than to copy the callers action list. + * + * Input Parameters: + * copy - Pointer of file actions + * + * Returned Value: + * A non-zero copy is returned on success. + * + ****************************************************************************/ + +int binfmt_copyactions(FAR const posix_spawn_file_actions_t **copy, + FAR const posix_spawn_file_actions_t *actions) +{ + FAR struct spawn_general_file_action_s *entry; + FAR struct spawn_general_file_action_s *prev; + FAR struct spawn_close_file_action_s *close; + FAR struct spawn_open_file_action_s *open; + FAR struct spawn_open_file_action_s *tmp; + FAR struct spawn_dup2_file_action_s *dup2; + FAR void *buffer; + int size = 0; + + if (actions == NULL) + { + *copy = NULL; + return OK; + } + + for (entry = (FAR struct spawn_general_file_action_s *)actions; + entry != NULL; + entry = entry->flink) + { + switch (entry->action) + { + case SPAWN_FILE_ACTION_CLOSE: + size += sizeof(struct spawn_close_file_action_s); + break; + + case SPAWN_FILE_ACTION_DUP2: + size += sizeof(struct spawn_dup2_file_action_s); + break; + + case SPAWN_FILE_ACTION_OPEN: + open = (FAR struct spawn_open_file_action_s *)entry; + size += ROUNDUP(SIZEOF_OPEN_FILE_ACTION_S(strlen(open->path)), + sizeof(FAR void *)); + break; + + default: + return -EINVAL; + } + } + + *copy = buffer = kmm_malloc(size); + if (buffer == NULL) + { + return -ENOMEM; + } + + for (entry = (FAR struct spawn_general_file_action_s *)actions, + prev = NULL; entry != NULL; prev = entry, entry = entry->flink) + { + switch (entry->action) + { + case SPAWN_FILE_ACTION_CLOSE: + close = buffer; + memcpy(close, entry, sizeof(struct spawn_close_file_action_s)); + close->flink = NULL; + if (prev) + { + prev->flink = (FAR void *)close; + } + + buffer = close + 1; + break; + + case SPAWN_FILE_ACTION_DUP2: + dup2 = buffer; + memcpy(dup2, entry, sizeof(struct spawn_dup2_file_action_s)); + dup2->flink = NULL; + if (prev) + { + prev->flink = (FAR void *)dup2; + } + + buffer = dup2 + 1; + break; + + case SPAWN_FILE_ACTION_OPEN: + tmp = (FAR struct spawn_open_file_action_s *)entry; + open = buffer; + memcpy(open, entry, sizeof(struct spawn_open_file_action_s)); + open->flink = NULL; + if (prev) + { + prev->flink = (FAR void *)open; + } + + strcpy(open->path, tmp->path); + + buffer = (FAR char *)buffer + + ROUNDUP(SIZEOF_OPEN_FILE_ACTION_S(strlen(tmp->path)), + sizeof(FAR void *)); + break; + + default: + break; + } + } + + return OK; +} + +/**************************************************************************** + * Name: binfmt_freeactions + * + * Description: + * Release the copied file action list. + * + * Input Parameters: + * copy - Pointer of file actions + * + * Returned Value: + * None + * + ****************************************************************************/ + +void binfmt_freeactions(FAR const posix_spawn_file_actions_t *copy) +{ + /* Is there an allocated argument buffer */ + + if (copy != NULL) + { + /* Free the argument buffer */ + + kmm_free((FAR void *)copy); + } +} + +#endif diff --git a/binfmt/binfmt_execmodule.c b/binfmt/binfmt_execmodule.c index 59b008cd5c..3859a0bfe4 100644 --- a/binfmt/binfmt_execmodule.c +++ b/binfmt/binfmt_execmodule.c @@ -256,6 +256,12 @@ int exec_module(FAR struct binary_s *binp, goto errout_with_args; } + ret = binfmt_copyactions(&actions, actions); + if (ret < 0) + { + goto errout_with_envp; + } + #if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL) /* If there is no argument vector, the process name must be copied here */ @@ -271,7 +277,7 @@ int exec_module(FAR struct binary_s *binp, if (ret < 0) { berr("ERROR: addrenv_select() failed: %d\n", ret); - goto errout_with_envp; + goto errout_with_actions; } ret = up_addrenv_vheap(addrenv, &vheap); @@ -310,12 +316,14 @@ int exec_module(FAR struct binary_s *binp, if (argv && argv[0]) { ret = nxtask_init(tcb, argv[0], binp->priority, stackaddr, - binp->stacksize, binp->entrypt, &argv[1], envp); + binp->stacksize, binp->entrypt, &argv[1], + envp, actions); } else { ret = nxtask_init(tcb, filename, binp->priority, stackaddr, - binp->stacksize, binp->entrypt, argv, envp); + binp->stacksize, binp->entrypt, argv, + envp, actions); } if (ret < 0) @@ -326,6 +334,7 @@ int exec_module(FAR struct binary_s *binp, /* The copied argv and envp can now be released */ + binfmt_freeactions(actions); binfmt_freeargv(argv); binfmt_freeenv(envp); @@ -376,10 +385,6 @@ int exec_module(FAR struct binary_s *binp, } #endif - /* Close the file descriptors with O_CLOEXEC before active task */ - - files_close_onexec(&tcb->cmn); - if (!spawn) { exec_swap(this_task(), (FAR struct tcb_s *)tcb); @@ -400,17 +405,6 @@ int exec_module(FAR struct binary_s *binp, } #endif - /* Perform file actions */ - - if (actions != NULL) - { - ret = spawn_file_actions(&tcb->cmn, actions); - if (ret < 0) - { - goto errout_with_tcbinit; - } - } - /* Set the attributes */ if (attr) @@ -442,8 +436,10 @@ errout_with_tcbinit: errout_with_addrenv: #if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL) addrenv_restore(binp->oldenv); -errout_with_envp: +errout_with_actions: + binfmt_freeactions(actions); #endif +errout_with_envp: binfmt_freeenv(envp); errout_with_args: binfmt_freeargv(argv); diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c index e61728832f..8729f48e19 100644 --- a/fs/inode/fs_files.c +++ b/fs/inode/fs_files.c @@ -36,6 +36,7 @@ #include #include #include +#include #ifdef CONFIG_FDSAN # include @@ -462,20 +463,23 @@ int file_allocate(FAR struct inode *inode, int oflags, off_t pos, * ****************************************************************************/ -int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist) +int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist, + FAR const posix_spawn_file_actions_t *actions, + bool cloexec) { + bool fcloexec; int ret; + int fd; int i; int j; - DEBUGASSERT(clist); - DEBUGASSERT(plist); - for (i = 0; i < plist->fl_rows; i++) { for (j = 0; j < CONFIG_NFILE_DESCRIPTORS_PER_BLOCK; j++) { FAR struct file *filep; + + fd = i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j; #ifdef CONFIG_FDCLONE_STDIO /* Determine how many file descriptors to clone. If @@ -485,7 +489,7 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist) * cloned. Otherwise all file descriptors will be cloned. */ - if (i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j >= 3) + if (fd >= 3) { return OK; } @@ -497,6 +501,22 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist) continue; } + fcloexec = (cloexec && (filep->f_oflags & O_CLOEXEC)); + + /* Skip file dup if file action is unnecessary to duplicate */ + + if (actions != NULL) + { + if (!spawn_file_is_duplicateable(actions, fd, fcloexec)) + { + continue; + } + } + else if (fcloexec) + { + continue; + } + ret = files_extend(clist, i + 1); if (ret < 0) { @@ -505,8 +525,7 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist) /* Yes... duplicate it for the child, include O_CLOEXEC flag. */ - ret = file_dup3(filep, files_fget_by_index(clist, i, j), - filep->f_oflags & O_CLOEXEC ? O_CLOEXEC : 0); + ret = file_dup2(filep, files_fget_by_index(clist, i, j)); if (ret < 0) { return ret; @@ -517,41 +536,6 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist) return OK; } -/**************************************************************************** - * Name: files_close_onexec - * - * Description: - * Close specified task's file descriptors with O_CLOEXEC before exec. - * - ****************************************************************************/ - -void files_close_onexec(FAR struct tcb_s *tcb) -{ - FAR struct filelist *list; - int i; - int j; - - /* Get the file descriptor list. It should not be NULL in this context. */ - - list = nxsched_get_files_from_tcb(tcb); - DEBUGASSERT(list != NULL); - - for (i = 0; i < list->fl_rows; i++) - { - for (j = 0; j < CONFIG_NFILE_DESCRIPTORS_PER_BLOCK; j++) - { - FAR struct file *filep; - - filep = files_fget_by_index(list, i, j); - if (filep->f_inode != NULL && - (filep->f_oflags & O_CLOEXEC) != 0) - { - file_close(filep); - } - } - } -} - /**************************************************************************** * Name: fs_getfilep * diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h index 7e3813695c..0016b87b09 100644 --- a/include/nuttx/fs/fs.h +++ b/include/nuttx/fs/fs.h @@ -39,6 +39,7 @@ #include #include #include +#include /**************************************************************************** * Pre-processor Definitions @@ -877,17 +878,9 @@ int files_countlist(FAR struct filelist *list); * ****************************************************************************/ -int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist); - -/**************************************************************************** - * Name: files_close_onexec - * - * Description: - * Close specified task's file descriptors with O_CLOEXEC before exec. - * - ****************************************************************************/ - -void files_close_onexec(FAR struct tcb_s *tcb); +int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist, + FAR const posix_spawn_file_actions_t *actions, + bool cloexec); /**************************************************************************** * Name: file_allocate_from_tcb diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h index c9581456ed..4dd1468fc5 100644 --- a/include/nuttx/sched.h +++ b/include/nuttx/sched.h @@ -923,7 +923,8 @@ FAR struct filelist *nxsched_get_files(void); int nxtask_init(FAR struct task_tcb_s *tcb, const char *name, int priority, FAR void *stack, uint32_t stack_size, main_t entry, - FAR char * const argv[], FAR char * const envp[]); + FAR char * const argv[], FAR char * const envp[], + FAR const posix_spawn_file_actions_t *actions); /**************************************************************************** * Name: nxtask_uninit diff --git a/include/nuttx/spawn.h b/include/nuttx/spawn.h index c552e15f9a..5e0f4da572 100644 --- a/include/nuttx/spawn.h +++ b/include/nuttx/spawn.h @@ -100,9 +100,14 @@ extern "C" void add_file_action(FAR posix_spawn_file_actions_t *file_action, FAR struct spawn_general_file_action_s *entry); +struct tcb_s; int spawn_file_actions(FAR struct tcb_s *tcb, FAR const posix_spawn_file_actions_t *actions); +bool +spawn_file_is_duplicateable(FAR const posix_spawn_file_actions_t *actions, + int fd, bool cloexec); + #ifdef __cplusplus } #endif diff --git a/sched/group/group.h b/sched/group/group.h index ee07650323..839e4e2710 100644 --- a/sched/group/group.h +++ b/sched/group/group.h @@ -120,6 +120,8 @@ void group_remove_children(FAR struct task_group_s *group); /* Group data resource configuration */ int group_setupidlefiles(void); -int group_setuptaskfiles(FAR struct task_tcb_s *tcb); +int group_setuptaskfiles(FAR struct task_tcb_s *tcb, + FAR const posix_spawn_file_actions_t *actions, + bool cloexec); #endif /* __SCHED_GROUP_GROUP_H */ diff --git a/sched/group/group_setuptaskfiles.c b/sched/group/group_setuptaskfiles.c index 1345252f2e..4c074da887 100644 --- a/sched/group/group_setuptaskfiles.c +++ b/sched/group/group_setuptaskfiles.c @@ -45,7 +45,9 @@ * file descriptors and streams from the parent task. * * Input Parameters: - * tcb - tcb of the new task. + * tcb - tcb of the new task. + * actions - The spawn file actions + * cloexec - Perform O_CLOEXEC on setup task files * * Returned Value: * Zero (OK) is returned on success; A negated errno value is returned on @@ -55,7 +57,9 @@ * ****************************************************************************/ -int group_setuptaskfiles(FAR struct task_tcb_s *tcb) +int group_setuptaskfiles(FAR struct task_tcb_s *tcb, + FAR const posix_spawn_file_actions_t *actions, + bool cloexec) { FAR struct task_group_s *group = tcb->cmn.group; int ret = OK; @@ -75,7 +79,12 @@ int group_setuptaskfiles(FAR struct task_tcb_s *tcb) /* Duplicate the parent task's file descriptors */ - ret = files_duplist(&rtcb->group->tg_filelist, &group->tg_filelist); + ret = files_duplist(&rtcb->group->tg_filelist, + &group->tg_filelist, actions, cloexec); + if (ret >= 0 && actions != NULL) + { + ret = spawn_file_actions(&tcb->cmn, actions); + } #endif sched_trace_end(); diff --git a/sched/init/nx_start.c b/sched/init/nx_start.c index 9398a417aa..176c010999 100644 --- a/sched/init/nx_start.c +++ b/sched/init/nx_start.c @@ -649,7 +649,7 @@ void nx_start(void) { /* Clone stdout, stderr, stdin from the CPU0 IDLE task. */ - DEBUGVERIFY(group_setuptaskfiles(&g_idletcb[i])); + DEBUGVERIFY(group_setuptaskfiles(&g_idletcb[i], NULL, true)); } else { diff --git a/sched/task/task_create.c b/sched/task/task_create.c index f59eca8eac..17a84202d0 100644 --- a/sched/task/task_create.c +++ b/sched/task/task_create.c @@ -96,17 +96,13 @@ int nxthread_create(FAR const char *name, uint8_t ttype, int priority, /* Initialize the task */ ret = nxtask_init(tcb, name, priority, stack_addr, stack_size, - entry, argv, envp); + entry, argv, envp, NULL); if (ret < OK) { kmm_free(tcb); return ret; } - /* Close the file descriptors with O_CLOEXEC before active task */ - - files_close_onexec(&tcb->cmn); - /* Get the assigned pid before we start the task */ pid = tcb->cmn.pid; diff --git a/sched/task/task_fork.c b/sched/task/task_fork.c index bfb2d89a70..a10db44087 100644 --- a/sched/task/task_fork.c +++ b/sched/task/task_fork.c @@ -160,7 +160,7 @@ FAR struct task_tcb_s *nxtask_setup_fork(start_t retaddr) /* Associate file descriptors with the new task */ - ret = group_setuptaskfiles(child); + ret = group_setuptaskfiles(child, NULL, false); if (ret < OK) { goto errout_with_tcb; diff --git a/sched/task/task_init.c b/sched/task/task_init.c index 5ff5355ead..bce71184cd 100644 --- a/sched/task/task_init.c +++ b/sched/task/task_init.c @@ -86,7 +86,8 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char *name, int priority, FAR void *stack, uint32_t stack_size, main_t entry, FAR char * const argv[], - FAR char * const envp[]) + FAR char * const envp[], + FAR const posix_spawn_file_actions_t *actions) { uint8_t ttype = tcb->cmn.flags & TCB_FLAG_TTYPE_MASK; int ret; @@ -127,7 +128,7 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char *name, int priority, /* Associate file descriptors with the new task */ - ret = group_setuptaskfiles(tcb); + ret = group_setuptaskfiles(tcb, actions, true); if (ret < 0) { goto errout_with_group; diff --git a/sched/task/task_spawn.c b/sched/task/task_spawn.c index 44cc924f7c..9554fdaad2 100644 --- a/sched/task/task_spawn.c +++ b/sched/task/task_spawn.c @@ -104,7 +104,7 @@ static int nxtask_spawn_create(FAR const char *name, int priority, /* Initialize the task */ ret = nxtask_init(tcb, name, priority, stack_addr, stack_size, - entry, argv, envp); + entry, argv, envp, actions); if (ret < OK) { kmm_free(tcb); @@ -115,21 +115,6 @@ static int nxtask_spawn_create(FAR const char *name, int priority, pid = tcb->cmn.pid; - /* Perform file actions */ - - if (actions != NULL) - { - ret = spawn_file_actions(&tcb->cmn, actions); - if (ret < 0) - { - goto errout_with_taskinit; - } - } - - /* Close the file descriptors with O_CLOEXEC before active task */ - - files_close_onexec(&tcb->cmn); - /* Set the attributes */ if (attr) diff --git a/sched/task/task_spawnparms.c b/sched/task/task_spawnparms.c index bf032a4c8d..a9d94c4d42 100644 --- a/sched/task/task_spawnparms.c +++ b/sched/task/task_spawnparms.c @@ -247,7 +247,7 @@ int spawn_execattrs(pid_t pid, FAR const posix_spawnattr_t *attr) * * Input Parameters: * - * attr - The spawn file actions + * actions - The spawn file actions * * Returned Value: * 0 (OK) on success; A negated errno value is returned on failure. @@ -296,3 +296,77 @@ int spawn_file_actions(FAR struct tcb_s *tcb, return ret; } + +/**************************************************************************** + * Name: spawn_file_is_duplicateable + * + * Description: + * Check the input file descriptor is duplicateable from spawn actions + * + * Input Parameters: + * + * actions - The spawn file actions + * fd - file descriptor + * + * Returned Value: + * True is returned if file descriptor is duplicate able + * + ****************************************************************************/ + +bool +spawn_file_is_duplicateable(FAR const posix_spawn_file_actions_t *actions, + int fd, bool cloexec) +{ + FAR struct spawn_general_file_action_s *entry; + FAR struct spawn_close_file_action_s *close; + FAR struct spawn_open_file_action_s *open; + FAR struct spawn_dup2_file_action_s *dup2; + + /* check each file action */ + + for (entry = (FAR struct spawn_general_file_action_s *)actions; + entry != NULL; + entry = entry->flink) + { + switch (entry->action) + { + case SPAWN_FILE_ACTION_CLOSE: + close = (FAR struct spawn_close_file_action_s *)entry; + if (close->fd == fd) + { + return false; + } + break; + + case SPAWN_FILE_ACTION_DUP2: + dup2 = (FAR struct spawn_dup2_file_action_s *)entry; + if (dup2->fd1 == fd) + { + return true; + } + else if (dup2->fd2 == fd) + { + return false; + } + break; + + case SPAWN_FILE_ACTION_OPEN: + open = (FAR struct spawn_open_file_action_s *)entry; + if (open->fd == fd) + { + return false; + } + break; + + default: + break; + } + } + + if (cloexec) + { + return false; + } + + return true; +}