mbox series

[v5,0/5] Implement STRICT_MODULE_RWX for powerpc

Message ID 20191030073111.140493-1-ruscur@russell.cc (mailing list archive)
Headers show
Series Implement STRICT_MODULE_RWX for powerpc | expand

Message

Russell Currey Oct. 30, 2019, 7:31 a.m. UTC
v4 cover letter: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198268.html
v3 cover letter: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198023.html

Changes since v4:
	[1/5]: Addressed review comments from Michael Ellerman (thanks!)
	[4/5]: make ARCH_HAS_STRICT_MODULE_RWX depend on
	       ARCH_HAS_STRICT_KERNEL_RWX to simplify things and avoid
	       STRICT_MODULE_RWX being *on by default* in cases where
	       STRICT_KERNEL_RWX is *unavailable*
	[5/5]: split skiroot_defconfig changes out into its own patch

The whole Kconfig situation is really weird and confusing, I believe the
correct resolution is to change arch/Kconfig but the consequences are so
minor that I don't think it's worth it, especially given that I expect
powerpc to have mandatory strict RWX Soon(tm).

Russell Currey (5):
  powerpc/mm: Implement set_memory() routines
  powerpc/kprobes: Mark newly allocated probes as RO
  powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
  powerpc: Set ARCH_HAS_STRICT_MODULE_RWX
  powerpc/configs: Enable STRICT_MODULE_RWX in skiroot_defconfig

 arch/powerpc/Kconfig                   |  2 +
 arch/powerpc/Kconfig.debug             |  6 +-
 arch/powerpc/configs/skiroot_defconfig |  1 +
 arch/powerpc/include/asm/set_memory.h  | 32 +++++++++++
 arch/powerpc/kernel/kprobes.c          |  3 +
 arch/powerpc/mm/Makefile               |  1 +
 arch/powerpc/mm/pageattr.c             | 77 ++++++++++++++++++++++++++
 arch/powerpc/mm/ptdump/ptdump.c        | 21 ++++++-
 8 files changed, 140 insertions(+), 3 deletions(-)
 create mode 100644 arch/powerpc/include/asm/set_memory.h
 create mode 100644 arch/powerpc/mm/pageattr.c

Comments

Christophe Leroy Oct. 30, 2019, 8:58 a.m. UTC | #1
Le 30/10/2019 à 08:31, Russell Currey a écrit :
> v4 cover letter: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198268.html
> v3 cover letter: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198023.html
> 
> Changes since v4:
> 	[1/5]: Addressed review comments from Michael Ellerman (thanks!)
> 	[4/5]: make ARCH_HAS_STRICT_MODULE_RWX depend on
> 	       ARCH_HAS_STRICT_KERNEL_RWX to simplify things and avoid
> 	       STRICT_MODULE_RWX being *on by default* in cases where
> 	       STRICT_KERNEL_RWX is *unavailable*
> 	[5/5]: split skiroot_defconfig changes out into its own patch
> 
> The whole Kconfig situation is really weird and confusing, I believe the
> correct resolution is to change arch/Kconfig but the consequences are so
> minor that I don't think it's worth it, especially given that I expect
> powerpc to have mandatory strict RWX Soon(tm).

I'm not such strict RWX can be made mandatory due to the impact it has 
on some subarches:
- On the 8xx, unless all areas are 8Mbytes aligned, there is a 
significant overhead on TLB misses. And Aligning everthing to 8M is a 
waste of RAM which is not acceptable on systems having very few RAM.
- On hash book3s32, we are able to map the kernel BATs. With a few 
alignment constraints, we are able to provide STRICT_KERNEL_RWX. But we 
are unable to provide exec protection on page granularity. Only on 
256Mbytes segments. So for modules, we have to have the vmspace X. It is 
also not possible to have a kernel area RO. Only user areas can be made RO.

Christophe

