New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 791988 link

Starred by 2 users

CVE-2017-1000405: Security: "Dirty COW" variant on transparent huge pages

Project Member Reported by jorgelo@chromium.org, Dec 5 2017

Issue description

http://www.openwall.com/lists/oss-security/2017/11/30/1 for details.

From the link:

"This bug is not as severe as the original "Dirty cow" because an ext4 file
(or any other regular file) cannot be mapped using THP. Nevertheless, it
does allow us to overwrite read-only huge pages. For example, the zero huge
page and sealed shmem files can be overwritten (since their mapping can be
populated using THP). Note that after the first write page-fault to the
zero page, it will be replaced with a new fresh (and zeroed) thp."

Tentatively marking P2, we should consider landing the fix for M65 as it gets backported to the stable kernels.
 
Cc: -groeck@chromium.org wonderfly@google.com
Owner: groeck@chromium.org
Status: Assigned (was: Available)
Summary: CVE-2017-1000405: Security: "Dirty COW" variant on transparent huge pages (was: Security: "Dirty COW" variant on transparent huge pages)
Upstream commit a8f97366452ed491d13cf1e44241bc0b5740b1f0 ("mm, thp: Do not make page table dirty unconditionally in touch_p[mu]d()").

From earlier comments: "At least for ChromeOS, sys/kernel/mm/transparent_hugepage/enabled is set to 'madvise', not 'always', so presumably we are not directly affected (unless the exploit runs as root, but then everything is lost anyway)"


Cc: ckl@google.com
Guenter, do you know if we even need this in the first place? Given that it's marked "madvise", I feel we're only compiling it in because nobody took the time to disable it.
We'll get it through stable release merges (v4.414.4 and v4.4.104). I do not plan to apply the fix to stable releases.

Ah sorry I misspoke. By "need this" I meant the THP functionality, not the fix. Maybe it's easier to disable this feature in the kernel config?
#6: I don't think I understand. Easier than what ? Stable tree merges ?

Fixed in chromeos-4.14 with crbug.com/712119 (merge of v4.14.4 into chromeos-4.14).


Easier (and safer) than fixing the feature is just not having the feature, I'd say =)

But yeah, taking the fixes from stable is fine.
Cc: yinghan@google.com
> "At least for ChromeOS, sys/kernel/mm/transparent_hugepage/enabled is set to 'madvise', not 'always', so presumably we are not directly affected (unless the exploit runs as root, but then everything is lost anyway)"

Users could call madvise to allocate transparent huge pages and if use_zero_page is set to 1, which is the case for lakitu (and maybe other chromeos boards too), a thp_zero_page could get allocated and thus exploited. Unfortunately the POC from the CVE report didn't succeed on a lakitu image I tried, but based on our conversation with other kernel folks we believe it's still a possibility.

Guenter/Jorge, what do you guys think? If this does apply, then we'd like to cut a patch release on all branches asap.
Labels: -Pri-2 Pri-1
If we think this can be exploited then I'd say it's easier to disable THP in the kernel config than to get the patch applied to 4.4 (or maybe we can set use_zero_page to 0?).
If the concern is about the disruptiveness of the patch, it had been merged to linux-stable v4.4-y and didn't look intrusive. I have sent CL:810025 for backporting it.

I don't disagree with disabling THP or zero page for good, but I'd rather do it at head and not a stable branch (m63). FWIW, since m64, with CL:753406, /proc/self/mem is made read-only on ChromeOS including lakitu so I am not too worried about m64 and m65.
Alright, the patches look pretty non-intrusive so we should definitely land in 65. We do need backports for the other kernel versions though -- but that's what Guenter is working on.
Cc: kbleicher@chromium.org
Labels: M-64 Merge-Request-64 Merge-Request-63 M-63
We reached an agreement, that this has a high risk (at least for lakitu) and we want to patch all active milestones for it.

The backports to 64 and 63 are CL:810024 and CL:810025, both of which have got a CR+2 and trybots passed.
https://uberchromegw.corp.google.com/i/chromiumos.tryserver/builders/release/builds/18013
https://uberchromegw.corp.google.com/i/chromiumos.tryserver/builders/release/builds/18016

We have decided that the backport to 65 (current head) will come in with the merge of v4.4.104 that Guenter is working on and currently in pre-cq.

