1291553 - Assertion failure: sb.st_nlink >= 3, at security/sandbox/linux/common/SandboxInfo.cpp:73 when running under Ubuntu on Windows

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Gregory Szorc [:gps]]

I was able to compile mozilla-central under Ubuntu for Windows. For the most part "it just worked."

Unfortunately, I get an assertion on startup:

  Assertion failure: sb.st_nlink >= 3, at security/sandbox/linux/common/SandboxInfo.cpp:73

The asserting code in question:

static bool
IsSingleThreaded()
{
  // This detects the thread count indirectly.  /proc/<pid>/task has a
  // subdirectory for each thread in <pid>'s thread group, and the
  // link count on the "task" directory follows Unix expectations: the
  // link from its parent, the "." link from itself, and the ".." link
  // from each subdirectory; thus, 2+N links for N threads.
  struct stat sb;
  if (stat("/proc/self/task", &sb) < 0) {
    MOZ_DIAGNOSTIC_ASSERT(false, "Couldn't access /proc/self/task!");
    return false;
  }
  MOZ_DIAGNOSTIC_ASSERT(sb.st_nlink >= 3);
  return sb.st_nlink == 3;
}

I'm not a procfs expert, but I believe the alternate, better ways to count threads in a process are:

1) Find the "Threads: <int>" line in /proc/<proc>/status
2) Count directory entries in /proc/<proc>/task

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

That's transcribed from how Chromium does it: https://cs.chromium.org/search/?q=file:sandbox+function:IsSingleThreadedImpl&sq=package:chromium&type=cs

It has the advantage of needing only one system call, rather than needing to parse text (possibly a large amount of it) or directory entries, dealing with short reads, understanding whether the resulting race conditions mean it could read the process as singlethreaded when it's not, etc.

I'd rather just detect Ubuntu For Windows and disable sandboxing, because it can't work without very Linux-specific that I'm pretty sure the simulated kernel doesn't have (and probably doesn't even have plans to support).  Can this be done at build time, or do we need to probe for it at runtime so the same binaries can work on that and genuine Linux?

Comment 2

[Gian-Carlo Pascutto [:gcp]]

The IsSingleThreaded calls are all in relation to seccompf-bpf. Is the "Linux-specific" thing you're talking about seccomp-bpf? In that case maybe we can reverse the logic to not care about IsSingleThreaded if there's no seccomp-bpf support anyway.

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #2)
> The IsSingleThreaded calls are all in relation to seccompf-bpf. Is the
> "Linux-specific" thing you're talking about seccomp-bpf? In that case maybe
> we can reverse the logic to not care about IsSingleThreaded if there's no
> seccomp-bpf support anyway.

In our case it's not about seccomp-bpf — it's for unsharing namespaces.  These assertions aren't conditional on actually having namespace support because I wanted to catch regressions early… but that was before automation had support and that might be fixed now — TaskCluster Docker containers should all be running on a new-enough kernel, as I understand it.

(Background: Chromium was able to design for sandboxing enough to start seccomp-bpf while single-threaded, at least on not-Android (and then they wrote seccomp thread sync and upstreamed it to Linux when that wasn't enough), but in Gecko we're always multithreaded at that point and we have the signal broadcasting stuff from bug 969040.)

A question about Ubuntu On Windows: does /proc/<pid>/ns exist, and if so what does it contain?  If HasUserNamespaceSupport() will return false there — and if it returns true on actual-Linux automation — then this should be easy to fix up.

Comment 4

[Gregory Szorc [:gps]]

/proc/<pid>/ns does not exist. Here is what exists:

/proc/1/
/proc/1/attr
/proc/1/attr/current
/proc/1/auxv
/proc/1/comm
/proc/1/cmdline
/proc/1/cwd
/proc/1/exe
/proc/1/fd
/proc/1/fd/3
/proc/1/fd/0
/proc/1/fd/1
/proc/1/fd/2
/proc/1/fd/4
/proc/1/fd/5
/proc/1/limits
/proc/1/maps
/proc/1/mountinfo
/proc/1/mounts
/proc/1/mountstats
/proc/1/oom_adj
/proc/1/root
/proc/1/schedstat
/proc/1/smaps
/proc/1/stat
/proc/1/statm
/proc/1/status
/proc/1/net
/proc/1/net/if_inet6
/proc/1/net/netlink
/proc/1/net/tcp
/proc/1/net/tcp6
/proc/1/net/xt_qtaguid
/proc/1/net/xt_qtaguid/ctrl
/proc/1/net/xt_qtaguid/iface_stat_all
/proc/1/net/xt_qtaguid/iface_stat_fmt
/proc/1/net/xt_qtaguid/stats
/proc/1/net/udp
/proc/1/net/udp6
/proc/1/task
/proc/1/task/1
/proc/1/task/1/attr
/proc/1/task/1/attr/current
/proc/1/task/1/auxv
/proc/1/task/1/comm
/proc/1/task/1/cmdline
/proc/1/task/1/cwd
/proc/1/task/1/exe
/proc/1/task/1/fd
/proc/1/task/1/fd/3
/proc/1/task/1/fd/0
/proc/1/task/1/fd/1
/proc/1/task/1/fd/2
/proc/1/task/1/fd/4
/proc/1/task/1/fd/5
/proc/1/task/1/limits
/proc/1/task/1/maps
/proc/1/task/1/mountinfo
/proc/1/task/1/mounts
/proc/1/task/1/mountstats
/proc/1/task/1/oom_adj
/proc/1/task/1/root
/proc/1/task/1/schedstat
/proc/1/task/1/smaps
/proc/1/task/1/stat
/proc/1/task/1/statm
/proc/1/task/1/status

Comment 5

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

This code was removed in bug 1401062, so this should be fixed now.