930258 - Remove open() from seccomp whitelist on B2G

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

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

Known users of open() after sandbox startup:
* Font loading, apparently?
* MediaCache (bug 906996)
* SPS writing profile files (used on B2G, maybe elsewhere)
* about:memory writing to files (the only interface available on B2G)
* libgenlock on qcom B2G devices; opens /dev/genlock O_RDWR.

The Gecko-based callers should go through IPDL in a suitably high-level way.

Callers in B2G system libraries: we should be able to intercept these in libmozglue and proxy them through the parent (which will have to check the arguments *carefully*, because this bypasses uid-based permissions), and we may have to resort to that.

Other Linux platforms (desktop, Android) I don't know as much about.

A bunch of these don't have bugs yet....

Comment 1

[Stephanie Ouillon [:arroway] (needinfo me)]

I added some calls to open I recorded when starting the homescreen and using basic apps such as the camera.
There should be more to find by testing webRTC.

Comment 2

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

This comment might belong under one of the dependent bugs instead, but: Chromium's sandbox/linux/services/broker_process.cc could be useful here — it forks a process to do open() and access() calls on behalf of the sandboxed process.  Advantages:

* async signal safe
* probably faster than regular IPC
* the code is mostly already written
* it seems to not depend on the rest of the sandbox (so we can include it without making major changes)

Disadvantages:

* doesn't handle O_CREAT 
* doesn't handle any other syscalls (but stat (not lstat) can be replaced with open/fstat/close)
* the whitelist is for fixed paths, so no allowing /data/local/tmp/profile_* or similar