Dear TPMs, can we get your approval for the merge to 64 and 63? We have a tight schedule to release the patched images, hopefully by tomorrow. Thanks.
Project Member

Comment 17 by sheriffbot@chromium.org, Dec 6 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: gkihumba@chromium.org
Offline conversation with wonderfly@: Merge this to M64, kick off builders. Do some spot testing tomorrow on 4.4 devices and then merge to 63 if all looks ok.
I think this plan makes sense.
Labels: -Merge-Request-64 Merge-Approved-64
Approving merge to M64 Chrome OS.
Guenter, are you also tracking non-4.4 kernels?
Depends what you mean with non-4.4 kernels. chromeos-4.14, yes. I consider 4.12 obsolete, so no. 3.18 and older - I make a call on a case-by-case basis, but in general only fix severe problems.
For this bug, chromeos does not have CONFIG_TRANSPARENT_HUGEPAGE enabled, so I concluded that the patch it is not needed in older kernels.

I meant older Chrome OS kernels.

I don't think that's the case though, this is from my Samus (3.14) device:

localhost ~ # uname -a             
Linux localhost 3.14.0 #22 SMP PREEMPT Mon Nov 13 11:53:21 EST 2017 x86_64 Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz GenuineIntel GNU/Linux

localhost ~ # modprobe configs                                                                         
localhost ~ # zcat /proc/config.gz | grep HUGE                                                         
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
CONFIG_TRANSPARENT_HUGEPAGE=y
# CONFIG_TRANSPARENT_HUGEPAGE_ALWAYS is not set
CONFIG_TRANSPARENT_HUGEPAGE_MADVISE=y
You are right. This is quite interesting: You are correct, it is enabled, but I don't see what enables it (it is enabled with USE=transparent_hugepage, but I don't see that use flag set anywhere). As a result my scripts are missing it. Do you have an idea where the configuration is coming from ?

I do not where this is coming from (I assume this is not a default-on flag). Will take another look.
Ok looks like the patch *might* need to land on older kernels...I'm holding off building until we have all merges that are needed.
Hey Grace, if you can (and if it's easy to) kick off a build of a board using 4.4 kernel, that might give the lakitu folks some confidence for 63 anyways, even if we're waiting for the other kernel versions.

We should just backport this thing without waiting to see where the flag gets enabled.
I think I'll have to kick off the master builder rather than 4.14 boards only (slaves run into versioning issues it seems). So it's more efficient to have all needed merges in branch and kick everything off at once.
But if you prefer that I kick off master with just 4.14 merge, I can do that too. 
Quick clarification: *4.4*, not *4.14* for this, but we can still wait for now. Thanks!
Cc: ameyd@google.com
Status: Started (was: Assigned)
Does this code exist on 3.8/3.10? I didn't see backports, but I'm happy to help.
3.8 and 3.10 have so many unresolved security vulnerabilities that it hardly matters. On top of that, they don't run ARC++ which reduces the attack surface significantly. I'd rather focus on getting systems off 3.8 and 3.10 instead of trying to fix all security vulnerabilities in those kernels. It might take person-years to fix the unresolved vulnerabilities in 3.18 and older, and then we would face the same problem we are facing with uprevs: We might introduce new bugs because we are missing literally thousands of stable release patches, and those bugs might take months to track down and resolve.
3.18 and 3.14 are only marginally better than 3.10 and 3.8.
Also, the bug is tagged as low severity. For Medium and Low severity security bugs, my policy for 3.18 and older is: Apply an available fix unless it causes conflicts. Reasoning: if it does cause conflicts, the risk of introducing new problems is higher than the underlying vulnerability. Maybe we need to write a document describing the official policy ?

Labels: -Security_Severity-Low Security_Severity-Medium
That's fair -- although we marked it Low when we knew it required madvise, but before we realized that use_zero_page=1 would still allow escalation (as per c#11). This should likely be Medium now.

A document with that policy would likely be useful!
Project Member

Comment 37 by bugdroid1@chromium.org, Dec 7 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2a6ca2cc58a0c79606ad0065f1a6de1b6d6e6385

commit 2a6ca2cc58a0c79606ad0065f1a6de1b6d6e6385
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date: Thu Dec 07 23:22:52 2017

UPSTREAM: mm, thp: Do not make page table dirty unconditionally in touch_p[mu]d()

commit a8f97366452ed491d13cf1e44241bc0b5740b1f0 upstream.

Currently, we unconditionally make page table dirty in touch_pmd().
It may result in false-positive can_follow_write_pmd().

We may avoid the situation, if we would only make the page table entry
dirty if caller asks for write access -- FOLL_WRITE.

The patch also changes touch_pud() in the same way.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Salvatore Bonaccorso: backport for 3.16:
 - Adjust context
 - Drop specific part for PUD-sized transparent hugepages. Support
   for PUD-sized transparent hugepages was added in v4.11-rc1
]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 2b7ef6bdd28610f2907cd766362cf95d0d023801)
Signed-off-by: Daniel Wang <wonderfly@google.com>

