[v16,00/23] selftests, powerpc, x86: Memory Protection Keys
mbox series

Message ID cover.1579507768.git.sandipan@linux.ibm.com
Headers show
Series
  • selftests, powerpc, x86: Memory Protection Keys
Related show

Message

Sandipan Das Jan. 20, 2020, 8:35 a.m. UTC
Memory protection keys enables an application to protect its address
space from inadvertent access by its own code.

This feature is now enabled on powerpc and has been available since
4.16-rc1. The patches move the selftests to arch neutral directory
and enhance their test coverage.

Tested on powerpc64 and x86_64 (Skylake-SP).

Link to development branch:
https://github.com/sandip4n/linux/tree/pkey-selftests

Changelog
---------
Link to previous version (v16):
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=153824

v17:
	(1) Fixed issues with i386 builds when running on x86_64
	    based on feedback from Dave.
	(2) Replaced patch 6 from previous version with patch 7.
	    This addresses u64 format specifier related concerns
	    that Michael had raised in v15.

v16:
	(1) Rebased on top of latest master.
	(2) Switched to u64 instead of using an arch-dependent
	    pkey_reg_t type for references to the pkey register
	    based on suggestions from Dave, Michal and Michael.
	(3) Removed build time determination of page size based
	    on suggestion from Michael.
	(4) Fixed comment before the definition of __page_o_noops()
	    from patch 13 ("selftests/vm/pkeys: Introduce powerpc
	    support").

v15:
	(1) Rebased on top of latest master.
	(2) Addressed review comments from Dave Hansen.
	(3) Moved code for getting or setting pkey bits to new
	    helpers. These changes replace patch 7 of v14.
	(4) Added a fix which ensures that the correct count of
	    reserved keys is used across different platforms.
	(5) Added a fix which ensures that the correct page size
	    is used as powerpc supports both 4K and 64K pages.

v14:
	(1) Incorporated another round of comments from Dave Hansen.

v13:
	(1) Incorporated comments for Dave Hansen.
	(2) Added one more test for correct pkey-0 behavior.

v12:
	(1) Fixed the offset of pkey field in the siginfo structure for
	    x86_64 and powerpc. And tries to use the actual field
	    if the headers have it defined.

v11:
	(1) Fixed a deadlock in the ptrace testcase.

v10 and prior:
	(1) Moved the testcase to arch neutral directory.
	(2) Split the changes into incremental patches.

Desnes A. Nunes do Rosario (1):
  selftests/vm/pkeys: Fix number of reserved powerpc pkeys

Ram Pai (16):
  selftests/x86/pkeys: Move selftests to arch-neutral directory
  selftests/vm/pkeys: Rename all references to pkru to a generic name
  selftests/vm/pkeys: Move generic definitions to header file
  selftests/vm/pkeys: Fix pkey_disable_clear()
  selftests/vm/pkeys: Fix assertion in pkey_disable_set/clear()
  selftests/vm/pkeys: Fix alloc_random_pkey() to make it really random
  selftests/vm/pkeys: Introduce generic pkey abstractions
  selftests/vm/pkeys: Introduce powerpc support
  selftests/vm/pkeys: Fix assertion in test_pkey_alloc_exhaust()
  selftests/vm/pkeys: Improve checks to determine pkey support
  selftests/vm/pkeys: Associate key on a mapped page and detect access
    violation
  selftests/vm/pkeys: Associate key on a mapped page and detect write
    violation
  selftests/vm/pkeys: Detect write violation on a mapped
    access-denied-key page
  selftests/vm/pkeys: Introduce a sub-page allocator
  selftests/vm/pkeys: Test correct behaviour of pkey-0
  selftests/vm/pkeys: Override access right definitions on powerpc

Sandipan Das (5):
  selftests: vm: pkeys: Fix multilib builds for x86
  selftests: vm: pkeys: Use sane types for pkey register
  selftests: vm: pkeys: Add helpers for pkey bits
  selftests: vm: pkeys: Use the correct huge page size
  selftests: vm: pkeys: Use the correct page size on powerpc

Thiago Jung Bauermann (2):
  selftests/vm/pkeys: Move some definitions to arch-specific header
  selftests/vm/pkeys: Make gcc check arguments of sigsafe_printf()

 tools/testing/selftests/vm/.gitignore         |   1 +
 tools/testing/selftests/vm/Makefile           |  50 ++
 tools/testing/selftests/vm/pkey-helpers.h     | 225 ++++++
 tools/testing/selftests/vm/pkey-powerpc.h     | 136 ++++
 tools/testing/selftests/vm/pkey-x86.h         | 181 +++++
 .../selftests/{x86 => vm}/protection_keys.c   | 696 ++++++++++--------
 tools/testing/selftests/x86/.gitignore        |   1 -
 tools/testing/selftests/x86/Makefile          |   2 +-
 tools/testing/selftests/x86/pkey-helpers.h    | 219 ------
 9 files changed, 979 insertions(+), 532 deletions(-)
 create mode 100644 tools/testing/selftests/vm/pkey-helpers.h
 create mode 100644 tools/testing/selftests/vm/pkey-powerpc.h
 create mode 100644 tools/testing/selftests/vm/pkey-x86.h
 rename tools/testing/selftests/{x86 => vm}/protection_keys.c (74%)
 delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h

Comments

Dave Hansen Jan. 22, 2020, 6:45 p.m. UTC | #1
Still doesn't build for me:

> # make
> make --no-builtin-rules ARCH=x86_64 -C ../../../.. headers_install
> make[1]: Entering directory '/home/dave/linux.git'
>   INSTALL ./usr/include
> make[1]: Leaving directory '/home/dave/linux.git'
> make: *** No rule to make target '/home/dave/linux.git/tools/testing/selftests/vm/protection_keys_32', needed by 'all'.  Stop.
Sandipan Das Jan. 27, 2020, 10:11 a.m. UTC | #2
Hi Dave,

On 23/01/20 12:15 am, Dave Hansen wrote:
> Still doesn't build for me:
> 

I have this patch that hopefully fixes this. My understanding was
that the vm tests are supposed to be generic but this has quite a
bit of x86-specific conditional code which complicates things even
though it is not used by any of the other tests.

I'm not sure if we should keep x86 multilib build support for these
selftests but I'll let the maintainers take a call.

From a5609f79e5d5164c99c3cb599e14ca620de9c8d4 Mon Sep 17 00:00:00 2001
From: Sandipan Das <sandipan@linux.ibm.com>
Date: Sat, 18 Jan 2020 15:59:04 +0530
Subject: [PATCH] selftests: vm: pkeys: Fix multilib builds for x86

This ensures that both 32-bit and 64-bit binaries are generated
when this is built on a x86_64 system that has both 32-bit and
64-bit libraries. Most of the changes have been borrowed from
tools/testing/selftests/x86/Makefile.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 tools/testing/selftests/vm/Makefile | 72 +++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 4e9c741be6af..82031f84af21 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -18,7 +18,30 @@ TEST_GEN_FILES += on-fault-limit
 TEST_GEN_FILES += thuge-gen
 TEST_GEN_FILES += transhuge-stress
 TEST_GEN_FILES += userfaultfd
+
+ifeq ($(ARCH),x86_64)
+CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh $(CC) ../x86/trivial_32bit_program.c -m32)
+CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh $(CC) ../x86/trivial_64bit_program.c)
+CAN_BUILD_WITH_NOPIE := $(shell ./../x86/check_cc.sh $(CC) ../x86/trivial_program.c -no-pie)
+
+TARGETS := protection_keys
+BINARIES_32 := $(TARGETS:%=%_32)
+BINARIES_64 := $(TARGETS:%=%_64)
+
+ifeq ($(CAN_BUILD_WITH_NOPIE),1)
+CFLAGS += -no-pie
+endif
+
+ifeq ($(CAN_BUILD_I386),1)
+TEST_GEN_FILES += $(BINARIES_32)
+endif
+
+ifeq ($(CAN_BUILD_X86_64),1)
+TEST_GEN_FILES += $(BINARIES_64)
+endif
+else
 TEST_GEN_FILES += protection_keys
