sched/addrenv, binfmt: Always allocate address environment from heap

Instead of using a volatile storage for the address environment in the
binfmt / loadinfo structures, always allocate the address environment
from kheap.

This serves two purposes:
- If the task creation fails, any kernel thread that depends on the
  address environment created during task creation will not lose their
  mappings (because they hold a reference to it)
- The current address environment variable (g_addrenv) will NEVER contain
  a stale / incorrect value
- Releasing the address environment is simplified as any pointer given
  to addrenv_drop() can be assumed to be heap memory
- Makes the kludge function addrenv_clear_current irrelevant, as the
  system will NEVER have invalid mappings any more

binfmt/binfmt_execmodule.c

@@ -117,7 +117,7 @@ int exec_module(FAR struct binary_s *binp,
 {
   FAR struct task_tcb_s *tcb;
 #if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
-  FAR struct arch_addrenv_s *addrenv = &binp->addrenv.addrenv;
+  FAR struct arch_addrenv_s *addrenv = &binp->addrenv->addrenv;
   FAR void *vheap;
 #endif
   FAR void *stackaddr = NULL;
@@ -165,7 +165,7 @@ int exec_module(FAR struct binary_s *binp,
 #if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
   /* Instantiate the address environment containing the user heap */
 
-  ret = addrenv_select(&binp->addrenv);
+  ret = addrenv_select(binp->addrenv);
   if (ret < 0)
     {
       berr("ERROR: addrenv_select() failed: %d\n", ret);
@@ -242,7 +242,7 @@ int exec_module(FAR struct binary_s *binp,
 #ifdef CONFIG_ARCH_ADDRENV
   /* Attach the address environment to the new task */
 
-  ret = addrenv_attach((FAR struct tcb_s *)tcb, &binp->addrenv);
+  ret = addrenv_attach((FAR struct tcb_s *)tcb, binp->addrenv);
   if (ret < 0)
     {
       berr("ERROR: addrenv_attach() failed: %d\n", ret);

binfmt/elf.c

@@ -271,11 +271,7 @@ static int elf_loadbinary(FAR struct binary_s *binp,
    * needed when the module is executed.
    */
 
-  up_addrenv_clone(&loadinfo.addrenv.addrenv, &binp->addrenv.addrenv);
+  binp->addrenv = loadinfo.addrenv;
-
-  /* Take a reference to the address environment, so it won't get freed */
-
-  addrenv_take(&binp->addrenv);
 
 #else
   binp->alloc[0]  = (FAR void *)loadinfo.textalloc;

binfmt/libelf/libelf_addrenv.c

@@ -84,24 +84,30 @@ int elf_addrenv_alloc(FAR struct elf_loadinfo_s *loadinfo, size_t textsize,
                       size_t datasize, size_t heapsize)
 {
 #ifdef CONFIG_ARCH_ADDRENV
-  FAR struct arch_addrenv_s *addrenv = &loadinfo->addrenv.addrenv;
+  FAR struct arch_addrenv_s *addrenv;
   FAR void *vtext;
   FAR void *vdata;
   int ret;
 
   /* Create an address environment for the new ELF task */
 
+  loadinfo->addrenv = addrenv_allocate();
+  if (!loadinfo->addrenv)
+    {
+      return -ENOMEM;
+    }
+
+  /* Start creating the address environment sections */
+
+  addrenv = &loadinfo->addrenv->addrenv;
+
   ret = up_addrenv_create(textsize, datasize, heapsize, addrenv);
   if (ret < 0)
     {
       berr("ERROR: up_addrenv_create failed: %d\n", ret);
-      return ret;
+      goto errout_with_addrenv;
     }
 
-  /* Take a reference to the address environment, so it won't get freed */
-
-  addrenv_take(&loadinfo->addrenv);
-
   /* Get the virtual address associated with the start of the address
    * environment.  This is the base address that we will need to use to
    * access the ELF image (but only if the address environment has been
@@ -112,19 +118,24 @@ int elf_addrenv_alloc(FAR struct elf_loadinfo_s *loadinfo, size_t textsize,
   if (ret < 0)
     {
       berr("ERROR: up_addrenv_vtext failed: %d\n", ret);
-      return ret;
+      goto errout_with_addrenv;
     }
 
   ret = up_addrenv_vdata(addrenv, textsize, &vdata);
   if (ret < 0)
     {
       berr("ERROR: up_addrenv_vdata failed: %d\n", ret);
-      return ret;
+      goto errout_with_addrenv;
     }
 
   loadinfo->textalloc = (uintptr_t)vtext;
   loadinfo->dataalloc = (uintptr_t)vdata;
+
   return OK;
+
+errout_with_addrenv:
+  addrenv_drop(loadinfo->addrenv, false);
+  return ret;
 #else
   /* Allocate memory to hold the ELF image */
 
@@ -177,7 +188,7 @@ int elf_addrenv_select(FAR struct elf_loadinfo_s *loadinfo)
 
   /* Instantiate the new address environment */
 
-  ret = addrenv_select(&loadinfo->addrenv);
+  ret = addrenv_select(loadinfo->addrenv);
   if (ret < 0)
     {
       berr("ERROR: addrenv_select failed: %d\n", ret);
@@ -186,7 +197,7 @@ int elf_addrenv_select(FAR struct elf_loadinfo_s *loadinfo)
 
   /* Allow write access to .text */
 
-  ret = up_addrenv_mprot(&loadinfo->addrenv.addrenv, loadinfo->textalloc,
+  ret = up_addrenv_mprot(&loadinfo->addrenv->addrenv, loadinfo->textalloc,
                          loadinfo->textsize, ELF_TEXT_WRE);
   if (ret < 0)
     {
@@ -219,7 +230,7 @@ int elf_addrenv_restore(FAR struct elf_loadinfo_s *loadinfo)
 
   /* Remove write access to .text */
 
-  ret = up_addrenv_mprot(&loadinfo->addrenv.addrenv, loadinfo->textalloc,
+  ret = up_addrenv_mprot(&loadinfo->addrenv->addrenv, loadinfo->textalloc,
                          loadinfo->textsize, ELF_TEXT_WRD);
   if (ret < 0)
     {
@@ -261,15 +272,10 @@ int elf_addrenv_restore(FAR struct elf_loadinfo_s *loadinfo)
 void elf_addrenv_free(FAR struct elf_loadinfo_s *loadinfo)
 {
 #ifdef CONFIG_ARCH_ADDRENV
-  int ret;
 
   /* Free the address environment */
 
-  ret = up_addrenv_destroy(&loadinfo->addrenv.addrenv);
+  addrenv_drop(loadinfo->addrenv, false);
-  if (ret < 0)
-    {
-      berr("ERROR: up_addrenv_destroy failed: %d\n", ret);
-    }
 #else
 
   if (loadinfo->textalloc != 0)

include/nuttx/addrenv.h

@@ -297,35 +297,14 @@ struct addrenv_reserve_s
  *   Allocate an address environment for a new process.
  *
  * Input Parameters:
- *   tcb   - The tcb of the newly created task.
+ *   None
- *   ttype - The type of the task.
  *
  * Returned Value:
- *   This is a NuttX internal function so it follows the convention that
+ *   Pointer to the new address environment, or NULL if out of memory.
- *   0 (OK) is returned on success and a negated errno is returned on
- *   failure.
  *
  ****************************************************************************/
 
-int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype);
+FAR struct addrenv_s *addrenv_allocate(void);
-
-/****************************************************************************
- * Name: addrenv_free
- *
- * Description:
- *   Free an address environment for a process.
- *
- * Input Parameters:
- *   tcb - The tcb of the task.
- *
- * Returned Value:
- *   This is a NuttX internal function so it follows the convention that
- *   0 (OK) is returned on success and a negated errno is returned on
- *   failure.
- *
- ****************************************************************************/
-
-int addrenv_free(FAR struct tcb_s *tcb);
 
 /****************************************************************************
  * Name: addrenv_switch
@@ -364,8 +343,7 @@ int addrenv_switch(FAR struct tcb_s *tcb);
  *
  ****************************************************************************/
 
-int addrenv_attach(FAR struct tcb_s *tcb,
+int addrenv_attach(FAR struct tcb_s *tcb, FAR struct addrenv_s *addrenv);
-                   FAR const struct addrenv_s *addrenv);
 
 /****************************************************************************
  * Name: addrenv_join

include/nuttx/binfmt/binfmt.h

@@ -82,11 +82,11 @@ struct binary_s
 #ifdef CONFIG_ARCH_ADDRENV
   /* Address environment.
    *
-   * addrenv - This is the handle created by up_addrenv_create() that can be
+   * addrenv - This is the handle created by addrenv_allocate() that can be
    *   used to manage the tasks address space.
    */
 
-  addrenv_t addrenv;                   /* Address environment */
+  FAR addrenv_t *addrenv;              /* Address environment */
 #endif
 
   size_t mapsize;                      /* Size of the mapped address region (needed for munmap) */

include/nuttx/binfmt/elf.h

@@ -117,12 +117,12 @@ struct elf_loadinfo_s
 
   /* Address environment.
    *
-   * addrenv - This is the handle created by up_addrenv_create() that can be
+   * addrenv - This is the handle created by addrenv_allocate() that can be
    *   used to manage the tasks address space.
    */
 
 #ifdef CONFIG_ARCH_ADDRENV
-  addrenv_t          addrenv;    /* Address environment */
+  FAR addrenv_t     *addrenv;    /* Address environment */
 #endif
 
   uint16_t           symtabidx;  /* Symbol table section index */

sched/addrenv/addrenv.c

@@ -86,35 +86,6 @@ static void addrenv_destroy(FAR void *arg)
   kmm_free(addrenv);
 }
 
-/****************************************************************************
- * Name: addrenv_clear_current
- *
- * Description:
- *   Clear the current addrenv from g_addrenv, if it matches the input.
- *
- * Input Parameters:
- *   addrenv - Pointer to the addrenv to free.
- *
- * Returned Value:
- *   None.
- *
- ****************************************************************************/
-
-static void addrenv_clear_current(FAR const addrenv_t *addrenv)
-{
-  int i;
-
-  /* Mark no address environment */
-
-  for (i = 0; i < CONFIG_SMP_NCPUS; i++)
-    {
-      if (addrenv == g_addrenv[i])
-        {
-          g_addrenv[i] = NULL;
-        }
-    }
-}
-
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -237,62 +208,26 @@ int addrenv_switch(FAR struct tcb_s *tcb)
  *   Allocate an address environment for a new process.
  *
  * Input Parameters:
- *   tcb   - The tcb of the newly created task.
+ *   None
- *   ttype - The type of the task.
  *
  * Returned Value:
- *   This is a NuttX internal function so it follows the convention that
+ *   Pointer to the new address environment, or NULL if out of memory.
- *   0 (OK) is returned on success and a negated errno is returned on
- *   failure.
  *
  ****************************************************************************/
 
-int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype)
+FAR struct addrenv_s *addrenv_allocate(void)
 {
-  int ret = OK;
+  FAR struct addrenv_s *addrenv;
 
-  if ((ttype & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+  addrenv = (FAR struct addrenv_s *)kmm_zalloc(sizeof(struct addrenv_s));
+  if (addrenv)
     {
-      tcb->addrenv_own = NULL;
+      /* Take reference so this won't get freed */
-    }
+
-  else
+      addrenv->refs = 1;
-    {
-      tcb->addrenv_own = (FAR struct addrenv_s *)
-        kmm_zalloc(sizeof(struct addrenv_s));
-      if (tcb->addrenv_own == NULL)
-        {
-          ret = -ENOMEM;
-        }
     }
 
-  return ret;
+  return addrenv;
-}
-
-/****************************************************************************
- * Name: addrenv_free
- *
- * Description:
- *   Free an address environment for a process.
- *
- * Input Parameters:
- *   tcb - The tcb of the task.
- *
- * Returned Value:
- *   This is a NuttX internal function so it follows the convention that
- *   0 (OK) is returned on success and a negated errno is returned on
- *   failure.
- *
- ****************************************************************************/
-
-int addrenv_free(FAR struct tcb_s *tcb)
-{
-  if (tcb->addrenv_own != NULL)
-    {
-      kmm_free(tcb->addrenv_own);
-      tcb->addrenv_own = NULL;
-    }
-
-  return OK;
 }
 
 /****************************************************************************
@@ -313,24 +248,12 @@ int addrenv_free(FAR struct tcb_s *tcb)
  *
  ****************************************************************************/
 
-int addrenv_attach(FAR struct tcb_s *tcb,
+int addrenv_attach(FAR struct tcb_s *tcb, FAR struct addrenv_s *addrenv)
-                   FAR const struct addrenv_s *addrenv)
 {
-  int ret;
-
-  /* Clone the address environment for us */
-
-  ret = up_addrenv_clone(&addrenv->addrenv, &tcb->addrenv_own->addrenv);
-  if (ret < 0)
-    {
-      berr("ERROR: up_addrenv_clone failed: %d\n", ret);
-      return ret;
-    }
-
   /* Attach the address environment */
 
+  tcb->addrenv_own = addrenv;
   tcb->addrenv_curr = tcb->addrenv_own;
-  tcb->addrenv_own->refs = 1;
 
   return OK;
 }
@@ -452,14 +375,6 @@ int addrenv_restore(void)
 {
   FAR struct tcb_s *tcb = this_task();
   addrenv_give(tcb->addrenv_curr);
-
-  if (tcb->addrenv_own == NULL)
-    {
-      /* Kernel thread, clear g_addrenv, as it is not valid any more */
-
-      addrenv_clear_current(tcb->addrenv_curr);
-    }
-
   tcb->addrenv_curr = tcb->addrenv_own;
   return addrenv_switch(tcb);
 }

sched/task/task_init.c

@@ -97,12 +97,11 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char *name, int priority,
 #endif
 
 #ifdef CONFIG_ARCH_ADDRENV
-  /* Allocate address environment for the task */
+  /* Kernel threads do not own any address environment */
 
-  ret = addrenv_allocate(&tcb->cmn, tcb->cmn.flags);
+  if ((ttype & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
-  if (ret < 0)
     {
-      return ret;
+      tcb->cmn.addrenv_own = NULL;
     }
 #endif
 
@@ -111,7 +110,7 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char *name, int priority,
   ret = group_allocate(tcb, tcb->cmn.flags);
   if (ret < 0)
     {
-      goto errout_with_addrenv;
+      return ret;
     }
 
   /* Duplicate the parent tasks environment */
@@ -201,10 +200,6 @@ errout_with_group:
 
   group_leave(&tcb->cmn);
 
-errout_with_addrenv:
-#ifdef CONFIG_ARCH_ADDRENV
-  addrenv_free(&tcb->cmn);
-#endif
   return ret;
 }