NaCl docs: add ARM 32-bit sandbox

NaCl docs: add ARM 32-bit sandbox

As discussed we'll add sandbox documentation to our reference section. This CL adds details on the ARM 32-bit sandbox, and is mostly a port of the following wiki page:
  http://www.chromium.org/nativeclient/reference/arm-overview
It also updates some parts of this page. I'll delete the wiki page once the document is live.

R= binji@chromium.org, sbc@chromium.org, kschimpf@chromium.org, awatson@chromium.org, mackinlay@chromium.org
BUG= none
TEST= none
NOTRY=true
(documentation only change)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=249773

[JF, 2014-02-06 19:10:07]

I updated the code blocks:
 - Some were too wide and wrapped around.
 - I added ``:prettyprint: 0`` to all of them because the pretty printer gets
confused by assembly. **This doesn't work and still used the wrong syntax**.

I also added the generated documents for what I'm adding.

Note that I get other generated documents, mostly from doxygen, that I ``git
checkout --``, this seems bad: different doxygen versions generate different
output, and we don't have a canonical version that we use.

[binji, 2014-02-06 21:23:39]

lgtm

I'd like to see the diffs on the generated documents, if you can. Maybe we can
smooth over the differences.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
File
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst
(right):

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:241:
Alternative Sandboxing
It's not immediately clear to me if this sandboxing is allowed, or just a
potentially different way to do it. I assume it is allowed, but maybe a quick
sentence like "Another valid way to load..."

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:253:
was zero---if the two values had no one-bits in common. In this case,
nit: "no one-bits" reads like "no one" bits to me. Maybe reword?

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:306:
Client. We do so by defining alternate, predicable
sp: predictable

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:322:
forbidden in these cases.
This is a bit unclear. What happens if you try to use the tst-based sequence on
these processors? Does it just not run?

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:356:
guard regions.
But what happens when you execute one of these sp-updating operations three
times? Isn't it possible that sp is of bounds in that case?

Or am I misunderstanding:

ldr  rX,  [sp],  #4!        ; loads from stack, then adds 4 to sp

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:389:
ldr Rn, [r9]     ; load use thread pointer.
load use?

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:413:
address from a register. The ``b`` (branch) and ``bl``
(*branch*)? What is the (* *) syntax doing anyway?

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:524:
language and insulting them in the other.
I think I need an example of this sentence. :)

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:538:
stored in ``lr``:
This is describing returning, correct? The way I read this it is not clear.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:567:
The implication here is that a *call*\ ---the *branch* that enter
s/enter/enters/?

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:574:
place by having the program run forever!).
"!)." looks weird to me. Isn't it supposed to be just "!)"

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:581:
address' code. For more information on ARM's *call*/*return* stack see
I've usually seen address's, but I defer to Andy on this. :)

[Karl, 2014-02-06 21:56:26]

lgtm

[JF, 2014-02-06 23:26:35]

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
File
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst
(right):

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:241:
Alternative Sandboxing
On 2014/02/06 21:23:39, binji wrote:
> It's not immediately clear to me if this sandboxing is allowed, or just a
> potentially different way to do it. I assume it is allowed, but maybe a quick
> sentence like "Another valid way to load..."

It's currently disallowed because NaCl can't really half-allow it. PNaCl could,
so the feature is still in. I'll update the documentation to make that clear by
moving the note up and enhancing it.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:253:
was zero---if the two values had no one-bits in common. In this case,
On 2014/02/06 21:23:39, binji wrote:
> nit: "no one-bits" reads like "no one" bits to me. Maybe reword?

Changed to "had no set bits in common".

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:306:
Client. We do so by defining alternate, predicable
On 2014/02/06 21:23:39, binji wrote:
> sp: predictable

Done.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:322:
forbidden in these cases.
On 2014/02/06 21:23:39, binji wrote:
> This is a bit unclear. What happens if you try to use the tst-based sequence
on
> these processors? Does it just not run?

I moved this note up and clarified.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:356:
guard regions.
On 2014/02/06 21:23:39, binji wrote:
> But what happens when you execute one of these sp-updating operations three
> times? Isn't it possible that sp is of bounds in that case?
> 
> Or am I misunderstanding:
> 
> ldr  rX,  [sp],  #4!        ; loads from stack, then adds 4 to sp

Correct, but the key here is that you also perform an access when you do this
(either before or after the update), so doing these updates in a loop is
perfectly fine because you won't be able to jump over the red zones.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:389:
ldr Rn, [r9]     ; load use thread pointer.
On 2014/02/06 21:23:39, binji wrote:
> load use?

"user", already fixed.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:413:
address from a register. The ``b`` (branch) and ``bl``
On 2014/02/06 21:23:39, binji wrote:
> (*branch*)? What is the (* *) syntax doing anyway?

The parens are just part of the text, no ReST meaning. The stars put emphasis,
I'm using it consistently for load/store/branch/call/return and such in this
document when I talk about a conceptual CPU instruction (so, not an actual
assembly instruction, but a concept that corresponds to CPU instruction(s)).

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:524:
language and insulting them in the other.
On 2014/02/06 21:23:39, binji wrote:
> I think I need an example of this sentence. :)

I know right!!! I actually have a G+ post about this, you should read it ;-)
  https://plus.google.com/+JFBastien/posts/eeRQWfUTEK2

Actually screw it I'll copy/paste it here for legacy, I'd like to have a better
explanation of this but couldn't figure one out:

 
Lazy +: is there a word to describe "a sentence that has one meaning in a
language, and another meaning in a different language"? It doesn't necessarily
have to be written, it could be phonetic.

For programming languages this is apparently called a polyglot (since the
program "speaks" multiple languages), but I'm asking specifically for natural
languages.

The only example I can remember at the moment won't make sense unless you're
French Canadian and know Latin: "alacris apis hostis" would usually translate to
"cheerful bee enemy", but phonetically it sounds like a fairly colorful French
Canadian swear.

For context, I'm trying to explain mode-switching attacks on ARM where
innocuous-looking ARM instructions are actually malicious Thumb instructions.
This isn't unlike hiding instructions within constants or within
sub-instructions in x86. This matters because ARM has the wonderful BX and BLX
indirect branch instructions which, when the bottom bit of the destination
register is set, switch from ARM to Thumb mode. FWIW the ARM NaCl sandbox
defends against this in a few ways. I've never heard of people doing such
attacks because usually arbitrary code execution is its own ROPy means, but in
the NaCl context an attacker may have extra very theoretical hoops to jump
through, and may want to switch to Thumb to execute carefully-crafted malicious
code spray that otherwise passed ARM validation.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:538:
stored in ``lr``:
On 2014/02/06 21:23:39, binji wrote:
> This is describing returning, correct? The way I read this it is not clear.

Yes, I clarified.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:567:
The implication here is that a *call*\ ---the *branch* that enter
On 2014/02/06 21:23:39, binji wrote:
> s/enter/enters/?

Done.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:574:
place by having the program run forever!).
On 2014/02/06 21:23:39, binji wrote:
> "!)." looks weird to me. Isn't it supposed to be just "!)"

Done.

https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:581:
address' code. For more information on ARM's *call*/*return* stack see
On 2014/02/06 21:23:39, binji wrote:
> I've usually seen address's, but I defer to Andy on this. :)