+endif
 
 ifneq (,$(filter $(ARCH),arm64 ia64 mips64 parisc64 ppc64 riscv64 s390x sh64 sparc64 x86_64))
 TEST_GEN_FILES += va_128TBswitch
@@ -32,6 +55,55 @@ TEST_FILES := test_vmalloc.sh
 KSFT_KHDR_INSTALL := 1
 include ../lib.mk
 
+ifeq ($(ARCH),x86_64)
+BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
+BINARIES_64 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_64))
+
+define gen-target-rule-32
+$(1) $(1)_32: $(OUTPUT)/$(1)_32
+.PHONY: $(1) $(1)_32
+endef
+
+define gen-target-rule-64
+$(1) $(1)_64: $(OUTPUT)/$(1)_64
+.PHONY: $(1) $(1)_64
+endef
+
+ifeq ($(CAN_BUILD_I386),1)
+$(BINARIES_32): CFLAGS += -m32
+$(BINARIES_32): LDLIBS += -lrt -ldl -lm
+$(BINARIES_32): %_32: %.c
+	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(notdir $^) $(LDLIBS) -o $@
+$(foreach t,$(TARGETS),$(eval $(call gen-target-rule-32,$(t))))
+endif
+
+ifeq ($(CAN_BUILD_X86_64),1)
+$(BINARIES_64): CFLAGS += -m64
+$(BINARIES_64): LDLIBS += -lrt -ldl
+$(BINARIES_64): %_64: %.c
+	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(notdir $^) $(LDLIBS) -o $@
+$(foreach t,$(TARGETS),$(eval $(call gen-target-rule-64,$(t))))
+endif
+
+# x86_64 users should be encouraged to install 32-bit libraries
+ifeq ($(CAN_BUILD_I386)$(CAN_BUILD_X86_64),01)
+all: warn_32bit_failure
+
+warn_32bit_failure:
+	@echo "Warning: you seem to have a broken 32-bit build" 2>&1;		\
+	echo  "environment. This will reduce test coverage of 64-bit" 2>&1;	\
+	echo  "kernels. If you are using a Debian-like distribution," 2>&1;	\
+	echo  "try:"; 2>&1;							\
+	echo  "";								\
+	echo  "  apt-get install gcc-multilib libc6-i386 libc6-dev-i386";	\
+	echo  "";								\
+	echo  "If you are using a Fedora-like distribution, try:";		\
+	echo  "";								\
+	echo  "  yum install glibc-devel.*i686";				\
+	exit 0;
+endif
+endif
+
 $(OUTPUT)/userfaultfd: LDLIBS += -lpthread
 
 $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