BUG= chromium:791988 
TEST=trybot

Change-Id: I45832bb287b4712634fc2972cf7b4a086465a867
Reviewed-on: https://chromium-review.googlesource.com/812087
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Daniel Wang <wonderfly@google.com>

[modify] https://crrev.com/2a6ca2cc58a0c79606ad0065f1a6de1b6d6e6385/mm/huge_memory.c

Project Member

Comment 38 by bugdroid1@chromium.org, Dec 7 2017

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/332f31ccb26e74c097868147ba743c4bfa0f5496

commit 332f31ccb26e74c097868147ba743c4bfa0f5496
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date: Thu Dec 07 23:22:48 2017

UPSTREAM: mm, thp: Do not make page table dirty unconditionally in touch_p[mu]d()

commit a8f97366452ed491d13cf1e44241bc0b5740b1f0 upstream.

Currently, we unconditionally make page table dirty in touch_pmd().
It may result in false-positive can_follow_write_pmd().

We may avoid the situation, if we would only make the page table entry
dirty if caller asks for write access -- FOLL_WRITE.

The patch also changes touch_pud() in the same way.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Salvatore Bonaccorso: backport for 3.16:
 - Adjust context
 - Drop specific part for PUD-sized transparent hugepages. Support
   for PUD-sized transparent hugepages was added in v4.11-rc1
]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 2b7ef6bdd28610f2907cd766362cf95d0d023801)
Signed-off-by: Daniel Wang <wonderfly@google.com>

BUG= chromium:791988 
TEST=trybot

Change-Id: I45832bb287b4712634fc2972cf7b4a086465a867
Reviewed-on: https://chromium-review.googlesource.com/812086
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Daniel Wang <wonderfly@google.com>

[modify] https://crrev.com/332f31ccb26e74c097868147ba743c4bfa0f5496/mm/huge_memory.c

Labels: -Merge-Review-63 Merge-Approved-63
Approved merge to M63 after test team sanity checked 4.4 boards on M64 with the fix.
As per the revised internal guideline for this bug, madvise cases are exploitable. However, given the limited applicability (since FSs cannot be overwritten, you need to hope to exploit something by overwriting ZP), we can leave this at medium.
Project Member

Comment 41 by sheriffbot@chromium.org, Dec 11 2017

Cc: gkihumba@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

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 43 by bugdroid1@chromium.org, Dec 11 2017

Labels: merge-merged-release-R64-10176.B-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d933bfc76a9c20f6ae052f48ac8320f69bbb6ca8

commit d933bfc76a9c20f6ae052f48ac8320f69bbb6ca8
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date: Mon Dec 11 16:46:51 2017

UPSTREAM: mm, thp: Do not make page table dirty unconditionally in touch_p[mu]d()

commit a8f97366452ed491d13cf1e44241bc0b5740b1f0 upstream.

Currently, we unconditionally make page table dirty in touch_pmd().
It may result in false-positive can_follow_write_pmd().

We may avoid the situation, if we would only make the page table entry
dirty if caller asks for write access -- FOLL_WRITE.

The patch also changes touch_pud() in the same way.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Salvatore Bonaccorso: backport for 3.16:
 - Adjust context
 - Drop specific part for PUD-sized transparent hugepages. Support
   for PUD-sized transparent hugepages was added in v4.11-rc1
]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 2b7ef6bdd28610f2907cd766362cf95d0d023801)
Signed-off-by: Daniel Wang <wonderfly@google.com>

BUG= chromium:791988 
TEST=trybot

