pagemap: make useable for non-privilege users

This patch sets bit 56 in pagemap if this page is mapped only once. It allows to detect exclusively used pages without exposing PFN:

present file exclusive state

0       0    0         non-present
1       1    0         file page mapped somewhere else
1       1    1         file page mapped only here
1       0    0         anon non-CoWed page (shared with parent/child)
1       0    1         anon CoWed page (or never forked)

Quoted 3 lines · Konstantin Khlebnikov

This patch sets bit 56 in pagemap if this page is mapped only once. It allows to detect exclusively used pages without exposing PFN:

Quoted 6 lines

present file exclusive state

0       0    0         non-present
1       1    0         file page mapped somewhere else
1       1    1         file page mapped only here
1       0    0         anon non-CoWed page (shared with parent/child)
1       0    1         anon CoWed page (or never forked)

Probably, worth noting that file-private pages are anon in this context.

This patch sets bit 56 in pagemap if this page is mapped only once. It allows to detect exclusively used pages without exposing PFN:

present file exclusive state

0       0    0         non-present
1       1    0         file page mapped somewhere else
1       1    1         file page mapped only here
1       0    0         anon non-CoWed page (shared with parent/child)
1       0    1         anon CoWed page (or never forked)

Signed-off-by Konstantin Khlebnikov <[email protected]>

Link lkml.kernel.org/r/CAEVpBa+_RyACkhODZrRvQLs80iy0sqpdrd0AaP_-tgnX3Y9yNQ@mail.gmail.com

---

v2:
* handle transparent huge pages
* invert bit and rename shared -> exclusive (less confusing name)

---
 Documentation/vm/pagemap.txt |    3 ++-
 fs/proc/task_mmu.c           |   10 ++++++++++
 tools/vm/page-types.c        |   12 ++++++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

Documentation/vm/pagemap.txt +2 -1

index 6bfbc172cdb9..3cfbbb333ea1 100644
@@ -16,7 +16,8 @@ There are three components to pagemap:
     * Bits 0-4   swap type if swapped
     * Bits 5-54  swap offset if swapped
     * Bit  55    pte is soft-dirty (see Documentation/vm/soft-dirty.txt)
-    * Bits 56-60 zero
+    * Bit  56    page exlusively mapped
+    * Bits 57-60 zero
     * Bit  61    page is file-page or shared-anon
     * Bit  62    page swapped
     * Bit  63    page present

fs/proc/task_mmu.c +10 -0

