Re: [PATCH v7 13/41] mm: Make pte_mkwrite() take a VMA

2023-03-02 Thread Borislav Petkov
On Mon, Feb 27, 2023 at 02:29:29PM -0800, Rick Edgecombe wrote:
> [0] 
> https://lore.kernel.org/lkml/0e29a2d0-08d8-bcd6-ff26-4bea0e403...@redhat.com/#t

I guess that sub-thread about how you arrived at this "pass a VMA"
decision should be in the Link tag. But that's for the committer, I'd
say.

Thx.

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette



Re: [PATCH v7 13/41] mm: Make pte_mkwrite() take a VMA

2023-03-01 Thread David Hildenbrand

On 01.03.23 08:03, Christophe Leroy wrote:



Le 27/02/2023 à 23:29, Rick Edgecombe a écrit :

The x86 Control-flow Enforcement Technology (CET) feature includes a new
type of memory called shadow stack. This shadow stack memory has some
unusual properties, which requires some core mm changes to function
properly.

One of these unusual properties is that shadow stack memory is writable,
but only in limited ways. These limits are applied via a specific PTE
bit combination. Nevertheless, the memory is writable, and core mm code
will need to apply the writable permissions in the typical paths that
call pte_mkwrite().

In addition to VM_WRITE, the shadow stack VMA's will have a flag denoting
that they are special shadow stack flavor of writable memory. So make
pte_mkwrite() take a VMA, so that the x86 implementation of it can know to
create regular writable memory or shadow stack memory.

Apply the same changes for pmd_mkwrite() and huge_pte_mkwrite().


I'm not sure it is a good idea to add a second argument to
pte_mkwrite(). All pte_mk() only take a pte and nothing else.


We touched on this in previous revisions and so far there was no strong 
push back. This turned out to be cleaner and easier than the 
alternatives we evaluated.


pte_modify(), for example, takes another argument. Sure, we could try 
thinking about passing something else than a VMA to identify the 
writability type, but I am not convinced that will look particularly better.




