From 0cf7fc4e115c2dcc40901c44b238ab36b5d966fc Mon Sep 17 00:00:00 2001
From: Zach Koopmans <zkoopmans@google.com>
Date: Tue, 5 Feb 2019 10:00:22 -0800
Subject: [PATCH] Change /proc/PID/cmdline to read environment vector.

- Change proc to return envp on overwrite of argv with limitations from
upstream.
- Add unit tests
- Change layout of argv/envp on the stack so that end of argv is contiguous with
beginning of envp.

PiperOrigin-RevId: 232506107
Change-Id: I993880499ab2c1220f6dc456a922235c49304dec
---
 pkg/sentry/arch/stack.go        | 30 +++++++++++-------
 pkg/sentry/fs/proc/exec_args.go | 55 +++++++++++++++++++++++++++++----
 test/util/multiprocess_util.h   |  1 +
 3 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/pkg/sentry/arch/stack.go b/pkg/sentry/arch/stack.go
index 716a3574d..f2cfb0426 100644
--- a/pkg/sentry/arch/stack.go
+++ b/pkg/sentry/arch/stack.go
@@ -170,6 +170,24 @@ func (s *Stack) Load(args []string, env []string, aux Auxv) (StackLayout, error)
 	// Make sure we start with a 16-byte alignment.
 	s.Align(16)
 
+	// Push the environment vector so the end of the argument vector is adjacent to
+	// the beginning of the environment vector.
+	// While the System V abi for x86_64 does not specify an ordering to the
+	// Information Block (the block holding the arg, env, and aux vectors),
+	// support features like setproctitle(3) naturally expect these segments
+	// to be in this order. See: https://www.uclibc.org/docs/psABI-x86_64.pdf
+	// page 29.
+	l.EnvvEnd = s.Bottom
+	envAddrs := make([]usermem.Addr, len(env))
+	for i := len(env) - 1; i >= 0; i-- {
+		addr, err := s.Push(env[i])
+		if err != nil {
+			return StackLayout{}, err
+		}
+		envAddrs[i] = addr
+	}
+	l.EnvvStart = s.Bottom
+
 	// Push our strings.
 	l.ArgvEnd = s.Bottom
 	argAddrs := make([]usermem.Addr, len(args))
@@ -182,18 +200,6 @@ func (s *Stack) Load(args []string, env []string, aux Auxv) (StackLayout, error)
 	}
 	l.ArgvStart = s.Bottom
 
-	// Push our environment.
-	l.EnvvEnd = s.Bottom
-	envAddrs := make([]usermem.Addr, len(env))
-	for i := len(env) - 1; i >= 0; i-- {
-		addr, err := s.Push(env[i])
-		if err != nil {
-			return StackLayout{}, err
-		}
-		envAddrs[i] = addr
-	}
-	l.EnvvStart = s.Bottom
-
 	// We need to align the arguments appropriately.
 	//
 	// We must finish on a 16-byte alignment, but we'll play it
diff --git a/pkg/sentry/fs/proc/exec_args.go b/pkg/sentry/fs/proc/exec_args.go
index a716eb5f5..9daad5d2b 100644
--- a/pkg/sentry/fs/proc/exec_args.go
+++ b/pkg/sentry/fs/proc/exec_args.go
@@ -15,6 +15,7 @@
 package proc
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 
@@ -139,20 +140,62 @@ func (f *execArgFile) Read(ctx context.Context, _ *fs.File, dst usermem.IOSequen
 	// N.B. Technically this should be usermem.IOOpts.IgnorePermissions = true
 	// until Linux 4.9 (272ddc8b3735 "proc: don't use FOLL_FORCE for reading
 	// cmdline and environment").
-	copyN, copyErr := m.CopyIn(ctx, start, buf, usermem.IOOpts{})
+	copyN, err := m.CopyIn(ctx, start, buf, usermem.IOOpts{})
 	if copyN == 0 {
 		// Nothing to copy.
-		return 0, copyErr
+		return 0, err
 	}
 	buf = buf[:copyN]
 
-	// TODO: On Linux, if the NUL byte at the end of the
-	// argument vector has been overwritten, it continues reading the
-	// environment vector as part of the argument vector.
+	// On Linux, if the NUL byte at the end of the argument vector has been
+	// overwritten, it continues reading the environment vector as part of
+	// the argument vector.
+
+	if f.arg == cmdlineExecArg && buf[copyN-1] != 0 {
+		// Linux will limit the return up to and including the first null character in argv
+
+		copyN = bytes.IndexByte(buf, 0)
+		if copyN == -1 {
+			copyN = len(buf)
+		}
+		// If we found a NUL character in argv, return upto and including that character.
+		if copyN < len(buf) {
+			buf = buf[:copyN]
+		} else { // Otherwise return into envp.
+			lengthEnvv := int(m.EnvvEnd() - m.EnvvStart())
+
+			// Upstream limits the returned amount to one page of slop.
+			// https://elixir.bootlin.com/linux/v4.20/source/fs/proc/base.c#L208
+			// we'll return one page total between argv and envp because of the
+			// above page restrictions.
+			if lengthEnvv > usermem.PageSize-len(buf) {
+				lengthEnvv = usermem.PageSize - len(buf)
+			}
+			// Make a new buffer to fit the whole thing
+			tmp := make([]byte, length+lengthEnvv)
+			copyNE, err := m.CopyIn(ctx, m.EnvvStart(), tmp[copyN:], usermem.IOOpts{})
+			if err != nil {
+				return 0, err
+			}
+
+			// Linux will return envp up to and including the first NUL character, so find it.
+			for i, c := range tmp[copyN:] {
+				if c == 0 {
+					copyNE = i
+					break
+				}
+			}
+
+			copy(tmp, buf)
+			buf = tmp[:copyN+copyNE]
+
+		}
+
+	}
 
 	n, dstErr := dst.CopyOut(ctx, buf)
 	if dstErr != nil {
 		return int64(n), dstErr
 	}
-	return int64(n), copyErr
+	return int64(n), err
 }
diff --git a/test/util/multiprocess_util.h b/test/util/multiprocess_util.h
index c09d6167f..ba5f2601f 100644
--- a/test/util/multiprocess_util.h
+++ b/test/util/multiprocess_util.h
@@ -74,6 +74,7 @@ class ExecveArray {
   ExecveArray& operator=(ExecveArray&&) = delete;
 
   char* const* get() const { return ptrs_.data(); }
+  size_t get_size() { return str_.size(); }
 
  private:
   std::vector<char> str_;