env_dup: Fix copying of env between address environments

If address environments are in use, it is not possible to simply
memcpy from from one process to another. The current implementation
of env_dup does precisely this and thus, it fails at once when it is
attempted between two user processes.

The solution is to use the kernel's heap as an intermediate buffer.
This is a simple, effective and common way to do a fork().

Obviously this is not needed for kernel processes.
This commit is contained in:
Ville Juven 2022-04-04 13:11:06 +03:00 committed by Xiang Xiao
parent 6b1ee4c2e2
commit 4c1b66246d
17 changed files with 133 additions and 46 deletions

View File

@ -117,6 +117,50 @@ void binfmt_freeargv(FAR char * const *argv);
# define binfmt_freeargv(argv)
#endif
/****************************************************************************
* Name: binfmt_copyenv
*
* Description:
* In the kernel build, the environment exists in the parent's address
* environment and, hence, will be inaccessible when we switch to the
* address environment of the new process. So we do not have any real
* option other than to copy the parents envp list into an intermediate
* buffer that resides in neutral kernel memory.
*
* Input Parameters:
* envp - Allocated environment strings
*
* Returned Value:
* A non-zero copy is returned on success.
*
****************************************************************************/
#ifndef CONFIG_DISABLE_ENVIRON
# define binfmt_copyenv(envp) binfmt_copyargv(envp)
#else
# define binfmt_copyenv(envp) (envp)
#endif
/****************************************************************************
* Name: binfmt_freeenv
*
* Description:
* Release the copied envp[] list.
*
* Input Parameters:
* envp - Allocated environment strings
*
* Returned Value:
* None
*
****************************************************************************/
#ifndef CONFIG_DISABLE_ENVIRON
# define binfmt_freeenv(envp) binfmt_freeargv(envp)
#else
# define binfmt_freeenv(envp)
#endif
/****************************************************************************
* Name: builtin_initialize
*

View File

@ -55,6 +55,8 @@
* program.
* argv - A pointer to an array of string arguments. The end of the
* array is indicated with a NULL entry.
* envp - A pointer to an array of environment strings. Terminated with
* a NULL entry.
* exports - The address of the start of the caller-provided symbol
* table. This symbol table contains the addresses of symbols
* exported by the caller and made available for linking the
@ -69,8 +71,8 @@
****************************************************************************/
int exec_spawn(FAR const char *filename, FAR char * const *argv,
FAR const struct symtab_s *exports, int nexports,
FAR const posix_spawnattr_t *attr)
FAR char * const *envp, FAR const struct symtab_s *exports,
int nexports, FAR const posix_spawnattr_t *attr)
{
FAR struct binary_s *bin;
int pid;
@ -121,7 +123,7 @@ int exec_spawn(FAR const char *filename, FAR char * const *argv,
/* Then start the module */
pid = exec_module(bin, filename, argv);
pid = exec_module(bin, filename, argv, envp);
if (pid < 0)
{
ret = pid;
@ -228,7 +230,7 @@ int exec(FAR const char *filename, FAR char * const *argv,
{
int ret;
ret = exec_spawn(filename, argv, exports, nexports, NULL);
ret = exec_spawn(filename, argv, NULL, exports, nexports, NULL);
if (ret < 0)
{
set_errno(-ret);

View File

@ -112,7 +112,8 @@ static void exec_ctors(FAR void *arg)
****************************************************************************/
int exec_module(FAR const struct binary_s *binp,
FAR const char *filename, FAR char * const *argv)
FAR const char *filename, FAR char * const *argv,
FAR char * const *envp)
{
FAR struct task_tcb_s *tcb;
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
@ -150,6 +151,18 @@ int exec_module(FAR const struct binary_s *binp,
}
}
/* Make a copy of the environment here */
if (envp)
{
envp = binfmt_copyenv(envp);
if (!envp)
{
ret = -ENOMEM;
goto errout_with_args;
}
}
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Instantiate the address environment containing the user heap */
@ -157,7 +170,7 @@ int exec_module(FAR const struct binary_s *binp,
if (ret < 0)
{
berr("ERROR: up_addrenv_select() failed: %d\n", ret);
goto errout_with_args;
goto errout_with_envp;
}
binfo("Initialize the user heap (heapsize=%d)\n", binp->addrenv.heapsize);
@ -173,12 +186,12 @@ int exec_module(FAR const struct binary_s *binp,
if (argv && argv[0])
{
ret = nxtask_init(tcb, argv[0], binp->priority, NULL,
binp->stacksize, binp->entrypt, &argv[1]);
binp->stacksize, binp->entrypt, &argv[1], envp);
}
else
{
ret = nxtask_init(tcb, filename, binp->priority, NULL,
binp->stacksize, binp->entrypt, argv);
binp->stacksize, binp->entrypt, argv, envp);
}
if (ret < 0)
@ -187,7 +200,10 @@ int exec_module(FAR const struct binary_s *binp,
goto errout_with_addrenv;
}
/* The copied argv and envp can now be released */
binfmt_freeargv(argv);
binfmt_freeenv(envp);
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_ARCH_KERNEL_STACK)
/* Allocate the kernel stack */
@ -281,7 +297,9 @@ errout_with_tcbinit:
errout_with_addrenv:
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
up_addrenv_restore(&oldenv);
errout_with_envp:
#endif
binfmt_freeenv(envp);
errout_with_args:
binfmt_freeargv(argv);
errout_with_tcb:

View File

@ -42,7 +42,9 @@ namespace std
// Environment variable support
#ifndef CONFIG_DISABLE_ENVIRON
using ::get_environ_ptr;
#endif
using ::getenv;
using ::putenv;
using ::clearenv;

View File

@ -258,7 +258,8 @@ int unload_module(FAR struct binary_s *bin);
****************************************************************************/
int exec_module(FAR const struct binary_s *binp,
FAR const char *filename, FAR char * const *argv);
FAR const char *filename, FAR char * const *argv,
FAR char * const *envp);
/****************************************************************************
* Name: exec
@ -338,6 +339,8 @@ int exec(FAR const char *filename, FAR char * const *argv,
* program.
* argv - A pointer to an array of string arguments. The end of the
* array is indicated with a NULL entry.
* envp - A pointer to an array of environment strings. Terminated with
* a NULL entry.
* exports - The address of the start of the caller-provided symbol
* table. This symbol table contains the addresses of symbols
* exported by the caller and made available for linking the
@ -353,8 +356,8 @@ int exec(FAR const char *filename, FAR char * const *argv,
****************************************************************************/
int exec_spawn(FAR const char *filename, FAR char * const *argv,
FAR const struct symtab_s *exports, int nexports,
FAR const posix_spawnattr_t *attr);
FAR char * const *envp, FAR const struct symtab_s *exports,
int nexports, FAR const posix_spawnattr_t *attr);
/****************************************************************************
* Name: binfmt_exit

View File

@ -933,6 +933,7 @@ FAR struct streamlist *nxsched_get_streams(void);
* argv - A pointer to an array of input parameters. The array
* should be terminated with a NULL argv[] value. If no
* parameters are required, argv may be NULL.
* envp - A pointer to the program's environment, envp may be NULL
*
* Returned Value:
* OK on success; negative error value on failure appropriately. (See
@ -945,7 +946,7 @@ FAR struct streamlist *nxsched_get_streams(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 argv[], FAR char * const envp[]);
/****************************************************************************
* Name: nxtask_uninit

View File

@ -138,7 +138,11 @@ void arc4random_buf(FAR void *bytes, size_t nbytes);
/* Environment variable support */
#ifdef CONFIG_DISABLE_ENVIRON
# define get_environ_ptr() NULL
#else
FAR char **get_environ_ptr(void);
#endif
FAR char *getenv(FAR const char *name);
int putenv(FAR const char *string);
int clearenv(void);

View File

@ -50,9 +50,9 @@
* private, exact duplicate of the parent task's environment.
*
* Input Parameters:
* group - The child task group to receive the newly allocated copy of the
* parent task groups environment structure.
*
* group - The child task group to receive the newly allocated copy of
* the parent task groups environment structure.
* envcp - Pointer to the environment strings to copy.
* Returned Value:
* zero on success
*
@ -61,14 +61,14 @@
*
****************************************************************************/
int env_dup(FAR struct task_group_s *group)
int env_dup(FAR struct task_group_s *group, FAR char * const *envcp)
{
FAR struct tcb_s *ptcb = this_task();
FAR char **envp = NULL;
size_t envc = 0;
int ret = OK;
DEBUGASSERT(group != NULL && ptcb != NULL && ptcb->group != NULL);
DEBUGASSERT(group != NULL);
/* Pre-emption must be disabled throughout the following because the
* environment may be shared.
@ -76,13 +76,13 @@ int env_dup(FAR struct task_group_s *group)
sched_lock();
/* Does the parent task have an environment? */
/* Is there an environment ? */
if (ptcb->group != NULL && ptcb->group->tg_envp != NULL)
if (envcp || (envcp = (FAR char * const *)ptcb->group->tg_envp))
{
/* Yes.. The parent task has an environment allocation. */
/* Count the strings */
while (ptcb->group->tg_envp[envc] != NULL)
while (envcp[envc] != NULL)
{
envc++;
}
@ -113,8 +113,7 @@ int env_dup(FAR struct task_group_s *group)
while (envc-- > 0)
{
envp[envc] = group_malloc(group,
strlen(ptcb->group->tg_envp[envc]) + 1);
envp[envc] = group_malloc(group, strlen(envcp[envc]) + 1);
if (envp[envc] == NULL)
{
while (envp[++envc] != NULL)
@ -127,7 +126,7 @@ int env_dup(FAR struct task_group_s *group)
break;
}
strcpy(envp[envc], ptcb->group->tg_envp[envc]);
strcpy(envp[envc], envcp[envc]);
}
}
}

View File

@ -33,8 +33,8 @@
****************************************************************************/
#ifdef CONFIG_DISABLE_ENVIRON
# define env_dup(group) (0)
# define env_release(group) (0)
# define env_dup(group, envp) (0)
# define env_release(group) (0)
#else
/****************************************************************************
@ -65,6 +65,7 @@ extern "C"
* Input Parameters:
* group - The child task group to receive the newly allocated copy of the
* parent task groups environment structure.
* envp - Pointer to the environment strings.
*
* Returned Value:
* zero on success
@ -74,7 +75,7 @@ extern "C"
*
****************************************************************************/
int env_dup(FAR struct task_group_s *group);
int env_dup(FAR struct task_group_s *group, FAR char * const *envp);
/****************************************************************************
* Name: env_release

View File

@ -37,7 +37,6 @@
#include <nuttx/sched.h>
#include <nuttx/tls.h>
#include "environ/environ.h"
#include "sched/sched.h"
#include "group/group.h"
@ -201,15 +200,6 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype)
group_inherit_identity(group);
/* Duplicate the parent tasks environment */
ret = env_dup(group);
if (ret < 0)
{
tcb->cmn.group = NULL;
goto errout_with_taskinfo;
}
/* Initial user space semaphore */
nxsem_init(&group->tg_info->ta_sem, 0, 1);
@ -243,8 +233,6 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype)
return OK;
errout_with_taskinfo:
group_free(group, group->tg_info);
errout_with_member:
#ifdef HAVE_GROUP_MEMBERS
kmm_free(group->tg_members);

View File

@ -292,7 +292,7 @@ static inline void nx_start_application(void)
#ifndef CONFIG_ARCH_ADDRENV
attr.stacksize = CONFIG_INIT_STACKSIZE;
#endif
ret = exec_spawn(CONFIG_INIT_FILEPATH, argv,
ret = exec_spawn(CONFIG_INIT_FILEPATH, argv, NULL,
CONFIG_INIT_SYMTAB, CONFIG_INIT_NEXPORTS, &attr);
DEBUGASSERT(ret >= 0);
#endif

View File

@ -49,6 +49,7 @@ struct spawn_parms_s
FAR const posix_spawn_file_actions_t *file_actions;
FAR const posix_spawnattr_t *attr;
FAR char * const *argv;
FAR char * const *envp;
/* Parameters that differ for posix_spawn[p] and task_spawn */

View File

@ -91,7 +91,8 @@ static int nxthread_create(FAR const char *name, uint8_t ttype,
/* Initialize the task */
ret = nxtask_init(tcb, name, priority, stack_ptr, stack_size, entry, argv);
ret = nxtask_init(tcb, name, priority, stack_ptr, stack_size, entry, argv,
NULL);
if (ret < OK)
{
kmm_free(tcb);

View File

@ -36,6 +36,7 @@
#include <nuttx/tls.h>
#include "sched/sched.h"
#include "environ/environ.h"
#include "group/group.h"
#include "task/task.h"
@ -83,7 +84,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[])
main_t entry, FAR char * const argv[],
FAR char * const envp[])
{
uint8_t ttype = tcb->cmn.flags & TCB_FLAG_TTYPE_MASK;
FAR struct tls_info_s *info;
@ -103,6 +105,14 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char *name, int priority,
return ret;
}
/* Duplicate the parent tasks environment */
ret = env_dup(tcb->cmn.group, envp);
if (ret < 0)
{
goto errout_with_group;
}
/* Associate file descriptors with the new task */
ret = group_setuptaskfiles(tcb);

View File

@ -85,7 +85,8 @@
static int nxposix_spawn_exec(FAR pid_t *pidp, FAR const char *path,
FAR const posix_spawnattr_t *attr,
FAR char * const argv[])
FAR char * const argv[],
FAR char * const envp[])
{
FAR const struct symtab_s *symtab;
int nsymbols;
@ -107,7 +108,7 @@ static int nxposix_spawn_exec(FAR pid_t *pidp, FAR const char *path,
/* Start the task */
pid = exec_spawn(path, (FAR char * const *)argv, symtab, nsymbols, attr);
pid = exec_spawn(path, argv, envp, symtab, nsymbols, attr);
if (pid < 0)
{
ret = -pid;
@ -184,7 +185,8 @@ static int nxposix_spawn_proxy(int argc, FAR char *argv[])
/* Start the task */
ret = nxposix_spawn_exec(g_spawn_parms.pid, g_spawn_parms.u.posix.path,
g_spawn_parms.attr, g_spawn_parms.argv);
g_spawn_parms.attr, g_spawn_parms.argv,
g_spawn_parms.envp);
#ifdef CONFIG_SCHED_HAVE_PARENT
if (ret == OK)
@ -335,7 +337,7 @@ int posix_spawn(FAR pid_t *pid, FAR const char *path,
if ((file_actions == NULL || *file_actions == NULL) &&
(attr == NULL || (attr->flags & POSIX_SPAWN_SETSIGMASK) == 0))
{
return nxposix_spawn_exec(pid, path, attr, argv);
return nxposix_spawn_exec(pid, path, attr, argv, envp);
}
/* Otherwise, we will have to go through an intermediary/proxy task in
@ -365,6 +367,7 @@ int posix_spawn(FAR pid_t *pid, FAR const char *path,
g_spawn_parms.file_actions = file_actions ? *file_actions : NULL;
g_spawn_parms.attr = attr;
g_spawn_parms.argv = argv;
g_spawn_parms.envp = envp;
g_spawn_parms.u.posix.path = path;
/* Get the priority of this (parent) task */

View File

@ -365,6 +365,7 @@ int task_spawn(FAR const char *name, main_t entry,
g_spawn_parms.file_actions = file_actions ? *file_actions : NULL;
g_spawn_parms.attr = attr;
g_spawn_parms.argv = argv;
g_spawn_parms.envp = envp;
g_spawn_parms.u.task.name = name;
g_spawn_parms.u.task.entry = entry;

View File

@ -37,6 +37,7 @@
#include <nuttx/tls.h>
#include "sched/sched.h"
#include "environ/environ.h"
#include "group/group.h"
#include "task/task.h"
@ -151,6 +152,14 @@ FAR struct task_tcb_s *nxtask_setup_vfork(start_t retaddr)
goto errout_with_tcb;
}
/* Duplicate the parent tasks environment */
ret = env_dup(child->cmn.group, get_environ_ptr());
if (ret < 0)
{
goto errout_with_tcb;
}
/* Associate file descriptors with the new task */
ret = group_setuptaskfiles(child);