| Message ID | 20240328045535.194800-12-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support page table check PowerPC | expand |
Le 28/03/2024 à 05:55, Rohan McLure a écrit : > Page table checking depends on architectures providing an > implementation of p{te,md,ud}_user_accessible_page. With > refactorisations made on powerpc/mm, the pte_access_permitted() and > similar methods verify whether a userland page is accessible with the > required permissions. > > Since page table checking is the only user of > p{te,md,ud}_user_accessible_page(), implement these for all platforms, > using some of the same preliminary checks taken by pte_access_permitted() > on that platform. > > Since Commit 8e9bd41e4ce1 ("powerpc/nohash: Replace pte_user() by pte_read()") > pte_user() is no longer required to be present on all platforms as it > may be equivalent to or implied by pte_read(). Hence implementations of > pte_user_accessible_page() are specialised. > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > v9: New implementation > v10: Let book3s/64 use pte_user(), but otherwise default other platforms > to using the address provided with the call to infer whether it is a > user page or not. pmd/pud variants will warn on all other platforms, as > they should not be used for user page mappings > v11: Conditionally define p{m,u}d_user_accessible_page(), as not all > platforms have p{m,u}d_leaf(), p{m,u}d_pte() stubs. See my comment to v10 patch 10. p{m,u}d_leaf() is defined for all platforms (There is a fallback definition in include/linux/pgtable.h) so p{m,u}d_user_accessible_page() can be defined for all platforms, no need for a conditionally define. > --- > arch/powerpc/include/asm/book3s/32/pgtable.h | 5 +++++ > arch/powerpc/include/asm/book3s/64/pgtable.h | 17 +++++++++++++++++ > arch/powerpc/include/asm/nohash/pgtable.h | 5 +++++ > arch/powerpc/include/asm/pgtable.h | 8 ++++++++ > 4 files changed, 35 insertions(+) > > diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h > index 52971ee30717..83f7b98ef49f 100644 > --- a/arch/powerpc/include/asm/book3s/32/pgtable.h > +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h > @@ -436,6 +436,11 @@ static inline bool pte_access_permitted(pte_t pte, bool write) > return true; > } > > +static inline bool pte_user_accessible_page(pte_t pte, unsigned long addr) > +{ > + return pte_present(pte) && !is_kernel_addr(addr); > +} > + > /* Conversion functions: convert a page and protection to a page entry, > * and a page entry and page directory to the page they refer to. > * > diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h > index fac5615e6bc5..d8640ddbcad1 100644 > --- a/arch/powerpc/include/asm/book3s/64/pgtable.h > +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h > @@ -538,6 +538,11 @@ static inline bool pte_access_permitted(pte_t pte, bool write) > return arch_pte_access_permitted(pte_val(pte), write, 0); > } > > +static inline bool pte_user_accessible_page(pte_t pte, unsigned long addr) > +{ > + return pte_present(pte) && pte_user(pte); > +} > + > /* > * Conversion functions: convert a page and protection to a page entry, > * and a page entry and page directory to the page they refer to. > @@ -1441,5 +1446,17 @@ static inline bool pud_leaf(pud_t pud) > return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PTE)); > } > > +#define pmd_user_accessible_page pmd_user_accessible_page > +static inline bool pmd_user_accessible_page(pmd_t pmd, unsigned long addr) > +{ > + return pmd_leaf(pmd) && pte_user_accessible_page(pmd_pte(pmd), addr); > +} > + > +#define pud_user_accessible_page pud_user_accessible_page > +static inline bool pud_user_accessible_page(pud_t pud, unsigned long addr) > +{ > + return pud_leaf(pud) && pte_user_accessible_page(pud_pte(pud), addr); > +} > + > #endif /* __ASSEMBLY__ */ > #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */ > diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h > index 427db14292c9..413d01a51e6f 100644 > --- a/arch/powerpc/include/asm/nohash/pgtable.h > +++ b/arch/powerpc/include/asm/nohash/pgtable.h > @@ -213,6 +213,11 @@ static inline bool pte_access_permitted(pte_t pte, bool write) > return true; > } > > +static inline bool pte_user_accessible_page(pte_t pte, unsigned long addr) > +{ > + return pte_present(pte) && !is_kernel_addr(addr); > +} > + > /* Conversion functions: convert a page and protection to a page entry, > * and a page entry and page directory to the page they refer to. > * > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h > index ee8c82c0528f..f1ceae778cb1 100644 > --- a/arch/powerpc/include/asm/pgtable.h > +++ b/arch/powerpc/include/asm/pgtable.h > @@ -219,6 +219,14 @@ static inline int pud_pfn(pud_t pud) > } > #endif > > +#ifndef pmd_user_accessible_page > +#define pmd_user_accessible_page(pmd, addr) false > +#endif > + > +#ifndef pud_user_accessible_page > +#define pud_user_accessible_page(pud, addr) false > +#endif > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_PGTABLE_H */
On Thu, 2024-03-28 at 05:40 +0000, Christophe Leroy wrote: > > > Le 28/03/2024 à 05:55, Rohan McLure a écrit : > > Page table checking depends on architectures providing an > > implementation of p{te,md,ud}_user_accessible_page. With > > refactorisations made on powerpc/mm, the pte_access_permitted() and > > similar methods verify whether a userland page is accessible with > > the > > required permissions. > > > > Since page table checking is the only user of > > p{te,md,ud}_user_accessible_page(), implement these for all > > platforms, > > using some of the same preliminary checks taken by > > pte_access_permitted() > > on that platform. > > > > Since Commit 8e9bd41e4ce1 ("powerpc/nohash: Replace pte_user() by > > pte_read()") > > pte_user() is no longer required to be present on all platforms as > > it > > may be equivalent to or implied by pte_read(). Hence > > implementations of > > pte_user_accessible_page() are specialised. > > > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > > --- > > v9: New implementation > > v10: Let book3s/64 use pte_user(), but otherwise default other > > platforms > > to using the address provided with the call to infer whether it is > > a > > user page or not. pmd/pud variants will warn on all other > > platforms, as > > they should not be used for user page mappings > > v11: Conditionally define p{m,u}d_user_accessible_page(), as not > > all > > platforms have p{m,u}d_leaf(), p{m,u}d_pte() stubs. > > See my comment to v10 patch 10. > > p{m,u}d_leaf() is defined for all platforms (There is a fallback > definition in include/linux/pgtable.h) so > p{m,u}d_user_accessible_page() > can be defined for all platforms, no need for a conditionally define. The issue I see is that the definition in include/linux/pgtable.h occurs after this header is included. Prior to the removal of a local definition of p{m,u}d_leaf() etc we didn't run into this issue, but we still do now. Not insistent on doing it this way with ifndef, so amenable to suggestions if you have a preference. > > > --- > > arch/powerpc/include/asm/book3s/32/pgtable.h | 5 +++++ > > arch/powerpc/include/asm/book3s/64/pgtable.h | 17 > > +++++++++++++++++ > > arch/powerpc/include/asm/nohash/pgtable.h | 5 +++++ > > arch/powerpc/include/asm/pgtable.h | 8 ++++++++ > > 4 files changed, 35 insertions(+) > > > > diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h > > b/arch/powerpc/include/asm/book3s/32/pgtable.h > > index 52971ee30717..83f7b98ef49f 100644 > > --- a/arch/powerpc/include/asm/book3s/32/pgtable.h > > +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h > > @@ -436,6 +436,11 @@ static inline bool pte_access_permitted(pte_t > > pte, bool write) > > return true; > > } > > > > +static inline bool pte_user_accessible_page(pte_t pte, unsigned > > long addr) > > +{ > > + return pte_present(pte) && !is_kernel_addr(addr); > > +} > > + > > /* Conversion functions: convert a page and protection to a page > > entry, > > * and a page entry and page directory to the page they refer to. > > * > > diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h > > b/arch/powerpc/include/asm/book3s/64/pgtable.h > > index fac5615e6bc5..d8640ddbcad1 100644 > > --- a/arch/powerpc/include/asm/book3s/64/pgtable.h > > +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h > > @@ -538,6 +538,11 @@ static inline bool pte_access_permitted(pte_t > > pte, bool write) > > return arch_pte_access_permitted(pte_val(pte), write, 0); > > } > > > > +static inline bool pte_user_accessible_page(pte_t pte, unsigned > > long addr) > > +{ > > + return pte_present(pte) && pte_user(pte); > > +} > > + > > /* > > * Conversion functions: convert a page and protection to a page > > entry, > > * and a page entry and page directory to the page they refer to. > > @@ -1441,5 +1446,17 @@ static inline bool pud_leaf(pud_t pud) > > return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PTE)); > > } > > > > +#define pmd_user_accessible_page pmd_user_accessible_page > > +static inline bool pmd_user_accessible_page(pmd_t pmd, unsigned > > long addr) > > +{ > > + return pmd_leaf(pmd) && > > pte_user_accessible_page(pmd_pte(pmd), addr); > > +} > > + > > +#define pud_user_accessible_page pud_user_accessible_page > > +static inline bool pud_user_accessible_page(pud_t pud, unsigned > > long addr) > > +{ > > + return pud_leaf(pud) && > > pte_user_accessible_page(pud_pte(pud), addr); > > +} > > + > > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */ > > diff --git a/arch/powerpc/include/asm/nohash/pgtable.h > > b/arch/powerpc/include/asm/nohash/pgtable.h > > index 427db14292c9..413d01a51e6f 100644 > > --- a/arch/powerpc/include/asm/nohash/pgtable.h > > +++ b/arch/powerpc/include/asm/nohash/pgtable.h > > @@ -213,6 +213,11 @@ static inline bool pte_access_permitted(pte_t > > pte, bool write) > > return true; > > } > > > > +static inline bool pte_user_accessible_page(pte_t pte, unsigned > > long addr) > > +{ > > + return pte_present(pte) && !is_kernel_addr(addr); > > +} > > + > > /* Conversion functions: convert a page and protection to a page > > entry, > > * and a page entry and page directory to the page they refer to. > > * > > diff --git a/arch/powerpc/include/asm/pgtable.h > > b/arch/powerpc/include/asm/pgtable.h > > index ee8c82c0528f..f1ceae778cb1 100644 > > --- a/arch/powerpc/include/asm/pgtable.h > > +++ b/arch/powerpc/include/asm/pgtable.h > > @@ -219,6 +219,14 @@ static inline int pud_pfn(pud_t pud) > > } > > #endif > > > > +#ifndef pmd_user_accessible_page > > +#define pmd_user_accessible_page(pmd, addr) false > > +#endif > > + > > +#ifndef pud_user_accessible_page > > +#define pud_user_accessible_page(pud, addr) false > > +#endif > > + > > #endif /* __ASSEMBLY__ */ > > > > #endif /* _ASM_POWERPC_PGTABLE_H */
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h index 52971ee30717..83f7b98ef49f 100644 --- a/arch/powerpc/include/asm/book3s/32/pgtable.h +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h @@ -436,6 +436,11 @@ static inline bool pte_access_permitted(pte_t pte, bool write) return true; } +static inline bool pte_user_accessible_page(pte_t pte, unsigned long addr) +{ + return pte_present(pte) && !is_kernel_addr(addr); +} + /* Conversion functions: convert a page and protection to a page entry, * and a page entry and page directory to the page they refer to. * diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h index fac5615e6bc5..d8640ddbcad1 100644 --- a/arch/powerpc/include/asm/book3s/64/pgtable.h +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h @@ -538,6 +538,11 @@ static inline bool pte_access_permitted(pte_t pte, bool write) return arch_pte_access_permitted(pte_val(pte), write, 0); } +static inline bool pte_user_accessible_page(pte_t pte, unsigned long addr) +{ + return pte_present(pte) && pte_user(pte); +} + /* * Conversion functions: convert a page and protection to a page entry, * and a page entry and page directory to the page they refer to. @@ -1441,5 +1446,17 @@ static inline bool pud_leaf(pud_t pud) return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PTE)); } +#define pmd_user_accessible_page pmd_user_accessible_page +static inline bool pmd_user_accessible_page(pmd_t pmd, unsigned long addr) +{ + return pmd_leaf(pmd) && pte_user_accessible_page(pmd_pte(pmd), addr); +} + +#define pud_user_accessible_page pud_user_accessible_page +static inline bool pud_user_accessible_page(pud_t pud, unsigned long addr) +{ + return pud_leaf(pud) && pte_user_accessible_page(pud_pte(pud), addr); +} + #endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */ diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h index 427db14292c9..413d01a51e6f 100644 --- a/arch/powerpc/include/asm/nohash/pgtable.h +++ b/arch/powerpc/include/asm/nohash/pgtable.h @@ -213,6 +213,11 @@ static inline bool pte_access_permitted(pte_t pte, bool write) return true; } +static inline bool pte_user_accessible_page(pte_t pte, unsigned long addr) +{ + return pte_present(pte) && !is_kernel_addr(addr); +} + /* Conversion functions: convert a page and protection to a page entry, * and a page entry and page directory to the page they refer to. * diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h index ee8c82c0528f..f1ceae778cb1 100644 --- a/arch/powerpc/include/asm/pgtable.h +++ b/arch/powerpc/include/asm/pgtable.h @@ -219,6 +219,14 @@ static inline int pud_pfn(pud_t pud) } #endif +#ifndef pmd_user_accessible_page +#define pmd_user_accessible_page(pmd, addr) false +#endif + +#ifndef pud_user_accessible_page +#define pud_user_accessible_page(pud, addr) false +#endif + #endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_PGTABLE_H */
Page table checking depends on architectures providing an implementation of p{te,md,ud}_user_accessible_page. With refactorisations made on powerpc/mm, the pte_access_permitted() and similar methods verify whether a userland page is accessible with the required permissions. Since page table checking is the only user of p{te,md,ud}_user_accessible_page(), implement these for all platforms, using some of the same preliminary checks taken by pte_access_permitted() on that platform. Since Commit 8e9bd41e4ce1 ("powerpc/nohash: Replace pte_user() by pte_read()") pte_user() is no longer required to be present on all platforms as it may be equivalent to or implied by pte_read(). Hence implementations of pte_user_accessible_page() are specialised. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> --- v9: New implementation v10: Let book3s/64 use pte_user(), but otherwise default other platforms to using the address provided with the call to infer whether it is a user page or not. pmd/pud variants will warn on all other platforms, as they should not be used for user page mappings v11: Conditionally define p{m,u}d_user_accessible_page(), as not all platforms have p{m,u}d_leaf(), p{m,u}d_pte() stubs. --- arch/powerpc/include/asm/book3s/32/pgtable.h | 5 +++++ arch/powerpc/include/asm/book3s/64/pgtable.h | 17 +++++++++++++++++ arch/powerpc/include/asm/nohash/pgtable.h | 5 +++++ arch/powerpc/include/asm/pgtable.h | 8 ++++++++ 4 files changed, 35 insertions(+)