index 6dee68d013ff..29febec65de4 100644
@@ -982,6 +982,7 @@ struct pagemapread {
 #define PM_STATUS2(v2, x)   (__PM_PSHIFT(v2 ? x : PAGE_SHIFT))
 
 #define __PM_SOFT_DIRTY      (1LL)
+#define __PM_MMAP_EXCLUSIVE  (2LL)
 #define PM_PRESENT          PM_STATUS(4LL)
 #define PM_SWAP             PM_STATUS(2LL)
 #define PM_FILE             PM_STATUS(1LL)

@@ -1074,6 +1075,8 @@ static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
 
        if (page && !PageAnon(page))
                flags |= PM_FILE;
+       if (page && page_mapcount(page) == 1)
+               flags2 |= __PM_MMAP_EXCLUSIVE;
        if ((vma->vm_flags & VM_SOFTDIRTY))
                flags2 |= __PM_SOFT_DIRTY;
 

@@ -1119,6 +1122,13 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
                else
                        pmd_flags2 = 0;
 
+               if (pmd_present(*pmd)) {
+                       struct page *page = pmd_page(*pmd);
+
+                       if (page_mapcount(page) == 1)
+                               pmd_flags2 |= __PM_MMAP_EXCLUSIVE;
+               }
+
                for (; addr != end; addr += PAGE_SIZE) {
                        unsigned long offset;
                        pagemap_entry_t pme;

tools/vm/page-types.c +8 -0

index 8bdf16b8ba60..3a9f193526ee 100644
@@ -70,9 +70,12 @@
 #define PM_PFRAME(x)        ((x) & PM_PFRAME_MASK)
 
 #define __PM_SOFT_DIRTY      (1LL)
+#define __PM_MMAP_EXCLUSIVE  (2LL)
 #define PM_PRESENT          PM_STATUS(4LL)
 #define PM_SWAP             PM_STATUS(2LL)
+#define PM_FILE             PM_STATUS(1LL)
 #define PM_SOFT_DIRTY       __PM_PSHIFT(__PM_SOFT_DIRTY)
+#define PM_MMAP_EXCLUSIVE   __PM_PSHIFT(__PM_MMAP_EXCLUSIVE)
 
 
 /*

@@ -100,6 +103,8 @@
 #define KPF_SLOB_FREE          49
 #define KPF_SLUB_FROZEN                50
 #define KPF_SLUB_DEBUG         51
+#define KPF_FILE               62
+#define KPF_MMAP_EXCLUSIVE     63
 
 #define KPF_ALL_BITS           ((uint64_t)~0ULL)
 #define KPF_HACKERS_BITS       (0xffffULL << 32)

@@ -149,6 +154,9 @@ static const char * const page_flag_names[] = {
        [KPF_SLOB_FREE]         = "P:slob_free",
        [KPF_SLUB_FROZEN]       = "A:slub_frozen",
        [KPF_SLUB_DEBUG]        = "E:slub_debug",
+
+       [KPF_FILE]              = "F:file",
+       [KPF_MMAP_EXCLUSIVE]    = "1:mmap_exclusive",
 };
 
 

Hi Konstantin,

I hope you won't mind me thinking out loud here on the idea of adding a flag to the v2 pagemap fields... From a kernel PoV, I agree that this seems like the cleanest approach. However, with my application developer hat on:

1. I was hoping we'd be able to backport a compatible fix to older
kernels that might adopt the pagemap permissions change. Using the V2 format flags rules out doing this for kernels that are too old to have soft-dirty, I think.

2. From our software's PoV, I feel it's worth noting that it doesn't
strictly fix ABI compatibility, though I realise that's probably not your primary concern here. We'll need to modify our code to write the clear_refs file but that change is OK for us if it's the preferred solution.

In the patches I've been playing with, I was considering putting the Exclusive flag in the now-unused PFN field of the pagemap entries. Since we're specifically trying to work around for the lack of PFN information, would there be any appetite for mirroring this flag unconditionally into the now-empty PFN field (i.e. whether using v1 or v2 flags) when accessed by an unprivileged process?

I realise it's ugly from a kernel PoV and I feel a little bad for suggesting it - but it would address points 1 and 2 for us (our existing code just looks for changes in the pagemap entry, so sticking the flag in there would cause it to do the right thing).

I'm sorry to raise application-specific issues at this point; I appreciate that your primary concern is to improve the kernel and technically I like the approach that you've taken! I'll try and provide more code-oriented feedback once I've tried out the changes.

On 12.05.2015 15:05, Mark Williamson wrote:

Quoted 10 lines

1. I was hoping we'd be able to backport a compatible fix to older
kernels that might adopt the pagemap permissions change. Using the V2 format flags rules out doing this for kernels that are too old to have soft-dirty, I think.

2. From our software's PoV, I feel it's worth noting that it doesn't
strictly fix ABI compatibility, though I realise that's probably not your primary concern here. We'll need to modify our code to write the clear_refs file but that change is OK for us if it's the preferred solution.

I prefer to backport v2 format (except soft-dirty bit and clear_refs) into older kernels. Page-shift bits are barely used so nobody will see the difference.

Hi Konstantin,

Quoted 1 line · Konstantin Khlebnikov

On 12.05.2015 15:05, Mark Williamson wrote:

<snip>

Quoted 10 lines

Quoted 10 lines

1. I was hoping we'd be able to backport a compatible fix to older
kernels that might adopt the pagemap permissions change. Using the V2 format flags rules out doing this for kernels that are too old to have soft-dirty, I think.

2. From our software's PoV, I feel it's worth noting that it doesn't
strictly fix ABI compatibility, though I realise that's probably not your primary concern here. We'll need to modify our code to write the clear_refs file but that change is OK for us if it's the preferred solution.

<snip>

Quoted 3 lines

I prefer to backport v2 format (except soft-dirty bit and clear_refs) into older kernels. Page-shift bits are barely used so nobody will see the difference.

My concern was whether a change to format would be acceptable to include in the various -stable kernels; they are already including the additional protections on pagemap, so we're starting to need our fallback mode in distributions. Do you think that such a patch would be acceptable there?

(As an application vendor we're likely to be particularly stuck with what the commercial distributions decide to ship, which is why I'm trying to keep an eye on this)

I appreciate that this is a slightly administrative concern! I definitely like the technical approach of this code and it seems to work fine for us.

This patch makes pagemap readable for normal users back but hides physical addresses from them. For some use cases PFN isn't required at all: flags give information about presence, page type (anon/file/swap), soft-dirty mark, and hint about page mapcount state: exclusive(mapcount = 1) or (mapcount > 1).

Signed-off-by Konstantin Khlebnikov <[email protected]>

Fixes ab676b7d6fbf ("pagemap: do not leak physical addresses to non-privileged userspace")

Link lkml.kernel.org/r/1425935472-17949-1-git-send-email-kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org

---
 fs/proc/task_mmu.c |   36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

fs/proc/task_mmu.c +20 -11

index 29febec65de4..0b7a8ffec95f 100644
@@ -962,6 +962,7 @@ struct pagemapread {
        int pos, len;           /* units: PM_ENTRY_BYTES, not bytes */
        pagemap_entry_t *buffer;
        bool v2;
+       bool show_pfn;
 };
 
 #define PAGEMAP_WALK_SIZE      (PMD_SIZE)

@@ -1046,12 +1047,13 @@ out:
 static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
                struct vm_area_struct *vma, unsigned long addr, pte_t pte)
 {
-       u64 frame, flags;
+       u64 frame = 0, flags;
        struct page *page = NULL;
        int flags2 = 0;
 
        if (pte_present(pte)) {
-               frame = pte_pfn(pte);
+               if (pm->show_pfn)
+                       frame = pte_pfn(pte);
                flags = PM_PRESENT;
                page = vm_normal_page(vma, addr, pte);
                if (pte_soft_dirty(pte))

@@ -1087,15 +1089,19 @@ static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
 static void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
                pmd_t pmd, int offset, int pmd_flags2)
 {
+       u64 frame = 0;
+
        /*
         * Currently pmd for thp is always present because thp can not be
         * swapped-out, migrated, or HWPOISONed (split in such cases instead.)
         * This if-check is just to prepare for future implementation.
         */
-       if (pmd_present(pmd))
-               *pme = make_pme(PM_PFRAME(pmd_pfn(pmd) + offset)
-                               | PM_STATUS2(pm->v2, pmd_flags2) | PM_PRESENT);
-       else
+       if (pmd_present(pmd)) {
+               if (pm->show_pfn)
+                       frame = pmd_pfn(pmd) + offset;
+               *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
+                               PM_STATUS2(pm->v2, pmd_flags2));
+       } else
                *pme = make_pme(PM_NOT_PRESENT(pm->v2) | PM_STATUS2(pm->v2, pmd_flags2));
 }
 #else

