diff for duplicates of <1531868610.3541.21.camel@intel.com> diff --git a/a/content_digest b/N1/content_digest index ec31806..ca19db3 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -28,7 +28,10 @@ Mike Kravetz <mike.kravetz@oracle.com> Nadav Amit <nadav.amit@gmail.com> Oleg Nesterov <oleg@redhat.com> - " Pavel Machek <pavel@ucw.cz>Peter\0" + Pavel Machek <pavel@ucw.cz> + Peter Zijlstra <peterz@infradead.org> + Ravi V. Shankar <ravi.v.shankar@intel.com> + " Vedvyas Shanbhogue <vedvyas.shanbhogue@intel.com>\0" "\00:1\0" "b\0" "On Fri, 2018-07-13 at 11:26 -0700, Dave Hansen wrote:\n" @@ -106,4 +109,4 @@ "\302\240\n" "\302\240\tpage = vm_normal_page(vma, address, pte);" -31ed5e5878d3bb220e71c19ccff9bf46e851fa7a9a531a9421fff74bdbdc31b2 +260ab00548e0d8c099e0b717e98d59ab2557d96cc8501385358c615e5beff74a
diff --git a/a/1.txt b/N2/1.txt index f655ea6..7d6065b 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -72,3 +72,8 @@ index fc5f98069f4e..45a0837b27f9 100644 } page = vm_normal_page(vma, address, pte); + +-- +To unsubscribe from this list: send the line "unsubscribe linux-doc" in +the body of a message to majordomo@vger.kernel.org +More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/a/content_digest b/N2/content_digest index ec31806..d6a0b1a 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -28,7 +28,10 @@ Mike Kravetz <mike.kravetz@oracle.com> Nadav Amit <nadav.amit@gmail.com> Oleg Nesterov <oleg@redhat.com> - " Pavel Machek <pavel@ucw.cz>Peter\0" + Pavel Machek <pavel@ucw.cz> + Peter Zijlstra <peterz@infradead.org> + Ravi V. Shankar <ravi.v.shankar@intel.com> + " Vedvyas Shanbhogue <vedvyas.shanbhogue@intel.com>\0" "\00:1\0" "b\0" "On Fri, 2018-07-13 at 11:26 -0700, Dave Hansen wrote:\n" @@ -104,6 +107,11 @@ "+\t\t}\n" "\302\240\t}\n" "\302\240\n" - "\302\240\tpage = vm_normal_page(vma, address, pte);" + "\302\240\tpage = vm_normal_page(vma, address, pte);\n" + "\n" + "--\n" + "To unsubscribe from this list: send the line \"unsubscribe linux-doc\" in\n" + "the body of a message to majordomo@vger.kernel.org\n" + More majordomo info at http://vger.kernel.org/majordomo-info.html -31ed5e5878d3bb220e71c19ccff9bf46e851fa7a9a531a9421fff74bdbdc31b2 +b5a4125713f618e84f76dc7bdb278fde3b3c93e01a9a7ea4f2322bfa6fc25b60
diff --git a/a/1.txt b/N3/1.txt index f655ea6..0e65bc5 100644 --- a/a/1.txt +++ b/N3/1.txt @@ -2,34 +2,34 @@ On Fri, 2018-07-13 at 11:26 -0700, Dave Hansen wrote: > On 07/11/2018 10:05 AM, Yu-cheng Yu wrote: > > > > My understanding is that we don't want to follow write pte if the page -> > is shared as read-only. For a SHSTK page, that is (R/O + DIRTY_SW), -> > which means the SHSTK page has not been COW'ed. Is that right? +> > is shared as read-only. A For a SHSTK page, that is (R/O + DIRTY_SW), +> > which means the SHSTK page has not been COW'ed. A Is that right? > Let's look at the code again: > > > > > -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags) > > +static inline bool can_follow_write_pte(pte_t pte, unsigned int flags, > > + bool shstk) -> > { +> > A { > > + bool pte_cowed = shstk ? is_shstk_pte(pte):pte_dirty(pte); > > + -> > return pte_write(pte) || +> > A return pte_write(pte) || > > - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte)); > > + ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_cowed); -> > } +> > A } > This is another case where the naming of pte_*() is biting us vs. the -> perversion of the PTE bits. The lack of comments and explanation inthe +> perversion of the PTE bits.A A The lack of comments and explanation inthe > patch is compounding the confusion. > > We need to find a way to differentiate "someone can write to this PTE" > from "the write bit is set in this PTE". > > In this particular hunk, we need to make it clear that pte_write() is -> *never* true for shadowstack PTEs. In other words, shadow stack VMAs +> *never* true for shadowstack PTEs.A A In other words, shadow stack VMAs > will (should?) never even *see* a pte_write() PTE. > > I think this is a case where you just need to bite the bullet and -> bifurcate can_follow_write_pte(). Just separate the shadowstack and +> bifurcate can_follow_write_pte().A A Just separate the shadowstack and > non-shadowstack parts. In case I don't understand the exact issue. @@ -40,22 +40,22 @@ index fc5f98069f4e..45a0837b27f9 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -70,6 +70,12 @@ static inline bool can_follow_write_pte(pte_t pte, unsigned int flags) - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte)); - } - +A ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte)); +A } +A +static inline bool can_follow_write_shstk_pte(pte_t pte, unsigned int flags) +{ + return ((flags & FOLL_FORCE) && (flags & FOLL_COW) && + is_shstk_pte(pte)); +} + - static struct page *follow_page_pte(struct vm_area_struct *vma, - unsigned long address, pmd_t *pmd, unsigned int flags) - { +A static struct page *follow_page_pte(struct vm_area_struct *vma, +A unsigned long address, pmd_t *pmd, unsigned int flags) +A { @@ -105,9 +111,16 @@ static struct page *follow_page_pte(struct vm_area_struct *vma, - } - if ((flags & FOLL_NUMA) && pte_protnone(pte)) - goto no_page; +A } +A if ((flags & FOLL_NUMA) && pte_protnone(pte)) +A goto no_page; - if ((flags & FOLL_WRITE) && !can_follow_write_pte(pte, flags)) { - pte_unmap_unlock(ptep, ptl); - return NULL; @@ -69,6 +69,6 @@ index fc5f98069f4e..45a0837b27f9 100644 + pte_unmap_unlock(ptep, ptl); + return NULL; + } - } - - page = vm_normal_page(vma, address, pte); +A } +A +A page = vm_normal_page(vma, address, pte); diff --git a/a/content_digest b/N3/content_digest index ec31806..3b15ac9 100644 --- a/a/content_digest +++ b/N3/content_digest @@ -28,41 +28,44 @@ Mike Kravetz <mike.kravetz@oracle.com> Nadav Amit <nadav.amit@gmail.com> Oleg Nesterov <oleg@redhat.com> - " Pavel Machek <pavel@ucw.cz>Peter\0" + Pavel Machek <pavel@ucw.cz> + Peter Zijlstra <peterz@infradead.org> + Ravi V. Shankar <ravi.v.shankar@intel.com> + " Vedvyas Shanbhogue <vedvyas.shanbhogue@intel.com>\0" "\00:1\0" "b\0" "On Fri, 2018-07-13 at 11:26 -0700, Dave Hansen wrote:\n" "> On 07/11/2018 10:05 AM, Yu-cheng Yu wrote:\n" "> > \n" "> > My understanding is that we don't want to follow write pte if the page\n" - "> > is shared as read-only. \302\240For a SHSTK page, that is (R/O + DIRTY_SW),\n" - "> > which means the SHSTK page has not been COW'ed. \302\240Is that right?\n" + "> > is shared as read-only. A For a SHSTK page, that is (R/O + DIRTY_SW),\n" + "> > which means the SHSTK page has not been COW'ed. A Is that right?\n" "> Let's look at the code again:\n" "> \n" "> > \n" "> > -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)\n" "> > +static inline bool can_follow_write_pte(pte_t pte, unsigned int flags,\n" "> > +\t\t\t\t\tbool shstk)\n" - "> > \302\240{\n" + "> > A {\n" "> > +\tbool pte_cowed = shstk ? is_shstk_pte(pte):pte_dirty(pte);\n" "> > +\n" - "> > \302\240\treturn pte_write(pte) ||\n" + "> > A \treturn pte_write(pte) ||\n" "> > -\t\t((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));\n" "> > +\t\t((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_cowed);\n" - "> > \302\240}\n" + "> > A }\n" "> This is another case where the naming of pte_*() is biting us vs. the\n" - "> perversion of the PTE bits.\302\240\302\240The lack of comments and explanation inthe\n" + "> perversion of the PTE bits.A A The lack of comments and explanation inthe\n" "> patch is compounding the confusion.\n" "> \n" "> We need to find a way to differentiate \"someone can write to this PTE\"\n" "> from \"the write bit is set in this PTE\".\n" "> \n" "> In this particular hunk, we need to make it clear that pte_write() is\n" - "> *never* true for shadowstack PTEs.\302\240\302\240In other words, shadow stack VMAs\n" + "> *never* true for shadowstack PTEs.A A In other words, shadow stack VMAs\n" "> will (should?) never even *see* a pte_write() PTE.\n" "> \n" "> I think this is a case where you just need to bite the bullet and\n" - "> bifurcate can_follow_write_pte().\302\240\302\240Just separate the shadowstack and\n" + "> bifurcate can_follow_write_pte().A A Just separate the shadowstack and\n" "> non-shadowstack parts.\n" "\n" "In case I don't understand the exact issue.\n" @@ -73,22 +76,22 @@ "--- a/mm/gup.c\n" "+++ b/mm/gup.c\n" "@@ -70,6 +70,12 @@ static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)\n" - "\302\240\t\t((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));\n" - "\302\240}\n" - "\302\240\n" + "A \t\t((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));\n" + "A }\n" + "A \n" "+static inline bool can_follow_write_shstk_pte(pte_t pte, unsigned int flags)\n" "+{\n" "+\treturn ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&\n" "+\t\tis_shstk_pte(pte));\n" "+}\n" "+\n" - "\302\240static struct page *follow_page_pte(struct vm_area_struct *vma,\n" - "\302\240\t\tunsigned long address, pmd_t *pmd, unsigned int flags)\n" - "\302\240{\n" + "A static struct page *follow_page_pte(struct vm_area_struct *vma,\n" + "A \t\tunsigned long address, pmd_t *pmd, unsigned int flags)\n" + "A {\n" "@@ -105,9 +111,16 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,\n" - "\302\240\t}\n" - "\302\240\tif ((flags & FOLL_NUMA) && pte_protnone(pte))\n" - "\302\240\t\tgoto no_page;\n" + "A \t}\n" + "A \tif ((flags & FOLL_NUMA) && pte_protnone(pte))\n" + "A \t\tgoto no_page;\n" "-\tif ((flags & FOLL_WRITE) && !can_follow_write_pte(pte, flags)) {\n" "-\t\tpte_unmap_unlock(ptep, ptl);\n" "-\t\treturn NULL;\n" @@ -102,8 +105,8 @@ "+\t\t\tpte_unmap_unlock(ptep, ptl);\n" "+\t\t\treturn NULL;\n" "+\t\t}\n" - "\302\240\t}\n" - "\302\240\n" - "\302\240\tpage = vm_normal_page(vma, address, pte);" + "A \t}\n" + "A \n" + "A \tpage = vm_normal_page(vma, address, pte);" -31ed5e5878d3bb220e71c19ccff9bf46e851fa7a9a531a9421fff74bdbdc31b2 +0653f7222b4a97e25013f3893983f2b27514aa0794e03a8738c729361ee30634
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.