[MIPS] Remove NaClCheckDEP for MIPS

[MIPS] Remove NaClCheckDEP for MIPS

MIPS sandboxing does not require DEP, and the target MIPS platforms do not
support it. Thus, this change removes the code we do not need.

NOTRY=true
However, we have run a try job manually, and it passed fine, except for
the nacl-lucid64_validator_opt that is broken.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=2275
TEST= run smoke tests for MIPS platform

Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=11744

[petarj, 2013-07-08 22:28:24]

PTAL.

[Mark Seaborn, 2013-07-08 23:04:17]

On 8 July 2013 15:28, <petarj@mips.com> wrote:

> Description:
> [MIPS] Remove NaClCheckDEP for MIPS
>
> MIPS sandboxing does not require DEP, and the target MIPS platforms do not
> support it. Thus, this change removes the code we do not need.
>

Wow, really?  I had assumed that the MIPS sandbox worked like the ARM
sandbox in this regard.  It seems I didn't pay enough attention to the
definition of NACL_CONTROL_FLOW_MASK in nacl_config.h:
/* 16-byte bundles, 256MB code segment*/
# define NACL_CONTROL_FLOW_MASK     0x0FFFFFF0

This means there is a trivial sandbox escape from the MIPS sandbox at the
moment.  An attacker can load a nexe where the data segment starts below
the address 0x10000000, and they will be able to execute that data as code.

Currently sel_ldr does not prevent such data segments outright.  When the
IRT's code segment is loaded, this will prevent the user nexe's data
segment from being loaded very far below 0x10000000, but there's still a
gap between the top of the IRT's code segment and 0x10000000 where the user
nexe might place some data.

Would you like to do a separate change to fix this?

Can I suggest that you add your sandbox design doc
(native-client-mips-0.4.txt from
https://code.google.com/p/nativeclient/issues/detail?id=2275#c18) to NaCl's
documentation/ directory in SVN?  It would be useful to have this as a
code-reviewed document in the repo that we can all refer to.

LGTM for the present change.

Cheers,
Mark

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/groups/opt_out.

[petarj, 2013-07-09 17:44:48]

On 2013/07/08 23:04:17, Mark Seaborn wrote:
> An attacker can load a nexe where the data segment starts below
> the address 0x10000000, and they will be able to execute that data as code.

The data should never start below NACL_DATA_SEGMENT_START, I was under
impression there is already part of the code that prevents that.
I will check that and create a separate change to prevent it.


> Can I suggest that you add your sandbox design doc
> (native-client-mips-0.4.txt from
> https://code.google.com/p/nativeclient/issues/detail?id=2275#c18) to NaCl's
> documentation/ directory in SVN?

Sure, though that document needs updates first.

Regards,
Petar

[commit-bot: I haz the power, 2013-07-09 23:15:59]

CQ is trying da patch. Follow status at
https://nativeclient-status.appspot.com/cq/petarj@mips.com/18861003/1

[commit-bot: I haz the power, 2013-07-10 02:26:46]

Try job failure for 18861003-1 (retry) on nacl-win32_newlib_opt for steps
"annotate, large_tests".
It's a second try, previously, step "annotate" failed.
http://build.chromium.org/p/tryserver.nacl/buildstatus?builder=nacl-win32_new...

[commit-bot: I haz the power, 2013-07-10 07:42:30]

CQ is trying da patch. Follow status at
https://nativeclient-status.appspot.com/cq/petarj@mips.com/18861003/1

[commit-bot: I haz the power, 2013-07-10 14:25:29]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is nacl-lucid64_validator_opt, revision is 11668,
job name
was 18861003-1 (retry) (retry) (retry) (retry).

[commit-bot: I haz the power, 2013-07-11 10:17:13]

CQ is trying da patch. Follow status at
https://nativeclient-status.appspot.com/cq/petarj@mips.com/18861003/1

[commit-bot: I haz the power, 2013-07-11 17:08:40]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is nacl-lucid64_validator_opt, revision is 11668,
job name
was 18861003-1 (retry) (retry) (retry) (retry).

[commit-bot: I haz the power, 2013-07-11 22:11:28]

CQ is trying da patch. Follow status at
https://nativeclient-status.appspot.com/cq/petarj@mips.com/18861003/1

[commit-bot: I haz the power, 2013-07-11 22:11:47]

Change committed as 11744