I think you should do the same as commit d9ed9faac283 ("mm: add new
arch_make_huge_pte() method for tile support")



We already have 3 architectures intending to support shadow stacks in 
one way or the other. Replacing all pte_mkwrite() with 
arch_pte_mkwrite() doesn't sound particularly appealing to me.



--
Thanks,

David / dhildenb




Re: [PATCH v7 13/41] mm: Make pte_mkwrite() take a VMA

2023-02-28 Thread Christophe Leroy


Le 27/02/2023 à 23:29, Rick Edgecombe a écrit :
> The x86 Control-flow Enforcement Technology (CET) feature includes a new
> type of memory called shadow stack. This shadow stack memory has some
> unusual properties, which requires some core mm changes to function
> properly.
> 
> One of these unusual properties is that shadow stack memory is writable,
> but only in limited ways. These limits are applied via a specific PTE
> bit combination. Nevertheless, the memory is writable, and core mm code
> will need to apply the writable permissions in the typical paths that
> call pte_mkwrite().
> 
> In addition to VM_WRITE, the shadow stack VMA's will have a flag denoting
> that they are special shadow stack flavor of writable memory. So make
> pte_mkwrite() take a VMA, so that the x86 implementation of it can know to
> create regular writable memory or shadow stack memory.
> 
> Apply the same changes for pmd_mkwrite() and huge_pte_mkwrite().

I'm not sure it is a good idea to add a second argument to 
pte_mkwrite(). All pte_mk() only take a pte and nothing else.

I think you should do the same as commit d9ed9faac283 ("mm: add new 
arch_make_huge_pte() method for tile support")

Christophe

> 
> No functional change.
> 
> Cc: linux-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-al...@vger.kernel.org
> Cc: linux-snps-...@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-c...@vger.kernel.org
> Cc: linux-hexa...@vger.kernel.org
> Cc: linux-i...@vger.kernel.org
> Cc: loonga...@lists.linux.dev
> Cc: linux-m...@lists.linux-m68k.org
> Cc: Michal Simek 
> Cc: Dinh Nguyen 
> Cc: linux-m...@vger.kernel.org
> Cc: linux-openr...@vger.kernel.org
> Cc: linux-par...@vger.kernel.org
> Cc: linuxppc-...@lists.ozlabs.org
> Cc: linux-ri...@lists.infradead.org
> Cc: linux-s...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: sparcli...@vger.kernel.org
> Cc: linux...@lists.infradead.org
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-a...@vger.kernel.org
> Cc: linux...@kvack.org
> Tested-by: Pengfei Xu 
> Tested-by: John Allen 
> Tested-by: Kees Cook 
> Acked-by: Mike Rapoport (IBM) 
> Acked-by: Michael Ellerman 
> Acked-by: David Hildenbrand 
> Reviewed-by: Kees Cook 
> Suggested-by: David Hildenbrand 
> Signed-off-by: Rick Edgecombe 
> 
> ---
> Hi Non-x86 Arch’s,
> 
> x86 has a feature that allows for the creation of a special type of
> writable memory (shadow stack) that is only writable in limited specific
> ways. Previously, changes were proposed to core MM code to teach it to
> decide when to create normally writable memory or the special shadow stack
> writable memory, but David Hildenbrand suggested[0] to change
> pXX_mkwrite() to take a VMA, so awareness of shadow stack memory can be
> moved into x86 code.
> 
> Since pXX_mkwrite() is defined in every arch, it requires some tree-wide
> changes. So that is why you are seeing some patches out of a big x86
> series pop up in your arch mailing list. There is no functional change.
> After this refactor, the shadow stack series goes on to use the arch
> helpers to push shadow stack memory details inside arch/x86.
> 
> Testing was just 0-day build testing.
> 
> Hopefully that is enough context. Thanks!
> 
> [0] 
> https://lore.kernel.org/lkml/0e29a2d0-08d8-bcd6-ff26-4bea0e403...@redhat.com/#t
> 
> v6:
>   - New patch
> ---
>   Documentation/mm/arch_pgtable_helpers.rst|  9 ++---
>   arch/alpha/include/asm/pgtable.h |  6 +-
>   arch/arc/include/asm/hugepage.h  |  2 +-
>   arch/arc/include/asm/pgtable-bits-arcv2.h|  7 ++-
>   arch/arm/include/asm/pgtable-3level.h|  7 ++-
>   arch/arm/include/asm/pgtable.h   |  2 +-
>   arch/arm64/include/asm/pgtable.h |  4 ++--
>   arch/csky/include/asm/pgtable.h  |  2 +-
>   arch/hexagon/include/asm/pgtable.h   |  2 +-
>   arch/ia64/include/asm/pgtable.h  |  2 +-
>   arch/loongarch/include/asm/pgtable.h |  4 ++--
>   arch/m68k/include/asm/mcf_pgtable.h  |  2 +-
>   arch/m68k/include/asm/motorola_pgtable.h |  6 +-
>   arch/m68k/include/asm/sun3_pgtable.h |  6 +-
>   arch/microblaze/include/asm/pgtable.h|  2 +-
>   arch/mips/include/asm/pgtable.h  |  6 +++---
>   arch/nios2/include/asm/pgtable.h |  2 +-
>   arch/openrisc/include/asm/pgtable.h  |  2 +-
>   arch/parisc/include/asm/pgtable.h|  6 +-
>   arch/powerpc/include/asm/book3s/32/pgtable.h |  2 +-
>   arch/powerpc/include/asm/book3s/64/pgtable.h |  4 ++--
>   arch/powerpc/include/asm/nohash/32/pgtable.h |  2 +-
>   arch/powerpc/include/asm/nohash/32/pte-8xx.h |  2 +-
>   arch/powerpc/include/asm/nohash/64/pgtable.h |  2 +-
>   arch/riscv/include/asm/pgtable.h |  6 +++---
>   arch/s390/include/asm/hugetlb.h  |  4 ++--
>   arch/s390/include/asm/pgtable.h  |  4 ++--
>   arch/sh/include/asm/pgtable_32.h   

[PATCH v7 13/41] mm: Make pte_mkwrite() take a VMA

2023-02-27 Thread Rick Edgecombe
The x86 Control-flow Enforcement Technology (CET) feature includes a new
type of memory called shadow stack. This shadow stack memory has some
unusual properties, which requires some core mm changes to function
properly.

One of these unusual properties is that shadow stack memory is writable,
but only in limited ways. These limits are applied via a specific PTE
bit combination. Nevertheless, the memory is writable, and core mm code
will need to apply the writable permissions in the typical paths that
call pte_mkwrite().

In addition to VM_WRITE, the shadow stack VMA's will have a flag denoting
that they are special shadow stack flavor of writable memory. So make
pte_mkwrite() take a VMA, so that the x86 implementation of it can know to
create regular writable memory or shadow stack memory.

Apply the same changes for pmd_mkwrite() and huge_pte_mkwrite().

No functional change.

Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-al...@vger.kernel.org
Cc: linux-snps-...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-c...@vger.kernel.org
Cc: linux-hexa...@vger.kernel.org
Cc: linux-i...@vger.kernel.org
Cc: loonga...@lists.linux.dev
Cc: linux-m...@lists.linux-m68k.org
Cc: Michal Simek 
Cc: Dinh Nguyen 
Cc: linux-m...@vger.kernel.org
Cc: linux-openr...@vger.kernel.org
Cc: linux-par...@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org
Cc: linux-ri...@lists.infradead.org
Cc: linux-s...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: sparcli...@vger.kernel.org
Cc: linux...@lists.infradead.org
Cc: xen-devel@lists.xenproject.org
Cc: linux-a...@vger.kernel.org
Cc: linux...@kvack.org
Tested-by: Pengfei Xu 
Tested-by: John Allen 
Tested-by: Kees Cook 
Acked-by: Mike Rapoport (IBM) 
Acked-by: Michael Ellerman 
Acked-by: David Hildenbrand 
Reviewed-by: Kees Cook 
Suggested-by: David Hildenbrand 
Signed-off-by: Rick Edgecombe 

---
Hi Non-x86 Arch’s,

x86 has a feature that allows for the creation of a special type of
writable memory (shadow stack) that is only writable in limited specific
ways. Previously, changes were proposed to core MM code to teach it to
decide when to create normally writable memory or the special shadow stack
writable memory, but David Hildenbrand suggested[0] to change
pXX_mkwrite() to take a VMA, so awareness of shadow stack memory can be
moved into x86 code.

Since pXX_mkwrite() is defined in every arch, it requires some tree-wide
changes. So that is why you are seeing some patches out of a big x86
series pop up in your arch mailing list. There is no functional change.
After this refactor, the shadow stack series goes on to use the arch
helpers to push shadow stack memory details inside arch/x86.

Testing was just 0-day build testing.

Hopefully that is enough context. Thanks!

[0] 
https://lore.kernel.org/lkml/0e29a2d0-08d8-bcd6-ff26-4bea0e403...@redhat.com/#t

v6:
 - New patch
---
 Documentation/mm/arch_pgtable_helpers.rst|  9 ++---
 arch/alpha/include/asm/pgtable.h |  6 +-
 arch/arc/include/asm/hugepage.h  |  2 +-
 arch/arc/include/asm/pgtable-bits-arcv2.h|  7 ++-
 arch/arm/include/asm/pgtable-3level.h|  7 ++-
 arch/arm/include/asm/pgtable.h   |  2 +-
 arch/arm64/include/asm/pgtable.h |  4 ++--
 arch/csky/include/asm/pgtable.h  |  2 +-
 arch/hexagon/include/asm/pgtable.h   |  2 +-
 arch/ia64/include/asm/pgtable.h  |  2 +-
 arch/loongarch/include/asm/pgtable.h |  4 ++--
 arch/m68k/include/asm/mcf_pgtable.h  |  2 +-
 arch/m68k/include/asm/motorola_pgtable.h |  6 +-
 arch/m68k/include/asm/sun3_pgtable.h |  6 +-
 arch/microblaze/include/asm/pgtable.h|  2 +-
 arch/mips/include/asm/pgtable.h  |  6 +++---
 arch/nios2/include/asm/pgtable.h |  2 +-
 arch/openrisc/include/asm/pgtable.h  |  2 +-
 arch/parisc/include/asm/pgtable.h|  6 +-
 arch/powerpc/include/asm/book3s/32/pgtable.h |  2 +-
 arch/powerpc/include/asm/book3s/64/pgtable.h |  4 ++--
 arch/powerpc/include/asm/nohash/32/pgtable.h |  2 +-
 arch/powerpc/include/asm/nohash/32/pte-8xx.h |  2 +-
 arch/powerpc/include/asm/nohash/64/pgtable.h |  2 +-
 arch/riscv/include/asm/pgtable.h |  6 +++---
 arch/s390/include/asm/hugetlb.h  |  4 ++--
 arch/s390/include/asm/pgtable.h  |  4 ++--
 arch/sh/include/asm/pgtable_32.h | 10 --
 arch/sparc/include/asm/pgtable_32.h  |  2 +-
 arch/sparc/include/asm/pgtable_64.h  |  6 +++---
 arch/um/include/asm/pgtable.h|  2 +-
 arch/x86/include/asm/pgtable.h   |  6 --
 arch/xtensa/include/asm/pgtable.h|  2 +-
 include/asm-generic/hugetlb.h|  4 ++--
 include/linux/mm.h   |  2 +-
 mm/debug_vm_pgtable.c| 16 
 mm/huge_memory.c |  6 +++---
 mm/hugetlb.c