Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(498)

Issue 10039001: NaCl: Supply Windows handle-passing function (Closed)

Created:
8 years, 8 months ago by Mark Seaborn
Modified:
8 years, 8 months ago
CC:
chromium-reviews, yzshen+watch_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, brettw-cc_chromium.org, piman+watch_chromium.org, native-client-reviews_googlegroups.com, ihf+watch_chromium.org
Visibility:
Public.

Description

Supply Windows handle-passing function to NaCl Pass BrokerDuplicateHandle() to the NaCl loader process in nacl_listener.cc. Pass BrokerDuplicateHandle() to the NaCl trusted plugin. We need to add this to PPB_NaCl_Private in order to pass it through. Remove the use of the "init_handle_passing" SRPC call. Otherwise the NaCl process will attempt to do an imc_connect() to the renderer, which involves sending a handle to it, which fails. Add a wrapper for AddTargetPeer() to 'content' so that nacl_process_host.cc can use it. Change the renderer's handle-passing policy to allow sending handles other than Sections. The NaCl trusted plugin sends other handle types to the NaCl loader process. This change will allow the sandbox to be tightened up, in the future, so that the NaCl loader process and the renderer process do not have handles to each other. BUG=http://code.google.com/p/nativeclient/issues/detail?id=2719 TEST=nacl_integration etc. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=132496

Patch Set 1 #

Patch Set 2 : Update to new names #

Patch Set 3 : Add AddTargetPeer() call #

Patch Set 4 : Tweak types #

Total comments: 2

Patch Set 5 : Stricter handle policy #

Patch Set 6 : Rebase #

Patch Set 7 : Fix + comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -35 lines) Patch
M chrome/browser/nacl_host/nacl_process_host.cc View 1 2 3 4 5 3 chunks +15 lines, -0 lines 0 comments Download
M chrome/nacl/nacl_listener.cc View 1 2 3 4 5 3 chunks +18 lines, -0 lines 0 comments Download
M chrome/renderer/chrome_ppapi_interfaces.cc View 1 2 3 4 5 2 chunks +19 lines, -0 lines 0 comments Download
M content/common/sandbox_policy.cc View 1 2 3 4 5 6 2 chunks +12 lines, -0 lines 0 comments Download
M content/public/common/sandbox_init.h View 1 2 3 4 5 6 1 chunk +9 lines, -0 lines 0 comments Download
M ppapi/c/private/ppb_nacl_private.h View 1 2 3 4 5 3 chunks +18 lines, -2 lines 0 comments Download
M ppapi/native_client/src/trusted/plugin/module_ppapi.cc View 1 2 2 chunks +8 lines, -0 lines 0 comments Download
M ppapi/native_client/src/trusted/plugin/service_runtime.cc View 1 2 1 chunk +0 lines, -33 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Mark Seaborn
@Justin: This change is not committable yet (it depends on http://codereview.chromium.org/9960060/) and I am not ...
8 years, 8 months ago (2012-04-10 17:47:17 UTC) #1
Mark Seaborn
This should be ready to go now that http://codereview.chromium.org/9960045/ is committed. Please take a look. ...
8 years, 8 months ago (2012-04-12 17:40:52 UTC) #2
dmichael (off chromium)
ppapi/* lgtm
8 years, 8 months ago (2012-04-12 18:44:48 UTC) #3
jschuh
http://codereview.chromium.org/10039001/diff/12001/content/common/sandbox_policy.cc File content/common/sandbox_policy.cc (right): http://codereview.chromium.org/10039001/diff/12001/content/common/sandbox_policy.cc#newcode386 content/common/sandbox_policy.cc:386: L"*"); Does this really need to be "*"?
8 years, 8 months ago (2012-04-12 22:42:03 UTC) #4
Mark Seaborn
http://codereview.chromium.org/10039001/diff/12001/content/common/sandbox_policy.cc File content/common/sandbox_policy.cc (right): http://codereview.chromium.org/10039001/diff/12001/content/common/sandbox_policy.cc#newcode386 content/common/sandbox_policy.cc:386: L"*"); On 2012/04/12 22:42:03, Justin Schuh wrote: > Does ...
8 years, 8 months ago (2012-04-13 00:50:28 UTC) #5
jschuh
On 2012/04/13 00:50:28, Mark Seaborn wrote: > http://codereview.chromium.org/10039001/diff/12001/content/common/sandbox_policy.cc > File content/common/sandbox_policy.cc (right): > > http://codereview.chromium.org/10039001/diff/12001/content/common/sandbox_policy.cc#newcode386 ...
8 years, 8 months ago (2012-04-13 00:58:46 UTC) #6
sehr (please use chromium)
On 2012/04/13 00:58:46, Justin Schuh wrote: > On 2012/04/13 00:50:28, Mark Seaborn wrote: > > ...
8 years, 8 months ago (2012-04-13 19:54:02 UTC) #7
Mark Seaborn
I've rebased this now. @jam: Please review the content/public change.
8 years, 8 months ago (2012-04-16 17:47:18 UTC) #8
jam
8 years, 8 months ago (2012-04-16 23:46:23 UTC) #9
lgtm

Powered by Google App Engine
This is Rietveld 408576698