Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Description • 8 years ago

(In reply to Mike Hommey [:glandium] from bug 1268733 comment #16)
> Couldn't we also just have mozsandbox as a shared library, including the
> hooks, and have GeckoChildProcessHost set LD_PRELOAD to point to it?

I like this idea, but I didn't want to block bug 1268733 on it, so now it's a followup bug.

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 1 • 8 years ago

Looks like this needs peers from: sandboxing, build, and IPC.

	Julian Hector [:tedd] [:jhector]
	Comment 2 • 8 years ago

Comment on attachment 8806726 [details] [diff] [review]
bug1313218-sandbox-preload-hg1.diff

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

lgtm.

	Mike Hommey [:glandium]
	Comment 3 • 8 years ago

Comment on attachment 8806726 [details] [diff] [review]
bug1313218-sandbox-preload-hg1.diff

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

::: ipc/glue/GeckoChildProcessHost.cpp
@@ +843,5 @@
>  
> +#if defined(XP_LINUX) && defined(MOZ_SANDBOX)
> +  // Preload libmozsandbox.so so it can interpose sigprocmask et al.
> +  // (This could be made conditional on intent to use sandboxing, but
> +  // it's harmless for non-sandboxed processes.)

You could even add that it was loaded before in every case anyways by being linked to the executable.

@@ +855,5 @@
> +      preload.Append(' ');
> +      preload.Append(oldPreload);
> +    }
> +    // Explicitly construct the std::string to make it clear that this
> +    // isn't retaining a pointer to the nsCString's buffer.

FWIW, other places doing the same thing in the same file don't make it explicit. It's fine to make it explicit, but then, it should be explicit everywhere. Or just don't care and don't make it explicit.

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 4 • 8 years ago

Rebased (trivial conflict with bug 1272062) and adjusted a comment slightly as suggested by glandium.

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 5 • 8 years ago

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e4d9785ea0fb19ee237db83767b9026ca4bbcc0 (unsquashed, but the same changes).

	Pulsebot
	Comment 6 • 8 years ago

Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/97bf97176319
Preload libmozsandbox.so in child processes on Linux. r=tedd r=billm r=glandium

	Phil Ringnalda (:philor)
	Comment 7 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/97bf97176319