Leaving as-is for now.

[JF, 2014-02-07 19:27:41]

The CQ bit was checked by jfb@chromium.org

[commit-bot: I haz the power, 2014-02-07 19:30:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jfb@chromium.org/147803003/130001

[Andy, 2014-02-07 19:41:13]

LGTM with a few small comments inline.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File native_client_sdk/src/doc/_book.yaml (right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/_book.yaml:1: toc:
This file is being deprecated with the move to Chromesite.  Please update the
following file instead:

/src/chrome/common/extensions/docs/templates/json/chrome_sidenav.json

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst
(right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:5:
Native Client for ARM is a method for running programs---even malicious
"method" --> "sandboxing technology"

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:6:
ones---safely, on computers that use 32-bit ARM processors. It's an
"It's" --> "The ARM sandbox is"

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:7:
extension of earlier work on Native Client for x86 processors. This
"This security" --> "Security"

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File native_client_sdk/src/doc/reference/sandbox_internals/index.rst (right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/index.rst:7: The sandbox
internals explains implementation details about Native
Suggest beginning with:
"""
The sandbox internals documentation describes implementation details for Native
Client sandboxing,
"""

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/index.rst:11: Client
doesn't allow developers to write platform-specific assembly).
"doesn't allow developers to write platform-specific assembly" --> "does not
allow platform-specific assembly code"

[commit-bot: I haz the power, 2014-02-07 20:52:03]

Change committed as 249773

[JF, 2014-02-07 21:15:14]

Sorry Andy, I missed your comments before checking the commit box. I addressed
them in this CL instead:
  https://codereview.chromium.org/144253005

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File native_client_sdk/src/doc/_book.yaml (right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/_book.yaml:1: toc:
On 2014/02/07 19:41:13, Andy wrote:
> This file is being deprecated with the move to Chromesite.  Please update the
> following file instead:
> 
> /src/chrome/common/extensions/docs/templates/json/chrome_sidenav.json

That's pretty unfortunate: as a non-frequent contributor to documentation my
approach is to look at what other pages in our docs do, and then grep around in
native_client_sdk/docs. I didn't find this other sidenav file because it's
somewhere else!

Is there a way to fix this so that all the NaCl documentation is under
native_client_sdk/docs.

I'll update that file for now, it's just bound to get out of date IMO.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst
(right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:5:
Native Client for ARM is a method for running programs---even malicious
On 2014/02/07 19:41:13, Andy wrote:
> "method" --> "sandboxing technology"

Done.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:6:
ones---safely, on computers that use 32-bit ARM processors. It's an
On 2014/02/07 19:41:13, Andy wrote:
> "It's" --> "The ARM sandbox is"

Done.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:7:
extension of earlier work on Native Client for x86 processors. This
On 2014/02/07 19:41:13, Andy wrote:
> "This security" --> "Security"

Done.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File native_client_sdk/src/doc/reference/sandbox_internals/index.rst (right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/index.rst:7: The sandbox
internals explains implementation details about Native
On 2014/02/07 19:41:13, Andy wrote:
> Suggest beginning with:
> """
> The sandbox internals documentation describes implementation details for
Native
> Client sandboxing,
> """

Done.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/index.rst:11: Client
doesn't allow developers to write platform-specific assembly).
On 2014/02/07 19:41:13, Andy wrote:
> "doesn't allow developers to write platform-specific assembly" --> "does not
> allow platform-specific assembly code"

Done.