Change-Id: I45832bb287b4712634fc2972cf7b4a086465a867
Reviewed-on: https://chromium-review.googlesource.com/814415
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Guenter Roeck <groeck@chromium.org>
Tested-by: Guenter Roeck <groeck@chromium.org>

[modify] https://crrev.com/d933bfc76a9c20f6ae052f48ac8320f69bbb6ca8/mm/huge_memory.c

Project Member

Comment 44 by bugdroid1@chromium.org, Dec 11 2017

Labels: merge-merged-release-R64-10176.B-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7347b45f61e2de40a85a0b65b94ff05f302e8ba5

commit 7347b45f61e2de40a85a0b65b94ff05f302e8ba5
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date: Mon Dec 11 16:46:59 2017

UPSTREAM: mm, thp: Do not make page table dirty unconditionally in touch_p[mu]d()

commit a8f97366452ed491d13cf1e44241bc0b5740b1f0 upstream.

Currently, we unconditionally make page table dirty in touch_pmd().
It may result in false-positive can_follow_write_pmd().

We may avoid the situation, if we would only make the page table entry
dirty if caller asks for write access -- FOLL_WRITE.

The patch also changes touch_pud() in the same way.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Salvatore Bonaccorso: backport for 3.16:
 - Adjust context
 - Drop specific part for PUD-sized transparent hugepages. Support
   for PUD-sized transparent hugepages was added in v4.11-rc1
]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 2b7ef6bdd28610f2907cd766362cf95d0d023801)
Signed-off-by: Daniel Wang <wonderfly@google.com>

BUG= chromium:791988 
TEST=trybot

Change-Id: I45832bb287b4712634fc2972cf7b4a086465a867
Reviewed-on: https://chromium-review.googlesource.com/814414
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Guenter Roeck <groeck@chromium.org>
Tested-by: Guenter Roeck <groeck@chromium.org>

[modify] https://crrev.com/7347b45f61e2de40a85a0b65b94ff05f302e8ba5/mm/huge_memory.c

Labels: -Merge-Approved-63 -Merge-Approved-64
Status: Fixed (was: Started)
Thanks Guenter!
Project Member

Comment 47 by bugdroid1@chromium.org, Dec 12 2017

Labels: merge-merged-release-R62-9901.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a9f6452a69781b2a3b1e8431b74a460ff07702a5

commit a9f6452a69781b2a3b1e8431b74a460ff07702a5
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date: Tue Dec 12 01:32:55 2017

UPSTREAM: mm, thp: Do not make page table dirty unconditionally in touch_p[mu]d()

commit a8f97366452ed491d13cf1e44241bc0b5740b1f0 upstream.

Currently, we unconditionally make page table dirty in touch_pmd().
It may result in false-positive can_follow_write_pmd().

We may avoid the situation, if we would only make the page table entry
dirty if caller asks for write access -- FOLL_WRITE.

The patch also changes touch_pud() in the same way.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Salvatore Bonaccorso: backport for 3.16:
 - Adjust context
 - Drop specific part for PUD-sized transparent hugepages. Support
   for PUD-sized transparent hugepages was added in v4.11-rc1
]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 2b7ef6bdd28610f2907cd766362cf95d0d023801)
Signed-off-by: Daniel Wang <wonderfly@google.com>

BUG= chromium:791988 
BUG=b:70219322
TEST=trybot

Change-Id: I45832bb287b4712634fc2972cf7b4a086465a867
Reviewed-on: https://chromium-review.googlesource.com/821430
Trybot-Ready: Xuewei Zhang <xueweiz@google.com>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Commit-Queue: Xuewei Zhang <xueweiz@google.com>
Tested-by: Xuewei Zhang <xueweiz@google.com>

[modify] https://crrev.com/a9f6452a69781b2a3b1e8431b74a460ff07702a5/mm/huge_memory.c

Project Member

Comment 48 by sheriffbot@chromium.org, Dec 13 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
 Issue 796866  has been merged into this issue.
Project Member

Comment 50 by sheriffbot@chromium.org, Mar 20 2018

Labels: -Restrict-View-SecurityNotify 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
Project Member

Comment 51 by sheriffbot@chromium.org, Mar 27 2018

Labels: -M-63 -M-64

Sign in to add a comment