Setuid sandbox API versioning

content/zygote/zygote_main_linux.cc

Index: content/zygote/zygote_main_linux.cc
diff --git a/content/zygote/zygote_main_linux.cc b/content/zygote/zygote_main_linux.cc
index 5673f091d0b987866bf446753e1f3af1160ca456..abf95a32a48e823c9284ddda8b9eb2459611e978 100644
--- a/content/zygote/zygote_main_linux.cc
+++ b/content/zygote/zygote_main_linux.cc
@@ -529,14 +529,38 @@ static bool EnterSandbox(bool* using_suid_sandbox, bool* has_started_new_init) {
 
   const char* const sandbox_fd_string = getenv(kSUIDSandboxVar);
   if (sandbox_fd_string) {
+    char* endptr;
     // Use the SUID sandbox.  This still allows the seccomp sandbox to
     // be enabled by the process later.
     *using_suid_sandbox = true;
 
-    char* endptr;
+    // Check if the SUID sandbox provides the correct API version.

[Brad Chen, 2012/06/02 15:57:03]

Are you adding these checks only for sandbox use in the bots or is it needed in
Chrome as well? I assume it's needed everywhere but if it's only useful in the
bots then it might make sense to exclude the checking code in official builds.

[jln (very slow on Chromium), 2012/06/04 19:26:23]

Ohh yes, both. Well, in official builds it's unlikely to happen (because both
the sandbox and Chrome will be built at the same time).

But if anything, it should be more strict on Release builds, shouldn't it?

+    const char* const sandbox_api_string =
+                        getenv(base::kSandboxEnvironmentApiProvides);
+    // Assume API version 0 if no environment was found
+    long sandbox_api_num = 0;
+    if (sandbox_api_string) {
+      errno = 0;
+      sandbox_api_num = strtol(sandbox_api_string, &endptr, 10);
+      if (errno || *endptr) {
+        return false;
+      }
+    }
+
+    if (sandbox_api_num != base::kSUIDSandboxApiNumber) {
+      LOG(WARNING) << "You are using a wrong version of the setuid binary!\n"
+       "Please read "
+       "https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment."

[Brad Chen, 2012/06/02 15:57:03]

Should this doc be updated to document the version dependency?

[jln (very slow on Chromium), 2012/06/04 19:26:23]

Yes, good point. I'll do it when this lands.

+       "\n\n";
+    }
+
+    // Get the file descriptor to signal the chroot helper.
+    errno = 0;
     const long fd_long = strtol(sandbox_fd_string, &endptr, 10);
-    if (!*sandbox_fd_string || *endptr || fd_long < 0 || fd_long > INT_MAX)
+    if (errno || !*sandbox_fd_string || *endptr || fd_long < 0 ||
+        fd_long > INT_MAX) {
       return false;
+    }
     const int fd = fd_long;
 
     static const char kMsgChrootMe = 'C';