@@ -1171,11 +1177,14 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 static void huge_pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
                                        pte_t pte, int offset, int flags2)
 {
-       if (pte_present(pte))
-               *pme = make_pme(PM_PFRAME(pte_pfn(pte) + offset)        |
-                               PM_STATUS2(pm->v2, flags2)              |
-                               PM_PRESENT);
-       else
+       u64 frame = 0;
+
+       if (pte_present(pte)) {
+               if (pm->show_pfn)
+                       frame = pte_pfn(pte) + offset;
+               *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
+                               PM_STATUS2(pm->v2, flags2));
+       } else
                *pme = make_pme(PM_NOT_PRESENT(pm->v2)                  |
                                PM_STATUS2(pm->v2, flags2));
 }

fs/proc/task_mmu.c +0 -0

        pm.v2 = soft_dirty_cleared;
        pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
        pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);

fs/proc/task_mmu.c static ssize_t pagemap_read(struct file *file, char __user *buf, · line 1276

Quoted 9 lines · Konstantin Khlebnikov

fs/proc/task_mmu.c +2 -0

@@ -1260,6 +1269,8 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
         if (!count)
                 goto out_task;
 
+       /* do not disclose physical addresses: attack vector */
+       pm.show_pfn = capable(CAP_SYS_ADMIN);
         pm.v2 = soft_dirty_cleared;
         pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
         pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);