But at least some of the troublesome cases (e.g., MediaCache's unlinked temporary file) are only an issue in code we control and where we can use regular IPC.

Comment 3

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

(In reply to Jed Davis [:jld] from comment #2)
> Disadvantages:
> 
> * doesn't handle O_CREAT 
> * doesn't handle any other syscalls
> * the whitelist is for fixed paths

* Because it's fork-and-not-exec, it'll retain a copy of anything the real child COWs.  On B2G most of that would be shared with Nuwa, and on desktop it might be swappable, but the memshrink implications must be considered.

Comment 4

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

Attachment: Patch series for using the Chromium broker on B2G.

The patches I've been investigating with, cleaned up slightly (but still with a lot of FIXMEs and doing things the wrong way and breakage; apply at your own risk).

This is in `git format-patch` format, and contains multiple commits in one file; `git am` should know what to do with them.  (If anyone is interested who's using Hg for this, it should suffice to split at the "From " lines and run the patches through `git-patch-to-hg-patch` from the moz-git-tools project.)

Comment 5

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

Also, a thought about the “mprotect hack” approach: the path whitelist will probably be large regardless of how it's applied (see the font loading and library loading sub-bugs in particular), but we could do something like padding all the strings out to a power of two in contiguous virtual memory and, in the filter program, checking that the given address is in range and correctly aligned (to prevent opening a suffix of an allowed path).  This might be preferable to generating a large filter program with a long string of equality comparisons, as is done for the syscall number.

Comment 6

[Stephanie Ouillon [:arroway] (needinfo me)]

Attachment: 0001-Move-whitelist-to-SandboxFilter-and-add-more-pathnam.patch

I run Gaia UI tests to catch more libraries and pathnames to whitelist. I moved the list to SandboxFilter.cpp so it is with the syscalls whitelist.
One thing I had to modify but I don't really get why is the whitelisting of fstat64() with the ALLOW_SYSCALL macro, because it provoked seccomp violations.

Comment 7

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

(In reply to Stephanie Ouillon [:arroway] from comment #6)
> One thing I had to modify but I don't really get why is the whitelisting of
> fstat64() with the ALLOW_SYSCALL macro, because it provoked seccomp
> violations.

I don't completely understand this either, but after a lot of trial-and-error I've figured out what's causing it: there are system headers that do `#define fstat64 fstat` (and likewise for stat and lstat), which affects the SYSCALL_LARGEFILE macro but not using the SYSCALL macro directly.

Comment 8

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

I got the broker to build post-bug 1041886 without making large changes to the imported Chromium code, but:

                            |     megabytes     |
           NAME   PID  PPID CPU(s) NICE  USS  PSS  RSS SWAP VSIZE OOM_ADJ USER     
            b2g 13708     1   16.9    0 51.4 57.4 73.5  0.0 161.1       0 root     
         (Nuwa) 13712 13708    0.7    0  1.7  3.9 13.3  0.0  53.8     -16 root     
Built-in Keyboa 13846 13712    0.9   18  5.9  9.6 22.3  0.0  69.9      10 u0_a13846
      FS Broker 13882 13846    0.0   18  2.6  4.3  9.9  0.0  59.6       1 u0_a13846
     Homescreen 13893 13708    4.1   18  9.0 17.2 32.9  0.0  79.7       8 u0_a13893
      FS Broker 13924 13893    0.0    1  4.1  6.9  9.9  0.0  69.7       2 u0_a13893
(Preallocated a 14069 13712    0.5   18  3.5  6.9 18.7  0.0  59.6       1 u0_a14069
      FS Broker 14087 14069    0.0   18  2.2  4.1  9.9  0.0  59.6       1 u0_a14069

Note the USS column.  The broker has significant memory overhead, which I assume is due to its use of fork-and-not-exec (retaining old versions of pages COWed by the content process, even if the broker won't ever use them), so something will need to be done about that before it can be used on B2G.

Also something to keep in mind: if a broker process is OOM-killed, that will effectively kill its content process as well.

Comment 9

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

I've run into a new problem on Flame: apparently this version of the kgsl driver (Qualcomm graphics) can be used only from the process that opened it.  The relevant line of code seems to be https://github.com/mozilla-b2g/codeaurora_kernel_msm/blob/7731d63c809d/drivers/gpu/msm/kgsl_device.h#L657

My hope is that we can avoid this issue by creating the EGL context before starting the sandbox (which will also avoid its needing to open directories to find the libraries).

Comment 10

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

Reshaping this bug a little to be specifically about B2G.  Also, reflecting that I'm actively working on it.

In my opinion there's not much value in having a sandbox on desktop if it allows arbitrary open() — writing any file as the user is easy arbitrary code execution (from files like .bashrc), and even reading files is very far from harmless (private keys, cookies, other secret tokens).

Also, most of the “opens file X in content process” dependencies currently hanging off this bug are things where intercepting and brokering the open() will work, so they don't actually need to block; they can go under a tracker for minimizing the file broker whitelist (once such a thing exists).

Comment 11

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

Move process sandboxing bugs to the new Bugzilla component.

(Sorry for the bugspam; filter on 3c21328c-8cfb-4819-9d88-f6e965067350.)

Comment 12

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

Sorry for the bugspam; filter on 086f2ac3-ac15-4299-889b-009181af5029.

Comment 13

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

Attachment: Work In Progress (2015-07-14)

Back in January I wrote a file access broker which runs as a thread in the parent process in order to avoid the memory usage problems of a forked process, with the thread's effective uid set to the child process's uid on B2G.  This isn't quite ideal, because it's still a sandbox escape if the broker client can get arbitrary code excution on that thread, but not if it can only cause filesystem operations to be performed.  At the very least it's an easier way to experimentally get this running, and the implementation could be replaced with exec'ing a small executable without changing the interface much.

Now that the other prerequisites have landed, I've un-bit-rotted that code and redone the seccomp-bpf glue to use the Chromium policy compiler and gotten it to where it works on the emulator at least well enough to get to the homescreen.

This should be applied on top of bug 1181704, because it uses SANDBOX_LOG_ERROR in what's potentially async signal context to log error reponses from the broker.

Things this patch needs: a lot of comments; limiting it to a whitelist of devices that the broker policy is known to cover (at least initially); some cleanup of the seccomp-bpf glue in SandboxFilter; not calling access() on a thread with ruid!=euid; probably more that I'm not thinking of right now.

Also, this patch contains a fix for https://crbug.com/510193 applied to security/sandbox/chromium without the necessary record-keeping, but I think I'm going to drop that part and move the conditionals that provoke that bug out of the seccomp-bpf program and into the trap handlers.

Comment 14

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

Paul: What are your thoughts on using threads in the parent process to implement the broker?  See comment #13 about the security implications for B2G.  One alternative is that this could initially land using threads and then be converted to a standalone executable as a followup.

Also, I'd been thinking that this would be limited to a small set of devices (maybe just the emulators) to start with, and devices could be added as people test them and fix the policy as needed; does that sound reasonable?

Comment 15

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Jed Davis [:jld] {UTC-7} from comment #14)
> Paul: What are your thoughts on using threads in the parent process to
> implement the broker?  See comment #13 about the security implications for
> B2G.  One alternative is that this could initially land using threads and
> then be converted to a standalone executable as a followup.

Given the known weaknesses in the existing white list, anything to get this project moving I think would be worthwhile.


> 
> Also, I'd been thinking that this would be limited to a small set of devices
> (maybe just the emulators) to start with, and devices could be added as
> people test them and fix the policy as needed; does that sound reasonable?

That sounds fine - ideally we would this on the Aries device but flame if not that. Its hard to imagine there being priority for this though for 2.5, so we are probably looking at something in a 3.0 timeframe.

Comment 16

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

Attachment: Part 1: the file broker and some unit tests.

I've broken this patch into a few pieces in the hope of making it a little easier to read, but there isn't a good way to avoid this part being a big code drop.

I've added some f?s because this patch is doing delicate things with sockets (and potentially in async signal context), and it would be nice to have more eyes on it, but I'm not entirely sure who's best for that / if it'd make sense to ask for a full extra review.

Comment 17

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

Attachment: Part 2: seccomp-bpf integration.

The rest of these patches might make more sense to squash for landing, because they don't do anything until the last one adds the IPC glue.

This one has the seccomp-bpf trap routines that call the broker client.

Comment 18

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

Attachment: Part 3: the sandbox broker policy factory.

This is where the broker policy definition lives.  (Originally this was SandboxBrokerPolicy.{h,cpp}, but then I introduced a factory pattern to keep the leak checker from complaining, and it was confusing to have the file name not match the class name.)  The goldfish restriction is also here.

Comment 19

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

Attachment: Part 4: the PContent changes that glue everything together.

This is where everything is connected: getting the policy and starting the broker, using IPC to pass the fd to the child (optional, and always absent on other platforms; that seemed a little less inelegant than ifdef'ing the constructor message), and handing that down to the sandboxing code.

Comment 20

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8664589 [details] [diff] [review]
Part 4: the PContent changes that glue everything together.

Review of attachment 8664589 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ContentParent.cpp
@@ +2589,5 @@
> +#ifdef XP_LINUX
> +    if (shouldSandbox) {
> +        MOZ_ASSERT(!mSandboxBroker);
> +        brokerFd = FileDescriptor();
> +        auto policy = sSandboxBrokerPolicyFactory->GetContentPolicy(Pid());

Not a fan of auto here.

@@ +2590,5 @@
> +    if (shouldSandbox) {
> +        MOZ_ASSERT(!mSandboxBroker);
> +        brokerFd = FileDescriptor();
> +        auto policy = sSandboxBrokerPolicyFactory->GetContentPolicy(Pid());
> +        if (policy) {

As I understand the code, if |policy| is null then brokerFd will be a FileDescriptor with an invalid value, which will cause the content process to release assert. Is that what we want? I haven't looked to see when |policy| is null.

Comment 21

[Nathan Froyd [:froydnj]]

Comment on attachment 8664582 [details] [diff] [review]
Part 1: the file broker and some unit tests.

Review of attachment 8664582 [details] [diff] [review]:
-----------------------------------------------------------------

I am not an expert on socket handling and sending fds via them; with that caveat, this looks reasonable.  ++ for a decent amount of tests here as well.

A couple of comments below.

::: security/sandbox/linux/broker/SandboxBroker.cpp
@@ +100,5 @@
> +  return PL_DHASH_NEXT;
> +}
> +
> +SandboxBroker::Policy::Policy(const Policy& aOther) {
> +  aOther.mMap.EnumerateRead(HashCopyCallback, &mMap);

Could you please use iterators here rather than Enumerate?

http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsBaseHashtable.h#234

@@ +105,5 @@
> +}
> +
> +void
> +SandboxBroker::Policy::AddPath(int aPerms, const char* aPath,
> +                               bool aMightNotExist)

This parameter seems confusing, given that we're passing |true| for files we *know* exist to skip stat'ing them (AddTree, AddPrefix).  So a "true" aMightNotExist means "these really do exist", whereas "false" aMightNotExist means "might exists" or "definitively exists"?

I vote for using an enum parameter here (KnownExists, ConfirmExistence); perhaps that will make the callsites clearer as well.

@@ +144,5 @@
> +    struct dirent* de;
> +    if (!dirp) {
> +      return;
> +    }
> +    while ((de = readdir(dirp))) {

Might be able to avoid ugly parens with:

while(struct dirent* de = readdir(dirp)) {
  ...
}

::: security/sandbox/linux/broker/SandboxBroker.h
@@ +106,5 @@
> +  SandboxBroker(UniquePtr<const Policy> aPolicy, int aChildPid,
> +                int& aClientFd);
> +  void ThreadMain(void) override;
> +
> +  // Holding a UniquePtr should disallow copying, but to make that explicit:

++

Comment 22

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8664582 [details] [diff] [review]
Part 1: the file broker and some unit tests.

Review of attachment 8664582 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/sandbox/linux/SandboxBrokerClient.cpp
@@ +45,5 @@
> +  static const char kProcSelf[] = "/proc/self/";
> +  static const size_t kProcSelfLen = sizeof(kProcSelf) - 1;
> +  const char* path = aPath;
> +  // This buffer just needs to be large enough for any such path that
> +  // the policy would actually allow.  sizeof("/proc/2147483647/") == 18.

I wonder if this should use strlen(PID_MAX)+strlen(self)+2 (ie +6) (limits.h) (instead of 64) just in case things ever change ;)

::: security/sandbox/linux/gtest/TestBroker.cpp
@@ +58,5 @@
> +  }
> +  int LStat(const char* aPath, struct stat* aStat) {
> +    return mClient->LStat(aPath, aStat);
> +  }
> +  

nit extra space

Comment 23

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8664583 [details] [diff] [review]
Part 2: seccomp-bpf integration.

Review of attachment 8664583 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/sandbox/linux/SandboxFilter.cpp
@@ +450,5 @@
>  
>    virtual ResultExpr EvaluateSyscall(int sysno) const override {
> +    if (mBroker) {
> +      // Have broker; route the appropriate syscalls to it.
> +      switch (sysno) {

\o/

Comment 24

[Guillaume Destuynder [:kang] [this account is no longer active]]

@jld I'm wondering, for new calls what is the generally preferred way, using the thread-broker or calling b2g-parent functions via IPDL?
As per your comments it sounds like the later, but, I'm not ever sure what's the safest in reality since doing IPDL calls may seem more obscure than "whitelisting a path"

Comment 25

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8664585 [details] [diff] [review]
Part 3: the sandbox broker policy factory.

Review of attachment 8664585 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
@@ +41,5 @@
> +static const int wrlog = wronly | SandboxBroker::MAY_CREATE;
> +}
> +#endif
> +
> +SandboxBrokerPolicyFactory::SandboxBrokerPolicyFactory()

just a note that the rdonly (and rd* in general) may help bypassing ASLR in some cases (maps, libs), not sure if we should mention that here or not. its likely that if you reach that point you already had arbitrarily executable code somehow.

@@ +50,5 @@
> +  SandboxBroker::Policy* policy = new SandboxBroker::Policy;
> +
> +  policy->AddPath(rdwr, "/dev/genlock");  // bug 980924
> +  policy->AddPath(rdwr, "/dev/ashmem");   // bug 980947 (dangerous!)
> +  policy->AddPrefix(rdwr, "/dev", "kgsl");  // bug 995072

I'd argue that kgsl is potentially dangerous. Not sure what our stance is on this. I know that we have to leave it open until there's a GL proxy and other things (which can be a very long time)
Ex ttp://www.cvedetails.com/google-search-results.php?q=kgsl&sa=Search and https://www.codeaurora.org/projects/security-advisories/

@@ +62,5 @@
> +  policy->AddPath(rdonly, "/etc/media_profiles.xml"); // bug 1198419
> +  policy->AddPath(rdonly, "/etc/media_codecs.xml"); // bug 1198460
> +  policy->AddTree(rdonly, "/system/fonts"); // bug 1026063
> +
> +  policy->AddTree(wronly, "/dev/log"); // bug 1199857

just for clarity, might want to group all *w* policies together where possible (so all rdonly can't be easily confused with rdwr, wronly, wrlog by whoever reads this)

Comment 26

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8664589 [details] [diff] [review]
Part 4: the PContent changes that glue everything together.

Review of attachment 8664589 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ContentChild.cpp
@@ +1183,5 @@
> +    if (aBroker.type() == MaybeFileDesc::TFileDescriptor) {
> +        brokerFd = aBroker.get_FileDescriptor().PlatformHandle();
> +        // brokerFd < 0 means to allow direct filesystem access, so
> +        // make absolutely sure that doesn't happen if the parent
> +        // didn't intend it.

I guess that answers my comment 24 - should have looked at this first ;)

Comment 27

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

(In reply to Bill McCloskey (:billm) from comment #20)
> > +    if (shouldSandbox) {
> > +        MOZ_ASSERT(!mSandboxBroker);
> > +        brokerFd = FileDescriptor();
> > +        auto policy = sSandboxBrokerPolicyFactory->GetContentPolicy(Pid());
> > +        if (policy) {
> 
> As I understand the code, if |policy| is null then brokerFd will be a
> FileDescriptor with an invalid value, which will cause the content process
> to release assert. Is that what we want? I haven't looked to see when
> |policy| is null.

Yes, that's a mistake; thanks.  The parent process shouldn't be sending invalid FileDescriptor values; the nullptr policy means "allow direct FS access", which is what the void_t variant is for.  (We used to assert in the IPC code itself that invalid file descriptors weren't sent over IPC, but bug 831307 reduced it to a warning.)  The release assertion I have in ContentChild is doing what it's meant to… and it looks like it might plausibly go off if a dup()/DuplicateHandle() fails, but in that case there's not much we could reasonably do besides kill the process, so that's okay.

Comment 28

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

(In reply to Nathan Froyd [:froydnj] from comment #21)
> I vote for using an enum parameter here (KnownExists, ConfirmExistence);
> perhaps that will make the callsites clearer as well.

Good point, that “might not exist” isn't always what's going on; thanks.

But it'd be more like Unconditional and IfExistsNow, since the former includes cases where the file really doesn't exist yet (including when the broker itself is what creates it), and the latter includes cases where the file not existing is expected (e.g., on non-qcom devices).

Comment 29

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

(In reply to Guillaume Destuynder [:kang] from comment #22)
> > +  // This buffer just needs to be large enough for any such path that
> > +  // the policy would actually allow.  sizeof("/proc/2147483647/") == 18.
> 
> I wonder if this should use strlen(PID_MAX)+strlen(self)+2 (ie +6)
> (limits.h) (instead of 64) just in case things ever change ;)

The buffer is the whole path, so it also needs to include the length of the longest procfs file in the policy (e.g., /proc/N/smaps); making assumptions about that seems more fragile than depending on PID_MAX.  Also, even 64-bit pids would still leave a lot of space (and there are stronger dependencies on 32-bit pids in other places).

Comment 30

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

(In reply to Guillaume Destuynder [:kang] from comment #24)
> @jld I'm wondering, for new calls what is the generally preferred way, using
> the thread-broker or calling b2g-parent functions via IPDL?
> As per your comments it sounds like the later, but, I'm not ever sure what's
> the safest in reality since doing IPDL calls may seem more obscure than
> "whitelisting a path"

My vague understanding was that we'd prefer IPDL, but I can see how using the transparent brokering could be easier and/or more ‘obviously’ correct.  One clear advantage of using IPDL is that it does (more or less) the same thing on all platforms, instead of being completely separate like sandboxing is, so there's no need to duplicate code/policy for cross-platform things.  The IPC owner/peers might also have some useful insight here.

For this bug, I've followed :arroway's lead in filing followups for every whitelist entry, even if it doesn't seem likely that they'll be removed.  My thinking is that in the worst case they can just be WONTFIXed, which still leaves the possibility that they can be REOPENED if the situation changes, and in any case it's as good a place as any to keep details on the rationale/background.

Comment 31

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

Attachment: Part 1: the file broker and some unit tests. [v2]

Carrying over r+.

Comment 32

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

Attachment: Part 2: seccomp-bpf integration. [v2]

Carrying over r+.

Comment 33

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

Attachment: Part 3: the sandbox broker policy factory. [v2]

Carrying over r+.  Starting to question the wisdom of breaking the changes up into this many patches.

Comment 34

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

Attachment: Part 4: the PContent changes that glue everything together. [v2]

Carrying over r+.

Comment 35

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

Ship it!  Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=938c1470657e

Comment 36

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

On second thought, I'll land this.

Comment 37

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4615c7b8177
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5e11173ec72
https://hg.mozilla.org/integration/mozilla-inbound/rev/c50fbae3d1a3
https://hg.mozilla.org/integration/mozilla-inbound/rev/004a93703fa8

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d4615c7b8177
https://hg.mozilla.org/mozilla-central/rev/f5e11173ec72
https://hg.mozilla.org/mozilla-central/rev/c50fbae3d1a3
https://hg.mozilla.org/mozilla-central/rev/004a93703fa8