Skip to content

Commit

Permalink
env_dup: Fix copying of env between address environments
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pussuw committed Apr 19, 2022
1 parent 500d3b1 commit 419ca36
Show file tree
Hide file tree
Showing 18 changed files with 386 additions and 89 deletions.
44 changes: 44 additions & 0 deletions binfmt/binfmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) (envp)
#endif

/****************************************************************************
* Name: builtin_initialize
*
Expand Down
10 changes: 6 additions & 4 deletions binfmt/binfmt_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
26 changes: 22 additions & 4 deletions binfmt/binfmt_execmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -150,14 +151,26 @@ 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 */

ret = up_addrenv_select(&binp->addrenv, &oldenv);
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);
Expand All @@ -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)
Expand All @@ -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 */
Expand Down Expand Up @@ -281,7 +297,9 @@ int exec_module(FAR const struct binary_s *binp,
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:
Expand Down
9 changes: 6 additions & 3 deletions include/nuttx/binfmt/binfmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion include/nuttx/sched.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sched/environ/Make.defs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ ifneq ($(CONFIG_DISABLE_ENVIRON),y)

CSRCS += env_getenvironptr.c env_dup.c env_release.c env_findvar.c
CSRCS += env_removevar.c env_clearenv.c env_getenv.c env_putenv.c
CSRCS += env_setenv.c env_unsetenv.c env_foreach.c
CSRCS += env_setenv.c env_unsetenv.c env_foreach.c env_set.c env_get.c

# Include environ build support

Expand Down
56 changes: 1 addition & 55 deletions sched/environ/env_dup.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@
int env_dup(FAR struct task_group_s *group)
{
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);
Expand All @@ -82,59 +80,7 @@ int env_dup(FAR struct task_group_s *group)
{
/* Yes.. The parent task has an environment allocation. */

while (ptcb->group->tg_envp[envc] != NULL)
{
envc++;
}

/* A special case is that the parent has an "empty" environment
* allocation, i.e., there is an allocation in place but it
* contains no variable definitions and, hence, envc == 0.
*/

if (envc > 0)
{
/* There is an environment, duplicate it */

envp = group_malloc(group, sizeof(*envp) * (envc + 1));
if (envp == NULL)
{
/* The parent's environment can not be inherited due to a
* failure in the allocation of the child environment.
*/

ret = -ENOMEM;
}
else
{
envp[envc] = NULL;

/* Duplicate the parent environment. */

while (envc-- > 0)
{
envp[envc] = group_malloc(group,
strlen(ptcb->group->tg_envp[envc]) + 1);
if (envp[envc] == NULL)
{
while (envp[++envc] != NULL)
{
group_free(group, envp[envc]);
}

group_free(group, envp);
ret = -ENOMEM;
break;
}

strcpy(envp[envc], ptcb->group->tg_envp[envc]);
}
}
}

/* Save the child environment allocation. */

group->tg_envp = envp;
env_set(group, (FAR char * const *)ptcb->group->tg_envp);
}

sched_unlock();
Expand Down
67 changes: 67 additions & 0 deletions sched/environ/env_get.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/****************************************************************************
* sched/environ/env_get.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 <nuttx/config.h>

#ifndef CONFIG_DISABLE_ENVIRON

#include <sched.h>
#include <assert.h>

#include "environ/environ.h"
#include "sched/sched.h"
#include "group/group.h"

/****************************************************************************
* Public Functions
****************************************************************************/

/****************************************************************************
* Name: env_get
*
* Description:
* Reference to the current group environment variables.
*
* Input Parameters:
* None.
*
* Returned Value:
* Reference to the group environment variables, or NULL if group or env is
* not set.
*
****************************************************************************/

FAR char * const *env_get(void)
{
FAR struct tcb_s *rtcb = this_task();

if (rtcb->group)
{
return (FAR char * const *)rtcb->group->tg_envp;
}

return NULL;
}

#endif /* CONFIG_DISABLE_ENVIRON */
Loading

0 comments on commit 419ca36

Please sign in to comment.