From 6021ece539ff9dd62ee1fe35af24800afdec9f11 Mon Sep 17 00:00:00 2001
From: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Date: Sun, 7 Dec 2025 18:52:47 +0100
Subject: [PATCH] Calculate total_args_envs_size in redox_rt::proc::fexec_impl

Calculating it earlier is a micro-optimization that doesn't seem like it
matters at all compared to all other costs of spawning a process. But at
the same time it makes things more fragile. And in fact bootstrap
actually passed the wrong value. It passed the
total_args_envs_auxvpointee_size that it computed rather than the
total_args_envs_size. While over-approximation doesn't cause UB, it
unnecessarily increases the amount of memory used after exec.

In addition pass &[&[u8]] rather than iterators for args and envs to
enable precomputing the total arg and env size. This also prevents you
from forgetting to pass a reversed iterator.
---
 redox-rt/src/proc.rs       | 26 ++++++++++----------------
 src/platform/redox/exec.rs | 29 ++++++-----------------------
 2 files changed, 16 insertions(+), 39 deletions(-)

diff --git a/redox-rt/src/proc.rs b/redox-rt/src/proc.rs
index e5a443a838..95f332cdb4 100644
--- a/redox-rt/src/proc.rs
+++ b/redox-rt/src/proc.rs
@@ -69,24 +69,17 @@ pub struct ExtraInfo<'a> {
     pub proc_fd: usize,
 }
 
-pub fn fexec_impl<A, E>(
+pub fn fexec_impl(
     image_file: FdGuardUpper,
     thread_fd: &FdGuardUpper,
     proc_fd: &FdGuardUpper,
     memory_scheme_fd: &FdGuardUpper,
     path: &[u8],
-    args: A,
-    envs: E,
-    total_args_envs_size: usize,
+    args: &[&[u8]],
+    envs: &[&[u8]],
     extrainfo: &ExtraInfo,
     mut interp_override: Option<InterpOverride>,
-) -> Result<FexecResult>
-where
-    A: IntoIterator,
-    E: IntoIterator,
-    A::Item: AsRef<[u8]>,
-    E::Item: AsRef<[u8]>,
-{
+) -> Result<FexecResult> {
     // Here, we do the minimum part of loading an application, which is what the kernel used to do.
     // We load the executable into memory (albeit at different offsets in this executable), fix
     // some misalignments, and then switch address space.
@@ -315,7 +308,8 @@ where
     )?;
     push(AT_PHENT)?;
 
-    let total_args_envs_auxvpointee_size = total_args_envs_size
+    let total_args_envs_auxvpointee_size = args.iter().map(|arg| arg.len()).sum::<usize>()
+        + envs.iter().map(|env| env.len()).sum::<usize>()
         + extrainfo.cwd.map_or(0, |s| s.len() + 1)
         + extrainfo.default_scheme.map_or(0, |s| s.len() + 1);
     let args_envs_size_aligned = total_args_envs_auxvpointee_size.next_multiple_of(PAGE_SIZE);
@@ -394,14 +388,14 @@ where
 
         push(0)?;
 
-        for env in envs {
-            push(append(env.as_ref())?)?;
+        for env in envs.iter().rev() {
+            push(append(env)?)?;
         }
 
         push(0)?;
 
-        for arg in args {
-            push(append(arg.as_ref())?)?;
+        for arg in args.iter().rev() {
+            push(append(arg)?)?;
             argc += 1;
         }
     }
diff --git a/src/platform/redox/exec.rs b/src/platform/redox/exec.rs
index 15190ca51a..0ad5b5d6b8 100644
--- a/src/platform/redox/exec.rs
+++ b/src/platform/redox/exec.rs
@@ -26,7 +26,6 @@ fn fexec_impl(
     path: &[u8],
     args: &[&[u8]],
     envs: &[&[u8]],
-    total_args_envs_size: usize,
     extrainfo: &ExtraInfo,
     interp_override: Option<InterpOverride>,
 ) -> Result<Infallible> {
@@ -38,9 +37,8 @@ fn fexec_impl(
         redox_rt::current_proc_fd(),
         &memory,
         path,
-        args.iter().rev(),
-        envs.iter().rev(),
-        total_args_envs_size,
+        args,
+        envs,
         extrainfo,
         interp_override,
     )? {
@@ -59,11 +57,7 @@ fn fexec_impl(
 
             return execve(
                 Executable::AtPath(path_cstr),
-                ArgEnv::Parsed {
-                    total_args_envs_size,
-                    args,
-                    envs,
-                },
+                ArgEnv::Parsed { args, envs },
                 Some(new_interp_override),
             );
         }
@@ -83,7 +77,6 @@ pub enum ArgEnv<'a> {
     Parsed {
         args: &'a [&'a [u8]],
         envs: &'a [&'a [u8]],
-        total_args_envs_size: usize,
     },
 }
 
@@ -179,17 +172,14 @@ pub fn execve(
             .map_err(|_| Error::new(EIO))?;
     }
 
-    let (total_args_envs_size, args, envs): (usize, Vec<_>, Vec<_>) = match arg_env {
+    let (args, envs): (Vec<_>, Vec<_>) = match arg_env {
         ArgEnv::C { mut argv, mut envp } => unsafe {
-            let mut args_envs_size_without_nul = 0;
-
             // Arguments
             while !argv.read().is_null() {
                 let arg = argv.read();
 
                 let len = strlen(arg);
                 args.push(core::slice::from_raw_parts(arg as *const u8, len));
-                args_envs_size_without_nul += len;
                 argv = argv.add(1);
             }
 
@@ -205,23 +195,17 @@ pub fn execve(
 
                 let len = strlen(env);
                 envs.push(core::slice::from_raw_parts(env as *const u8, len));
-                args_envs_size_without_nul += len;
                 envp = envp.add(1);
             }
-            (
-                args_envs_size_without_nul + args.len() + envs.len(),
-                args,
-                envs,
-            )
+            (args, envs)
         },
         ArgEnv::Parsed {
             args: new_args,
             envs,
-            total_args_envs_size,
         } => {
             let prev_size: usize = args.iter().map(|a| a.len()).sum();
             args.extend(new_args);
-            (total_args_envs_size + prev_size, args, Vec::from(envs))
+            (args, Vec::from(envs))
         }
     };
 
@@ -276,7 +260,6 @@ pub fn execve(
         arg0,
         &args,
         &envs,
-        total_args_envs_size,
         &extrainfo,
         interp_override,
     )