> 
> Russell Currey (5):
>    powerpc/mm: Implement set_memory() routines
>    powerpc/kprobes: Mark newly allocated probes as RO
>    powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
>    powerpc: Set ARCH_HAS_STRICT_MODULE_RWX
>    powerpc/configs: Enable STRICT_MODULE_RWX in skiroot_defconfig
> 
>   arch/powerpc/Kconfig                   |  2 +
>   arch/powerpc/Kconfig.debug             |  6 +-
>   arch/powerpc/configs/skiroot_defconfig |  1 +
>   arch/powerpc/include/asm/set_memory.h  | 32 +++++++++++
>   arch/powerpc/kernel/kprobes.c          |  3 +
>   arch/powerpc/mm/Makefile               |  1 +
>   arch/powerpc/mm/pageattr.c             | 77 ++++++++++++++++++++++++++
>   arch/powerpc/mm/ptdump/ptdump.c        | 21 ++++++-
>   8 files changed, 140 insertions(+), 3 deletions(-)
>   create mode 100644 arch/powerpc/include/asm/set_memory.h
>   create mode 100644 arch/powerpc/mm/pageattr.c
>
Kees Cook Oct. 30, 2019, 6:30 p.m. UTC | #2
On Wed, Oct 30, 2019 at 09:58:19AM +0100, Christophe Leroy wrote:
> 
> 
> Le 30/10/2019 à 08:31, Russell Currey a écrit :
> > v4 cover letter: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198268.html
> > v3 cover letter: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198023.html
> > 
> > Changes since v4:
> > 	[1/5]: Addressed review comments from Michael Ellerman (thanks!)
> > 	[4/5]: make ARCH_HAS_STRICT_MODULE_RWX depend on
> > 	       ARCH_HAS_STRICT_KERNEL_RWX to simplify things and avoid
> > 	       STRICT_MODULE_RWX being *on by default* in cases where
> > 	       STRICT_KERNEL_RWX is *unavailable*
> > 	[5/5]: split skiroot_defconfig changes out into its own patch
> > 
> > The whole Kconfig situation is really weird and confusing, I believe the
> > correct resolution is to change arch/Kconfig but the consequences are so
> > minor that I don't think it's worth it, especially given that I expect
> > powerpc to have mandatory strict RWX Soon(tm).
> 
> I'm not such strict RWX can be made mandatory due to the impact it has on
> some subarches:
> - On the 8xx, unless all areas are 8Mbytes aligned, there is a significant
> overhead on TLB misses. And Aligning everthing to 8M is a waste of RAM which
> is not acceptable on systems having very few RAM.
> - On hash book3s32, we are able to map the kernel BATs. With a few alignment
> constraints, we are able to provide STRICT_KERNEL_RWX. But we are unable to
> provide exec protection on page granularity. Only on 256Mbytes segments. So
> for modules, we have to have the vmspace X. It is also not possible to have
> a kernel area RO. Only user areas can be made RO.

As I understand it, the idea was for it to be mandatory (or at least
default-on) only for the subarches where it wasn't totally insane to
accomplish. :) (I'm not familiar with all the details on the subarchs,
but it sounded like the more modern systems would be the targets for
this?)
Christophe Leroy Oct. 30, 2019, 7:28 p.m. UTC | #3
Le 30/10/2019 à 19:30, Kees Cook a écrit :
> On Wed, Oct 30, 2019 at 09:58:19AM +0100, Christophe Leroy wrote:
>>
>>
>> Le 30/10/2019 à 08:31, Russell Currey a écrit :
>>> v4 cover letter: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198268.html
>>> v3 cover letter: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198023.html
>>>
>>> Changes since v4:
>>> 	[1/5]: Addressed review comments from Michael Ellerman (thanks!)
>>> 	[4/5]: make ARCH_HAS_STRICT_MODULE_RWX depend on
>>> 	       ARCH_HAS_STRICT_KERNEL_RWX to simplify things and avoid
>>> 	       STRICT_MODULE_RWX being *on by default* in cases where
>>> 	       STRICT_KERNEL_RWX is *unavailable*
>>> 	[5/5]: split skiroot_defconfig changes out into its own patch
>>>
>>> The whole Kconfig situation is really weird and confusing, I believe the
>>> correct resolution is to change arch/Kconfig but the consequences are so
>>> minor that I don't think it's worth it, especially given that I expect
>>> powerpc to have mandatory strict RWX Soon(tm).
>>
>> I'm not such strict RWX can be made mandatory due to the impact it has on
>> some subarches:
>> - On the 8xx, unless all areas are 8Mbytes aligned, there is a significant
>> overhead on TLB misses. And Aligning everthing to 8M is a waste of RAM which
>> is not acceptable on systems having very few RAM.
>> - On hash book3s32, we are able to map the kernel BATs. With a few alignment
>> constraints, we are able to provide STRICT_KERNEL_RWX. But we are unable to
>> provide exec protection on page granularity. Only on 256Mbytes segments. So
>> for modules, we have to have the vmspace X. It is also not possible to have
>> a kernel area RO. Only user areas can be made RO.
> 
> As I understand it, the idea was for it to be mandatory (or at least
> default-on) only for the subarches where it wasn't totally insane to
> accomplish. :) (I'm not familiar with all the details on the subarchs,
> but it sounded like the more modern systems would be the targets for
> this?)
> 