NO! Dammit, no, no, no!

How many times must people do this major security faux-pas before we learn?

WE DO NOT CHECK CURRENT CAPABILITIES AT READ/WRITE TIME!

It's a bug. It's a security issue. It's not how Unix capabilities work!

Capabilities are checked at open time.:

fs/proc/task_mmu.c out: · line 1340

Quoted 7 lines

fs/proc/task_mmu.c +0 -3

@@ -1335,9 +1346,6 @@ out:
 
  static int pagemap_open(struct inode *inode, struct file *file)
  {
-       /* do not disclose physical addresses: attack vector */
-       if (!capable(CAP_SYS_ADMIN))
-               return -EPERM;

THIS is where you are supposed to check for capabilities. The place where you removed it!

The reason we check capabilities at open time, and open time ONLY is because that is really very integral to the whole Unix security model. Otherwise, you get into this situation:

- unprivileged process opens file

- unprivileged process tricks suid process to do the actual access for it

where the traditional model is to just force a "write()" by opening the file as stderr, and then executing a suid process (traditionally "sudo") that writes an error message to it.

So *don't* do permission checks using read/write time credentials. They are wrong.

Now, if there is some reason that you really can't do it when opening the file, and you actually need to use capability information at read/write time, you use the "file->f_cred" field, which is the open-time capabilities. So you _can_ do permission checks at read/write time, but you have to use the credentials of the opener, not "current".

So in this case, I guess you could use

         pm.show_pfn = file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN);

if you really need to do this at read time, and cannot fill in that "show_pfn" at open-time.

Linus

This patch removes page-shift bits (scheduled to remove since 3.11) and completes migration to the new bit layout. Also it cleans messy macro.

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
 fs/proc/task_mmu.c    |  152 ++++++++++++++++---------------------------------
 tools/vm/page-types.c |   29 +++------
 2 files changed, 58 insertions(+), 123 deletions(-)

fs/proc/task_mmu.c +0 -4

index 0b7a8ffec95f..66bc7207ce90 100644
@@ -710,23 +710,6 @@ const struct file_operations proc_tid_smaps_operations = {
 	.release	= proc_map_release,
 };
 
-/*
- * We do not want to have constant page-shift bits sitting in
- * pagemap entries and are about to reuse them some time soon.
- *

Quoted 27 lines · Konstantin Khlebnikov

This patch removes page-shift bits (scheduled to remove since 3.11) and completes migration to the new bit layout. Also it cleans messy macro.

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
  fs/proc/task_mmu.c    |  152 ++++++++++++++++---------------------------------
  tools/vm/page-types.c |   29 +++------
  2 files changed, 58 insertions(+), 123 deletions(-)

fs/proc/task_mmu.c +0 -10

index 0b7a8ffec95f..66bc7207ce90 100644
@@ -710,23 +710,6 @@ const struct file_operations proc_tid_smaps_operations = {
  	.release	= proc_map_release,
  };
 
-/*
- * We do not want to have constant page-shift bits sitting in
- * pagemap entries and are about to reuse them some time soon.
- *
- * Here's the "migration strategy":
- * 1. when the system boots these bits remain what they are,
- *    but a warning about future change is printed in log;
- * 2. once anyone clears soft-dirty bits via clear_refs file,
- *    these flag is set to denote, that user is aware of the
- *    new API and those page-shift bits change their meaning.

fs/proc/task_mmu.c const struct file_operations proc_tid_smaps_operations = { · line 726

Quoted 4 lines

fs/proc/task_mmu.c +0 -4

- *    The respective warning is printed in dmesg;
- * 3. In a couple of releases we will remove all the mentions
- *    of page-shift in pagemap entries.
- */