Dave Hansen Jan. 27, 2020, 3:42 p.m. UTC | #3
On 1/27/20 2:11 AM, Sandipan Das wrote:
> Hi Dave,
> 
> On 23/01/20 12:15 am, Dave Hansen wrote:
>> Still doesn't build for me:
>>
> I have this patch that hopefully fixes this. My understanding was
> that the vm tests are supposed to be generic but this has quite a
> bit of x86-specific conditional code which complicates things even
> though it is not used by any of the other tests.
> 
> I'm not sure if we should keep x86 multilib build support for these
> selftests but I'll let the maintainers take a call.

How have you tested this patch (and the whole series for that matter)?
Sandipan Das Jan. 28, 2020, 9:38 a.m. UTC | #4
Hi Dave,

On 27/01/20 9:12 pm, Dave Hansen wrote:
> 
> How have you tested this patch (and the whole series for that matter)?
> 

I replaced the second patch with this one and did a build test.
Till v16, I had tested the whole series (build + run) on both a POWER8
system (with 4K and 64K page sizes) and a Skylake SP system but for
x86_64 only. Following that, I could only do a build test locally on
my laptop for i386 and x86_64 on my laptop as I did not have access to
the Skylake system anymore.

This is how I tested the build process:

$ cd linux
$ make -C tools/testing/selftests
...
make[1]: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
...
gcc -Wall -I ../../../../usr/include  -no-pie -m32  protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include  -no-pie -m64  protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
...

$ make -C tools/testing/selftests clean
$ make -C tools/testing/selftests/vm
make: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
make --no-builtin-rules ARCH=x86_64 -C ../../../.. headers_install
make[1]: Entering directory '/home/sandipan/.devel/linux'
  INSTALL ./usr/include
make[1]: Leaving directory '/home/sandipan/.devel/linux'
...
gcc -Wall -I ../../../../usr/include  -no-pie -m32  protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include  -no-pie -m64  protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
...

$ make -C tools/testing/selftests/vm clean
$ make -C tools/testing/selftests/vm protection_keys
make: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
gcc -Wall -I ../../../../usr/include  -no-pie -m32  protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include  -no-pie -m64  protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
make: Leaving directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'

$ make -C tools/testing/selftests/vm clean
$ make -C tools/testing/selftests/vm protection_keys_32
make: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
gcc -Wall -I ../../../../usr/include  -no-pie -m32  protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
make: Leaving directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'

$ make -C tools/testing/selftests/vm protection_keys_64
make: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
gcc -Wall -I ../../../../usr/include  -no-pie -m64  protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
make: Leaving directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'

$ make -C tools/testing/selftests/vm clean
$ cd tools/testing/selftests/vm
$ make
make --no-builtin-rules ARCH=x86_64 -C ../../../.. headers_install
make[1]: Entering directory '/home/sandipan/.devel/linux'
  INSTALL ./usr/include
make[1]: Leaving directory '/home/sandipan/.devel/linux'
...
gcc -Wall -I ../../../../usr/include  -no-pie -m32  protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include  -no-pie -m64  protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
...

$ make clean
$ make protection_keys
gcc -Wall -I ../../../../usr/include  -no-pie -m32  protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include  -no-pie -m64  protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64

$ make clean
$ make protection_keys_32
gcc -Wall -I ../../../../usr/include  -no-pie -m32  protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32

$ make protection_keys_64
gcc -Wall -I ../../../../usr/include  -no-pie -m64  protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64


