2020-06-04 10:40:50

by Fan Yang

[permalink] [raw]
Subject: [PATCH v3] mm: Fix mremap not considering huge pmd devmap

The original code in mm/mremap.c checks huge pmd by:

if (is_swap_pmd(*old_pmd) || pmd_trans_huge(*old_pmd)) {

However, a DAX mapped nvdimm is mapped as huge page (by default) but
it is not transparent huge page (_PAGE_PSE | PAGE_DEVMAP). This
commit changes the condition to include the case.

This addresses CVE-2020-10757.

Fixes: 5c7fb56e5e3f ("mm, dax: dax-pmd vs thp-pmd vs hugetlbfs-pmd")
Cc: <[email protected]>
Reported-by: Fan Yang <[email protected]>
Signed-off-by: Fan Yang <[email protected]>
Tested-by: Fan Yang <[email protected]>
Tested-by: Dan Williams <[email protected]>
Reviewed-by: Dan Williams <[email protected]>
Acked-by: Kirill A. Shutemov <[email protected]>

---

Changelog v2->v3:
- Added "Acked-by: Kirill..."

Changelog v1->v2:
- Removed some paragraph in commit msg, removed the comment in
mm/mremap.c, and added a NOTE in where pmd_trans_huge is defined.
- Added "Reviewed-by: Dan..."
- Added "Fixes: 5c7fb56e5e3f..."
- Added "Cc: <[email protected]>"
---
arch/x86/include/asm/pgtable.h | 1 +
mm/mremap.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 4d02e64af1b3..19cdeebfbde6 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -257,6 +257,7 @@ static inline int pmd_large(pmd_t pte)
}

#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+/* NOTE: when predicate huge page, consider also pmd_devmap, or use pmd_large */
static inline int pmd_trans_huge(pmd_t pmd)
{
return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE;
diff --git a/mm/mremap.c b/mm/mremap.c
index 6aa6ea605068..57b1f999f789 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -266,7 +266,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
new_pmd = alloc_new_pmd(vma->vm_mm, vma, new_addr);
if (!new_pmd)
break;
- if (is_swap_pmd(*old_pmd) || pmd_trans_huge(*old_pmd)) {
+ if (is_swap_pmd(*old_pmd) || pmd_trans_huge(*old_pmd) || pmd_devmap(*old_pmd)) {
if (extent == HPAGE_PMD_SIZE) {
bool moved;
/* See comment in move_ptes() */
--
2.25.4



2020-06-04 18:26:48

by Ajay Kaher

[permalink] [raw]
Subject: [PATCH v3] mm: Fix mremap not considering huge pmd devmap

Hi Fan,

Able to reproduce this issue on v4.19.y using your test program.

And as per commit message it fixes commit 5c7fb56e5e3f
("mm, dax: dax-pmd vs thp-pmd vs hugetlbfs-pm”) at kernel version v4.5.
So, v4.9.y should be vulnerable, however not able to reproduce on v4.9.y.
Does any specific scenerio need to test for v4.9.y?

For v4.9, modified test program as MAP_SHARED_VALIDATE is not available:
- return mmap(NULL, REGION_PM_SIZE, PROT, MAP_SHARED_VALIDATE|MAP_SYNC,
+ return mmap(NULL, REGION_PM_SIZE, PROT, MAP_SHARED|MAP_SYNC,

Let me know if I need to test some other way for v4.9.y.

-Ajay

2020-06-05 05:12:22

by Fan Yang

[permalink] [raw]
Subject: Re: [PATCH v3] mm: Fix mremap not considering huge pmd devmap

Hi Ajay,

> On Jun 5, 2020 at 02:23,Ajay Kaher <[email protected]> wrote:
>
> So, v4.9.y should be vulnerable, however not able to reproduce on v4.9.y.
> Does any specific scenerio need to test for v4.9.y?
>
> For v4.9, modified test program as MAP_SHARED_VALIDATE is not available:
> - return mmap(NULL, REGION_PM_SIZE, PROT, MAP_SHARED_VALIDATE|MAP_SYNC,
> + return mmap(NULL, REGION_PM_SIZE, PROT, MAP_SHARED|MAP_SYNC,
>
> Let me know if I need to test some other way for v4.9.y.
>

I further looked into this. In v4.9, fsdax (mount a dax file
system, then open, mmap, mremap) does not suffer this issue
because fsdax does not use huge page (FS_DAX_PMD is marked
BROKEN).

fs/dax.c:dax_pmd_fault:

if (!IS_ENABLED(CONFIG_FS_DAX_PMD))
return VM_FAULT_FALLBACK;

fs/Kconfig:

config FS_DAX_PMD
bool
default FS_DAX
depends on FS_DAX
depends on ZONE_DEVICE
depends on TRANSPARENT_HUGEPAGE
depends on BROKEN


However, I can re-produce the issue for the devdax mode. Here is how
I re-produce it:

1. It seems some interface changed for ndctl. So I use an old
commit (4295f1ea614a26e1304ed590fb7209c8c78270ab) in the repo
https://github.com/pmem/ndctl.
2. sudo ./ndctl/ndctl create-namespace -f -t pmem -m dax -e 'namespace0.0'
3. then use the following program:

#define _GNU_SOURCE
#include <sys/mman.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <fcntl.h>
#include <stdlib.h>
#include <errno.h>

#define PROT PROT_READ|PROT_WRITE

#define REGION_PM_TMP_PATH "/dev/dax0.0"

#define REGION_MEM_SIZE 1024ULL*1024*1024*2
#define REGION_PM_SIZE 1024ULL*1024*1024*4
#define REMAP_MEM_OFF 1024ULL*1024*1024*1
#define REMAP_PM_OFF 1024ULL*1024*1024*3
#define REMAP_SIZE 1024ULL*1024*1024*1

#define REGION_MEM_PTR ((void *)0x7fd400000000ULL)
#define REGION_PM_PTR ((void *)0x4fd300000000ULL)

char * map_tmp_pm_region(void)
{
int fd;

fd = open(REGION_PM_TMP_PATH, O_RDWR, 0644);
if (fd < 0) {
perror(REGION_PM_TMP_PATH);
exit(-1);
}

return mmap(REGION_PM_PTR, REGION_PM_SIZE, PROT, MAP_SHARED|MAP_SYNC,
fd, 0);
}

int main(int argc, char **argv)
{
char *regm, *regp, *remap;
int ret;

regm = mmap(REGION_MEM_PTR, REGION_MEM_SIZE, PROT, MAP_PRIVATE|MAP_ANONYMOUS,
-1, 0);
if (regm == MAP_FAILED) {
perror("regm");
return -1;
}

regp = map_tmp_pm_region();
if (regp == MAP_FAILED) {
perror("regp");
return -1;
}

memset(regm, 'a', REGION_MEM_SIZE);
memset(regp, 'i', REGION_PM_SIZE);

remap = mremap(regp + REMAP_PM_OFF, REMAP_SIZE, REMAP_SIZE,
MREMAP_MAYMOVE|MREMAP_FIXED, regm + REMAP_MEM_OFF);
if (remap != regm + REMAP_MEM_OFF) {
perror("mremap");
return -1;
}

*(regm + REMAP_MEM_OFF) = 0x00;
return 0;
}

4. Then I was able to see the "Corrupted page table" message in dmesg.

Best regards,
Fan