Yes I guess so.

I was just afraid by "I expect powerpc to have mandatory strict RWX"

By the way, we have an open issue #223 namely 'Make strict kernel RWX 
the default on 64-bit', so no worry as 32-bit is aside for now.

Christophe
Russell Currey Oct. 31, 2019, 12:09 a.m. UTC | #4
On Wed, 2019-10-30 at 09:58 +0100, Christophe Leroy wrote:
> 
> Le 30/10/2019 à 08:31, Russell Currey a écrit :
> > v4 cover letter: 
> > https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198268.html
> > v3 cover letter: 
> > https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198023.html
> > 
> > Changes since v4:
> > 	[1/5]: Addressed review comments from Michael Ellerman
> > (thanks!)
> > 	[4/5]: make ARCH_HAS_STRICT_MODULE_RWX depend on
> > 	       ARCH_HAS_STRICT_KERNEL_RWX to simplify things and avoid
> > 	       STRICT_MODULE_RWX being *on by default* in cases where
> > 	       STRICT_KERNEL_RWX is *unavailable*
> > 	[5/5]: split skiroot_defconfig changes out into its own patch
> > 
> > The whole Kconfig situation is really weird and confusing, I
> > believe the
> > correct resolution is to change arch/Kconfig but the consequences
> > are so
> > minor that I don't think it's worth it, especially given that I
> > expect
> > powerpc to have mandatory strict RWX Soon(tm).
> 
> I'm not such strict RWX can be made mandatory due to the impact it
> has 
> on some subarches:
> - On the 8xx, unless all areas are 8Mbytes aligned, there is a 
> significant overhead on TLB misses. And Aligning everthing to 8M is
> a 
> waste of RAM which is not acceptable on systems having very few RAM.
> - On hash book3s32, we are able to map the kernel BATs. With a few 
> alignment constraints, we are able to provide STRICT_KERNEL_RWX. But
> we 
> are unable to provide exec protection on page granularity. Only on 
> 256Mbytes segments. So for modules, we have to have the vmspace X. It
> is 
> also not possible to have a kernel area RO. Only user areas can be
> made RO.
> 

Yes, sorry, this was thoughtless from me, since in my mind I was just
thinking about the platforms I primarily work on (book3s64).

> Christophe
> 
> > Russell Currey (5):
> >    powerpc/mm: Implement set_memory() routines
> >    powerpc/kprobes: Mark newly allocated probes as RO
> >    powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
> >    powerpc: Set ARCH_HAS_STRICT_MODULE_RWX
> >    powerpc/configs: Enable STRICT_MODULE_RWX in skiroot_defconfig
> > 
> >   arch/powerpc/Kconfig                   |  2 +
> >   arch/powerpc/Kconfig.debug             |  6 +-
> >   arch/powerpc/configs/skiroot_defconfig |  1 +
> >   arch/powerpc/include/asm/set_memory.h  | 32 +++++++++++
> >   arch/powerpc/kernel/kprobes.c          |  3 +
> >   arch/powerpc/mm/Makefile               |  1 +
> >   arch/powerpc/mm/pageattr.c             | 77
> > ++++++++++++++++++++++++++
> >   arch/powerpc/mm/ptdump/ptdump.c        | 21 ++++++-
> >   8 files changed, 140 insertions(+), 3 deletions(-)
> >   create mode 100644 arch/powerpc/include/asm/set_memory.h
> >   create mode 100644 arch/powerpc/mm/pageattr.c
> >