- Sandipan
Sandipan Das Jan. 29, 2020, 9:56 a.m. UTC | #5
Hi Dave,

On 28/01/20 3:08 pm, Sandipan Das wrote:
> On 27/01/20 9:12 pm, Dave Hansen wrote:
>>
>> How have you tested this patch (and the whole series for that matter)?
>>
> 
> I replaced the second patch with this one and did a build test.
> Till v16, I had tested the whole series (build + run) on both a POWER8
> system (with 4K and 64K page sizes) and a Skylake SP system but for
> x86_64 only. Following that, I could only do a build test locally on
> my laptop for i386 and x86_64 on my laptop as I did not have access to
> the Skylake system anymore.
> 
> 

I got a chance to use the Skylake system today and tested (build + run)
the whole series (v17 with the fixed Makefile) for both i386 and x86_64.
Everything passed.

- Sandipan
Dave Hansen Jan. 29, 2020, 6:59 p.m. UTC | #6
On 1/28/20 1:38 AM, Sandipan Das wrote:
> On 27/01/20 9:12 pm, Dave Hansen wrote:
>> How have you tested this patch (and the whole series for that matter)?
>>
> I replaced the second patch with this one and did a build test.
> Till v16, I had tested the whole series (build + run) on both a POWER8
> system (with 4K and 64K page sizes) and a Skylake SP system but for
> x86_64 only.

Do you have any idea why I was seeing x86 build errors and you were not?
Florian Weimer Jan. 29, 2020, 7:04 p.m. UTC | #7
* Dave Hansen:

> Still doesn't build for me:
>
>> # make
>> make --no-builtin-rules ARCH=x86_64 -C ../../../.. headers_install
>> make[1]: Entering directory '/home/dave/linux.git'
>>   INSTALL ./usr/include
>> make[1]: Leaving directory '/home/dave/linux.git'
>> make: *** No rule to make target '/home/dave/linux.git/tools/testing/selftests/vm/protection_keys_32', needed by 'all'.  Stop.

Do you have 32-bit libraries installed?

Thanks,
Florian
Sandipan Das Jan. 30, 2020, 6:19 a.m. UTC | #8
Hi Dave,

On 30/01/20 12:29 am, Dave Hansen wrote:
> On 1/28/20 1:38 AM, Sandipan Das wrote:
>> On 27/01/20 9:12 pm, Dave Hansen wrote:
>>> How have you tested this patch (and the whole series for that matter)?
>>>
>> I replaced the second patch with this one and did a build test.
>> Till v16, I had tested the whole series (build + run) on both a POWER8
>> system (with 4K and 64K page sizes) and a Skylake SP system but for
>> x86_64 only.
> 
> Do you have any idea why I was seeing x86 build errors and you were not?
> 

There were problems with patch 2 from v17. The fixed patch is what I replied
with previously in this thread. The test results that I posted were with that
patch included. Will post out v18 today with the fix.

- Sandipan
Sandipan Das Jan. 30, 2020, 9:20 a.m. UTC | #9
On 30/01/20 11:49 am, Sandipan Das wrote:
> Hi Dave,
> 
> On 30/01/20 12:29 am, Dave Hansen wrote:
>> On 1/28/20 1:38 AM, Sandipan Das wrote:
>>> On 27/01/20 9:12 pm, Dave Hansen wrote:
>>>> How have you tested this patch (and the whole series for that matter)?
>>>>
>>> I replaced the second patch with this one and did a build test.
>>> Till v16, I had tested the whole series (build + run) on both a POWER8
>>> system (with 4K and 64K page sizes) and a Skylake SP system but for
>>> x86_64 only.
>>
>> Do you have any idea why I was seeing x86 build errors and you were not?
>>
> 
> There were problems with patch 2 from v17. The fixed patch is what I replied
> with previously in this thread. The test results that I posted were with that
> patch included. Will post out v18 today with the fix.
> 

In patch 2 of v17, the issue was with the target names. Upon adding something
to TEST_GEN_FILES, rules for targets like the following are expected to be
defined.
  <path-to-linux-source>/tools/testing/selftests/vm/protection_keys_32
  <path-to-linux-source>/tools/testing/selftests/vm/protection_keys_64
  <path-to-linux-source>/tools/testing/selftests/vm/protection_keys

But instead, I only defined rules for these.
  protection_keys_32
  protection_keys_64
  protection_keys

Hence the build was failing in these cases:
  $ make -C tools/testing/selftests
  $ make -C tools/testing/selftests/vm
  $ cd tools/testing/selftests/vm
  $ make

But worked in these cases:
  $ make -C tools/testing/selftests/vm protection_keys
  $ cd tools/testing/selftests/vm
  $ make protection_keys

This has been addressed in v18.

- Sandipan