Message ID | 20220912014703.185471-2-rmclure@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [1/3] powerpc: mm: move pud_pfn stub to common pgtable header | expand |
Le 12/09/2022 à 03:47, Rohan McLure a écrit : > Add the following helpers for detecting whether a page table entry > is a leaf and is accessible to user space. > > * pte_user_accessible_page > * pmd_user_accessible_page > * pud_user_accessible_page > > The heavy lifting is done by pte_user, which checks user accessibility > on a per-mmu level, provided prior to this commit. Not in this series it seems, so I guess that commit is already in the tree. Can you reference it ? > > On 32-bit systems, provide stub implementations for these methods, with > BUG(), as debug features such as page table checks will emit functions > that call p{md,ud}_user_accessible_page but must not be used. Please please please no new BUG or BUG_ON, please see the following discussion and especially the position of Linus Torvalds : https://lore.kernel.org/all/CAHk-=wg40EAZofO16Eviaj7mfqDhZ2gVEbvfsMf6gYzspRjYvw@mail.gmail.com/ > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > arch/powerpc/include/asm/pgtable.h | 35 ++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h > index 522145b16a07..8c1f5feb9360 100644 > --- a/arch/powerpc/include/asm/pgtable.h > +++ b/arch/powerpc/include/asm/pgtable.h > @@ -170,6 +170,41 @@ static inline int pud_pfn(pud_t pud) > return 0; > } > > +static inline bool pte_user_accessible_page(pte_t pte) > +{ > + return (pte_val(pte) & _PAGE_PRESENT) && pte_user(pte); > +} > + > +#ifdef CONFIG_PPC64 > + > +static inline bool pmd_user_accessible_page(pmd_t pmd) > +{ > + return pmd_is_leaf(pmd) && pmd_present(pmd) > + && pte_user(pmd_pte(pmd)); The && goes on previous line. > +} > + > +static inline bool pud_user_accessible_page(pud_t pud) > +{ > + return pud_is_leaf(pud) && pud_present(pud) > + && pte_user(pud_pte(pud)); Same > +} > + > +#else > + > +static inline bool pmd_user_accessible_page(pmd_t pmd) > +{ > + BUG(); Can you use BUILD_BUG() instead ? > + return false; > +} > + > +static inline bool pud_user_accessible_page(pud_t pud) > +{ > + BUG(); Same. > + return false; > +} > + > +#endif /* CONFIG_PPC64 */ > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_PGTABLE_H */ By the way, there is a great tool that can help you : $ ./arch/powerpc/tools/checkpatch.sh --strict -g HEAD~ CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the previous line #43: FILE: arch/powerpc/include/asm/pgtable.h:183: + return pmd_is_leaf(pmd) && pmd_present(pmd) + && pte_user(pmd_pte(pmd)); CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the previous line #49: FILE: arch/powerpc/include/asm/pgtable.h:189: + return pud_is_leaf(pud) && pud_present(pud) + && pte_user(pud_pte(pud)); WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() #56: FILE: arch/powerpc/include/asm/pgtable.h:196: + BUG(); WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() #62: FILE: arch/powerpc/include/asm/pgtable.h:202: + BUG();
>> +static inline bool pmd_user_accessible_page(pmd_t pmd) >> +{ >> + return pmd_is_leaf(pmd) && pmd_present(pmd) >> + && pte_user(pmd_pte(pmd)); > > The && goes on previous line. > By the way, there is a great tool that can help you : > > $ ./arch/powerpc/tools/checkpatch.sh --strict -g HEAD~ Thank you. Yes I should be using --strict with checkpatch. > >> +} >> + >> +#else >> + >> +static inline bool pmd_user_accessible_page(pmd_t pmd) >> +{ >> + BUG(); > > Can you use BUILD_BUG() instead ? As it stands, p{m,u}d_user_accessible page is invoked by __page_table_check_p{m,u}d_{set,clear}. So unfortunately this function will have to be compiled, but its body could be replaced with a WARN() and return false on 32-bit.
Le 12/09/2022 à 03:47, Rohan McLure a écrit : > Add the following helpers for detecting whether a page table entry > is a leaf and is accessible to user space. > > * pte_user_accessible_page > * pmd_user_accessible_page > * pud_user_accessible_page > > The heavy lifting is done by pte_user, which checks user accessibility > on a per-mmu level, provided prior to this commit. > > On 32-bit systems, provide stub implementations for these methods, with > BUG(), as debug features such as page table checks will emit functions > that call p{md,ud}_user_accessible_page but must not be used. Why "must not be used" ? > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > arch/powerpc/include/asm/pgtable.h | 35 ++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h > index 522145b16a07..8c1f5feb9360 100644 > --- a/arch/powerpc/include/asm/pgtable.h > +++ b/arch/powerpc/include/asm/pgtable.h > @@ -170,6 +170,41 @@ static inline int pud_pfn(pud_t pud) > return 0; > } > > +static inline bool pte_user_accessible_page(pte_t pte) > +{ > + return (pte_val(pte) & _PAGE_PRESENT) && pte_user(pte); Should use pte_present() instead ? > +} > + > +#ifdef CONFIG_PPC64 I think the functions could be valid for PPC32 as well with below changes: > + > +static inline bool pmd_user_accessible_page(pmd_t pmd) > +{ > + return pmd_is_leaf(pmd) && pmd_present(pmd) > + && pte_user(pmd_pte(pmd)); Define and use pmd_user() > +} > + > +static inline bool pud_user_accessible_page(pud_t pud) > +{ > + return pud_is_leaf(pud) && pud_present(pud) > + && pte_user(pud_pte(pud)); Use pud_user(). For ppc64 you have to define it in arch/powerpc/include/asm/book3s/64/pgtable.h and arch/powerpc/include/asm/nohash/64/pgtable.h For ppc32 it is already defined in include/asm-generic/pgtable-nopmd.h And use pud_leaf() instead of pud_is_leaf(). For ppc32 it is already defined in include/asm-generic/pgtable-nopmd.h > +} > + > +#else > + > +static inline bool pmd_user_accessible_page(pmd_t pmd) > +{ > + BUG(); > + return false; > +} > + > +static inline bool pud_user_accessible_page(pud_t pud) > +{ > + BUG(); > + return false; > +} > + > +#endif /* CONFIG_PPC64 */ > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_PGTABLE_H */
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h index 522145b16a07..8c1f5feb9360 100644 --- a/arch/powerpc/include/asm/pgtable.h +++ b/arch/powerpc/include/asm/pgtable.h @@ -170,6 +170,41 @@ static inline int pud_pfn(pud_t pud) return 0; } +static inline bool pte_user_accessible_page(pte_t pte) +{ + return (pte_val(pte) & _PAGE_PRESENT) && pte_user(pte); +} + +#ifdef CONFIG_PPC64 + +static inline bool pmd_user_accessible_page(pmd_t pmd) +{ + return pmd_is_leaf(pmd) && pmd_present(pmd) + && pte_user(pmd_pte(pmd)); +} + +static inline bool pud_user_accessible_page(pud_t pud) +{ + return pud_is_leaf(pud) && pud_present(pud) + && pte_user(pud_pte(pud)); +} + +#else + +static inline bool pmd_user_accessible_page(pmd_t pmd) +{ + BUG(); + return false; +} + +static inline bool pud_user_accessible_page(pud_t pud) +{ + BUG(); + return false; +} + +#endif /* CONFIG_PPC64 */ + #endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_PGTABLE_H */
Add the following helpers for detecting whether a page table entry is a leaf and is accessible to user space. * pte_user_accessible_page * pmd_user_accessible_page * pud_user_accessible_page The heavy lifting is done by pte_user, which checks user accessibility on a per-mmu level, provided prior to this commit. On 32-bit systems, provide stub implementations for these methods, with BUG(), as debug features such as page table checks will emit functions that call p{md,ud}_user_accessible_page but must not be used. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> --- arch/powerpc/include/asm/pgtable.h | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)