Wouldn't it be better to just have v2=1 by default for couple releases to see if anything breaks? This way we can revert easily if regression reported. I guess someone could miss this change coming if he didn't touch clear_refs.

This patchset tries to make pagemap useable again in the safe way. First patch adds bit 'map-exlusive' which is set if page is mapped only here. Second patch restores access for non-privileged users but hides pfn if task has no capability CAP_SYS_ADMIN. Third patch removes page-shift bits and completes migration to the new pagemap format (flags soft-dirty and mmap-exlusive are available only in the new format).

---

Konstantin Khlebnikov (3):
pagemap: add mmap-exclusive bit for marking pages mapped only here
pagemap: hide physical addresses from non-privileged users
pagemap: switch to the new format and do some cleanup

 Documentation/vm/pagemap.txt |    3 -
 fs/proc/task_mmu.c           |  178 +++++++++++++++++-------------------------
 tools/vm/page-types.c        |   35 ++++----
 3 files changed, 91 insertions(+), 125 deletions(-)

Hi Konstantin,

Thanks very much for continuing to look at this! It's very much appreciated. I've been investigating from our end but got caught up in some gnarly details of our pagemap-consuming code.

I like the approach and it seems like the information you're exposing will be useful for our application. I'll test the patch and see if it works for us as-is.

Will follow up with any comments on the individual patches.

Thanks,
Mark

Quoted 19 lines · Konstantin Khlebnikov

This patchset tries to make pagemap useable again in the safe way. First patch adds bit 'map-exlusive' which is set if page is mapped only here. Second patch restores access for non-privileged users but hides pfn if task has no capability CAP_SYS_ADMIN. Third patch removes page-shift bits and completes migration to the new pagemap format (flags soft-dirty and mmap-exlusive are available only in the new format).

---

Konstantin Khlebnikov (3):
pagemap: add mmap-exclusive bit for marking pages mapped only here
pagemap: hide physical addresses from non-privileged users
pagemap: switch to the new format and do some cleanup

 Documentation/vm/pagemap.txt |    3 -
 fs/proc/task_mmu.c           |  178 +++++++++++++++++-------------------------
 tools/vm/page-types.c        |   35 ++++----
 3 files changed, 91 insertions(+), 125 deletions(-)

Hi Konstantin,

I modified our code to check for the map-exclusive flag where it used to compare pageframe numbers. First tests look pretty promising, so this patch looks like a viable approach for us.

Is there anything further we can do to help?

Thanks,
Mark

Quoted 36 lines · Mark Williamson

Hi Konstantin,

Thanks very much for continuing to look at this! It's very much appreciated. I've been investigating from our end but got caught up in some gnarly details of our pagemap-consuming code.

I like the approach and it seems like the information you're exposing will be useful for our application. I'll test the patch and see if it works for us as-is.

Will follow up with any comments on the individual patches.

Thanks,
Mark

Quoted 19 lines · Konstantin Khlebnikov

This patchset tries to make pagemap useable again in the safe way. First patch adds bit 'map-exlusive' which is set if page is mapped only here. Second patch restores access for non-privileged users but hides pfn if task has no capability CAP_SYS_ADMIN. Third patch removes page-shift bits and completes migration to the new pagemap format (flags soft-dirty and mmap-exlusive are available only in the new format).

---

Konstantin Khlebnikov (3):
pagemap: add mmap-exclusive bit for marking pages mapped only here
pagemap: hide physical addresses from non-privileged users
pagemap: switch to the new format and do some cleanup

 Documentation/vm/pagemap.txt |    3 -
 fs/proc/task_mmu.c           |  178 +++++++++++++++++-------------------------
 tools/vm/page-types.c        |   35 ++++----
 3 files changed, 91 insertions(+), 125 deletions(-)