mozilla :: #e10s

19 Apr 2017
16:16firebotBug 1349363 FIXED, [e10s-multi] Beta 54 experiment
16:16elandoes this have to be uplifted to beta for SV to start testing?
16:16elanor should I fire up the conversation, now?
16:20elanwait, mrbkap: can you get the uplift request handled for this ticket?
16:23felipeWe can give them a custom build with some settings changed so that it behaves like Beta
16:23felipeSo they can start earlier
16:25elanthat would be wonderful
16:25mrbkapelan: I'll do that.
16:25elanok, thank you so much
16:25mrbkapIt should be a beta build, right?
16:25elanfelipe: ^
16:25elanI think so but
16:26mrbkapfelipe: I'll push to try in about an hour to generate builds
16:27elanoh god wrong URL
16:27elanI'm in URL hell, one sec
16:27firebotBug 1352388 NEW, [Request] Softvision Testing for the e10s-multi System Add-On For Firefox 54 Beta Experiments
16:27elanmrbkap: ^
16:27elanthat's where you can add information about custom build readiness
16:28mrbkapelan: got it
16:28felipemrbkap: if you use that workaround patch that I showed you, it doesn't need to be a beta build
16:28felipeAh, hmm
16:28felipeYour new code also checks updateChannel == Beta
16:28felipeSo you would need to change that too
16:57mrbkapfelipe: what advantage is there to not giving them a beta build?
17:02felipemrbkap: what you mean by "a beta build"?
17:03mrbkapfelipe: a build off of the beta branch
17:03felipeah, right, but built from tryserver you mean, or locally?
17:04felipemrbkap: but to go straight to the point, just building from beta is not enough, it needs to have --enable-update-channel=beta on the mozconfig... And the advantage is me not having to discover how to tell tryserver to do this :P If you know how to do it, then no prob
17:05mrbkapfrom trysever I meant.
17:21mrbkapfelipe: fyi
17:21firebotBug 1280663 NEW, Uplift Simulation configs
17:23felipeoh that's useful!
19:59Gankrojimm / bobowen: I have been told to ask you about multi process security in here; I'm doing some IPC work, and I'd like to better understand what I can/can't let a malicious process do to another.
20:08jimmGankro: can you elaborate a bit more on what you're doing?
20:08jimmor is there a bug for the work?
20:10Gankrojimm: I'm working on this but idk if that will be very informative to you
20:10GankroI'm trying to harden the (currently unsound) communication between the content process and the render process
20:12Gankro(this is code in servo and firefox)
20:13elanmrbkap, felipe: I've lost track of where we are in the process of handing off something for SV to test
20:13elanwhat's the latest?
20:13felipemrbkap was gonna send the code to tryserver to produce builds for them to test
20:17jimmGankro: hmm interesting. this isn't something we can kick around in an irc conversation to get right and there are a couple poeple I can think of who should be involved who can help you. (Paul Theriault and Daniel Veditz come to mind). Can you put together a Google doc summarizing what the problem is, and maybe notes on approach ideas if you have them, and then cc people into that to kick off a
20:19jimmGankro: Alex_Gaynor might have some input as well. This has less to do with sandboxing, more to do with security conformance. Paul manages that group.
20:19Gankrojimm: ok, I'll look into writing up my notes (been talking to Alex in #security already :))
20:20jimmDefinitely get Paul involved, this is the kind of stuff he works on
20:23Alex_Gaynorjimm: one of the proximate questions that surfaced in #security was whether the parent should be resillient to DoS by large allocation from the child; is that something you have an opinion on, or more pauljt's domain?
20:24GankroAlso more broadly "what does the child do if the parent acts in a way that appears malicious" (e.g. sending 3 for a bool)
20:25Alex_GaynorThat probably has an answer that can be found by reading the paramtraits impl that the C++ IPC stuff uses. let me see if I can research this
20:25jimmwell, that would be a nice to have, if we can do it. I think a DoS though isn't as concerning as some sort of UAF issue triggered by the child. again though that's really Paul's domain.
20:27jimmNote that both the compositor and content processes will be sandboxed
20:28Gankrojimm: that varies a bit by platform, right? (iirc there's issues delegating opengl contexts on macos?)
20:28jimmand if we have those at the same level, an attack vector from content -> compositor isn't as serious as say, an attack vector from compositor -> browser
20:29jimmGankro: yes, we're still working on removing access to "risky things" in the content process
20:29jimmand that varies across platforms currently
20:30jimmAlso I think the compositor process will always have access to graphics driver resources, so mitigating attacks from content -> compositor is worthwhile
20:32jimmwe've also discussed whether certain gfx related resources should remain accessible to content (webgl is a good example)
20:39Alex_GaynorGankro: I'm pretty sure we eitehr ignore that message or we kill the process, and I'm trying to figure out which :-) I realize that's quite a range
20:43GankroAlex_Gaynor: it seems a bit bad to... not render..?
20:43jimmpretty sure we kill it
20:44Alex_GaynorGankro: well, these are cases that should never happen in regular operations right?
20:45jimmthe reading and writing code is here -
20:46jimmthe handling of errors is in the auto-generated code
20:47Alex_Gaynoroh, that explains why I wasn't finding it by grepping
20:47GankroAlex_Gaynor: i mean as a response to the browser misbehaving, appearing to lock up seems like a bad one
20:48jimmdoes DXR have auto-generated code indexed yet? pretty sure searchfox does.
20:48Alex_Gaynorjimm: I'm still in the habbit of using rg locally, I guess I should break that habbit and start using dxr/searchfox
20:49Alex_GaynorGankro: by misbehaving do you mean "there's a bug" or "malicious website"?
20:50Alex_GaynorGankro: If we consider a really malicious content process, chrome killing it is one of the better outcomes
20:50GankroAlex_Gaynor: not sure there's a difference
20:50Alex_GaynorGankro: well, "freezing up" is really bad UX if we're concerned this might happen in normal usage, but relatively ok compared to someone exploiting us to escape the sandbox
20:51GankroI guess "only" dropping a frame if it's "only" a bug is fine
20:51GankroBut I mean, not rendering seems like strictly worse than explicitly crashing
20:52jimmI'd be more concerned with a pwn'd content process, what can it throw at the browser or compositor that might trigger an exploit?
20:52Alex_GaynorWell, in the current context of content->chrome just killing the one content process makes sense, because the others live on. Maybe there's a different behavior for a compositor process
20:56jimmhere's a good example of reading -
21:10elanfelipe: thank you
22:03mrbkapelan: current status: I have a try run going that will generate builds for softvision to test with.
22:03mrbkapelan: that also means that I have patches that apply to beta and I'll request approval for them to land in a second.
22:04mrbkapelan: They have to land after the patches for bug 1346288 (which has approval to land on beta) but I have to regenerate one of those beta patches due to orange on try pointed out by my try push.
22:04firebot FIXED, Disable e10s-multi for SDK addon users
22:05* mrbkap is running on too-little sleep and repeating himself :(
22:11elanmrbkap: thank you. as long as this lands before go to build in taipei, we will be ok
22:11elanI'm worried SV wasn't able to validate it first though
22:12elanand that we are making a mistake by landing it and shipping it without them testing
22:12elanfelipe: do you have an opinion?
22:13mrbkapelan: When does it go live?
22:15mrbkapelan: Or, honestly, I can run through the most important stuff myself today.
22:24mrbkapelan: btw, do you still need info from Shell on the experiment bug? If not, I can clear that request.
22:34elanmrbkap: It should go live tomorrow, it will build while we are sleeping and be live by when we are awake, I think
22:34elanno I don't need anything from Shell thank you
22:35mrbkapelan: ok, I'll run through some smoketests on the builds produced by my try run, then.
22:35elanwonderful, thank you
22:35elanGrover is likely still working so you can just comment in the bug for testing too
22:35elanjust give him the Try link
22:35elanthank you
22:36mrbkapelan: fyi, I spoke with the devtools guys about some changes that affect the service worker debugger. I'm working on that patch now but I don't think that it needs to make beta 1.
22:36mrbkapelan: I'll try to get the changes in for beta 2 though.
22:37mrbkapelan: it's mostly cleanup but also changing the text displayed to the user to reflect what we're actually doing so worth uplifting.
22:37mrbkapelan: (that's bug 1357909)
22:37firebot NEW, Devtools followup for bug 1349363
22:39mrbkapah, that'll be a late string change
22:39* mrbkap wonders if it's worth it.
22:43elanwe can't unfreeze strings
22:43elanso I wouln't even go down that path unless they are preexisting
22:43elanmrbkap: ^
22:44elanOh I see, beta 2
22:44elanyes, let's focus on getting the experiment out with beta 1
22:44elanthis is super critical
22:44elanplease can you test before you work on the devtools debugger stuff?
22:44elanmrbkap: ^
22:44mrbkapelan: Yeah.
22:44elanthank you
23:11mrbkapelan: at first glance things seem to be working as expected. I get the cohort wrong if the user opted in and has an extension installed though.
23:12mrbkapelan: not sure how important that is.
23:22mrbkapelan: if the user opted into e10s via browser.tabs.remote.force-enable they won't get multi.
23:28mrbkapelan: in general, things work as expected.
23:28mrbkapfelipe: ping (when you're here)
23:30felipemrbkap: pong-ish (not at home but can answer something quick)
23:31mrbkapfelipe: of the stuff that I've found, how important do you think it is?
23:31mrbkapfelipe: also, the cohort-assigning code is a bit wonky, do you think it's worth fixing?
23:33felipemrbkap: what are you referring to? Something mentioned in an email?
23:33mrbkapfelipe: no, my messages to erin from ~10 minutes ago in this channel.
23:33felipeAh, let me see
23:34felipeOh, just the opt-in stuff?
23:34felipeDefinitely not important for beta 1
23:34felipeWhat is the "cohort-assigning is a bit wonky?"
23:38mrbkapfelipe: we don't update the cohort if addons prevent us from enabling multi (but allow single-process e10s)
23:39felipeNo big deal for a few days
23:39mrbkapfelipe: cool.
23:40mrbkapfelipe: btw, on beta, force-disabling e10s (not multi) causes us to get stuck with e10s off.
23:41mrbkapfelipe: we set browser.tabs.remote.autostart.2 to false and then it looks like it was user set, even after the force-disable pref is unset.
23:42felipeYou mean even prior to this patch?
23:42mrbkapfelipe: Yes.
23:52pauljtAlex_Gaynor: tedd and I talked about this during the audit of IPDL protocols
23:53pauljtthis would be a nice to have, but currently there are a lot of protocols which pass paramaters which result in largely unconstrained memory allocations
23:53pauljtespecially width/height parameters
23:54pauljtmy understanding was that if that solving this would require a concerted effort to review, identify and resolve all the places where we do this
23:56pauljtI think we have a doc somewhere which tracks the scale of this
23:57pauljtI'll find it when I get to the office
20 Apr 2017
No messages
Last message: 9 days and 2 hours ago