[Linux-kselftest-mirror] [PATCH 0/5] selftests/x86: fixes for !CONFIG_IA32_EMULATION and vsyscall=none
On systems with CONFIG_IA32_EMULATION disabled and vsyscall disabled, a couple of selftests fail spectacularly.
Also throw in a fix for the Makefile, which still wants to build the moved 5lvl test.
Dominik Brodowski (5): selftests/x86: 5lvl test has been moved selftests/x86: fix vDSO selftest segfault for vsyscall=none selftests/x86: do not rely on int $0x80 in test_mremap_vdso.c selftests/x86: do not rely on int $0x80 in single_step_syscall.c selftests/x86: disable tests requiring 32bit support on pure 64bit systems
tools/testing/selftests/x86/Makefile | 24 +++++++---- tools/testing/selftests/x86/single_step_syscall.c | 5 ++- tools/testing/selftests/x86/test_mremap_vdso.c | 4 ++ tools/testing/selftests/x86/test_vdso.c | 50 +++++++++++++++++++---- 4 files changed, 67 insertions(+), 16 deletions(-)
Fixes: 235266b8e11c "selftests/vm: move 128TB mmap boundary test to generic directory" Signed-off-by: Dominik Brodowski linux@dominikbrodowski.net --- tools/testing/selftests/x86/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index 10ca46df1449..ce2615a2d105 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -11,7 +11,7 @@ TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_sysc TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \ test_FCMOV test_FCOMI test_FISTTP \ vdso_restorer -TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip 5lvl +TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip
TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY) TARGETS_C_64BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_64BIT_ONLY)
The vDSO selftest tries to execute a vsyscall unconditionally, even if it is not present on the test system (e.g. if booted with vsyscall=none or with CONFIG_LEGACY_VSYSCALL_NONE=y set. Fix this by copying (and tweaking) the vsyscall check from test_vsyscall.c
CC: Andrew Lutomirski luto@kernel.org Signed-off-by: Dominik Brodowski linux@dominikbrodowski.net --- tools/testing/selftests/x86/test_vdso.c | 50 ++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/x86/test_vdso.c b/tools/testing/selftests/x86/test_vdso.c index 29973cde06d3..558c8207e7b9 100644 --- a/tools/testing/selftests/x86/test_vdso.c +++ b/tools/testing/selftests/x86/test_vdso.c @@ -28,18 +28,52 @@
int nerrs = 0;
+typedef long (*getcpu_t)(unsigned *, unsigned *, void *); + +getcpu_t vgetcpu; +getcpu_t vdso_getcpu; + +static void *vsyscall_getcpu(void) +{ #ifdef __x86_64__ -# define VSYS(x) (x) + FILE *maps; + char line[128]; + bool found = false; + + maps = fopen("/proc/self/maps", "r"); + if (!maps) /* might still be present, but ignore it here, as we test vDSO not vsyscall */ + return NULL; + + while (fgets(line, sizeof(line), maps)) { + char r, x; + void *start, *end; + char name[128]; + if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s", + &start, &end, &r, &x, name) != 5) + continue; + + if (strcmp(name, "[vsyscall]")) + continue; + + /* assume entries are OK, as we test vDSO here not vsyscall */ + found = true; + break; + } + + fclose(maps); + + if (!found) { + printf("Warning: failed to find vsyscall getcpu\n"); + return NULL; + } + return (void *) (0xffffffffff600800); #else -# define VSYS(x) 0 + return NULL; #endif +}
-typedef long (*getcpu_t)(unsigned *, unsigned *, void *); - -const getcpu_t vgetcpu = (getcpu_t)VSYS(0xffffffffff600800); -getcpu_t vdso_getcpu;
-void fill_function_pointers() +static void fill_function_pointers() { void *vdso = dlopen("linux-vdso.so.1", RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); @@ -54,6 +88,8 @@ void fill_function_pointers() vdso_getcpu = (getcpu_t)dlsym(vdso, "__vdso_getcpu"); if (!vdso_getcpu) printf("Warning: failed to find getcpu in vDSO\n"); + + vgetcpu = (getcpu_t) vsyscall_getcpu(); }
static long sys_getcpu(unsigned * cpu, unsigned * node,
* Dominik Brodowski linux@dominikbrodowski.net wrote:
char name[128];if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s",&start, &end, &r, &x, name) != 5)
So that's a buffer overflow waiting to happen, if a line in 'maps' gets too large, right?
Thanks,
Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 11, 2018 at 12:21:53PM +0100, Ingo Molnar wrote:
- Dominik Brodowski linux@dominikbrodowski.net wrote:
char name[128];if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s",&start, &end, &r, &x, name) != 5)So that's a buffer overflow waiting to happen, if a line in 'maps' gets too large, right?
... as does tools/testing/selftests/x86/test_vsyscall.c already now, right? Will fix both up with an additional patch.
But a more generic question: Is there a quick, easy but reliable test available in userspace to determine whether int $0x80 vsyscall is available on a given system, or will cause a segfault?
Thanks, Dominik -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 11, 2018 at 01:17:14PM +0100, Dominik Brodowski wrote:
On Sun, Feb 11, 2018 at 12:21:53PM +0100, Ingo Molnar wrote:
- Dominik Brodowski linux@dominikbrodowski.net wrote:
char name[128];if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s",&start, &end, &r, &x, name) != 5)So that's a buffer overflow waiting to happen, if a line in 'maps' gets too large, right?
... as does tools/testing/selftests/x86/test_vsyscall.c already now, right? Will fix both up with an additional patch.
Maybe no fix is needed after all: The fgets() call a few lines above limits "line" to 127 chars max. So "name" can't even get close to 128 chars, right?
char line[128]; ... while (fgets(line, sizeof(line), maps)) {
Thanks, Dominik -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Dominik Brodowski linux@dominikbrodowski.net wrote:
On Sun, Feb 11, 2018 at 01:17:14PM +0100, Dominik Brodowski wrote:
On Sun, Feb 11, 2018 at 12:21:53PM +0100, Ingo Molnar wrote:
- Dominik Brodowski linux@dominikbrodowski.net wrote:
char name[128];if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s",&start, &end, &r, &x, name) != 5)So that's a buffer overflow waiting to happen, if a line in 'maps' gets too large, right?
... as does tools/testing/selftests/x86/test_vsyscall.c already now, right? Will fix both up with an additional patch.
Maybe no fix is needed after all: The fgets() call a few lines above limits "line" to 127 chars max. So "name" can't even get close to 128 chars, right?
char line[128]; ... while (fgets(line, sizeof(line), maps)) {
Yeah, probably - but still, this connection and the sscanf() guarantee is not obvious at first sight, so please improve this to derive from the same value (define a LINE_MAX size or such), plus maybe add a comment to the sscanf() line that this is safe because strlen(name) >= strlen(line).
Thanks,
Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Suggested-by: Ingo Molnar mingo@kernel.org CC: Andy Lutomirski luto@kernel.org Signed-off-by: Dominik Brodowski linux@dominikbrodowski.net
---
Yeah, probably - but still, this connection and the sscanf() guarantee is not obvious at first sight, so please improve this to derive from the same value (define a LINE_MAX size or such), plus maybe add a comment to the sscanf() line that this is safe because strlen(name) >= strlen(line).
Sounds reasonable. Patch (which applies on top of the five patches for selftests/x86 I sent out earlier today) is attached.
Thanks, Dominik
diff --git a/tools/testing/selftests/x86/test_vdso.c b/tools/testing/selftests/x86/test_vdso.c index 558c8207e7b9..7ade625f10ed 100644 --- a/tools/testing/selftests/x86/test_vdso.c +++ b/tools/testing/selftests/x86/test_vdso.c @@ -26,6 +26,9 @@ # endif #endif
+/* max length of lines in /proc/self/maps - anything longer is skipped here */ +#define MAPS_LINE_LEN 128 + int nerrs = 0;
typedef long (*getcpu_t)(unsigned *, unsigned *, void *); @@ -37,17 +40,19 @@ static void *vsyscall_getcpu(void) { #ifdef __x86_64__ FILE *maps; - char line[128]; + char line[MAPS_LINE_LEN]; bool found = false;
maps = fopen("/proc/self/maps", "r"); if (!maps) /* might still be present, but ignore it here, as we test vDSO not vsyscall */ return NULL;
- while (fgets(line, sizeof(line), maps)) { + while (fgets(line, MAPS_LINE_LEN, maps)) { char r, x; void *start, *end; - char name[128]; + char name[MAPS_LINE_LEN]; + + /* sscanf is safe here as strlen(name) >= strlen(line) */ if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s", &start, &end, &r, &x, name) != 5) continue; diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c index 7a744fa7b786..ee92e4727f18 100644 --- a/tools/testing/selftests/x86/test_vsyscall.c +++ b/tools/testing/selftests/x86/test_vsyscall.c @@ -33,6 +33,9 @@ # endif #endif
+/* max length of lines in /proc/self/maps - anything longer is skipped here */ +#define MAPS_LINE_LEN 128 + static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *), int flags) { @@ -98,7 +101,7 @@ static int init_vsys(void) #ifdef __x86_64__ int nerrs = 0; FILE *maps; - char line[128]; + char line[MAPS_LINE_LEN]; bool found = false;
maps = fopen("/proc/self/maps", "r"); @@ -108,10 +111,12 @@ static int init_vsys(void) return 0; }
- while (fgets(line, sizeof(line), maps)) { + while (fgets(line, MAPS_LINE_LEN, maps)) { char r, x; void *start, *end; - char name[128]; + char name[MAPS_LINE_LEN]; + + /* sscanf is safe here as strlen(name) >= strlen(line) */ if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s", &start, &end, &r, &x, name) != 5) continue; -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/11/2018 01:59 PM, Dominik Brodowski wrote:
Suggested-by: Ingo Molnar mingo@kernel.org CC: Andy Lutomirski luto@kernel.org Signed-off-by: Dominik Brodowski linux@dominikbrodowski.net
Missing commit log. Please add one.
thanks, -- Shuah
Yeah, probably - but still, this connection and the sscanf() guarantee is not obvious at first sight, so please improve this to derive from the same value (define a LINE_MAX size or such), plus maybe add a comment to the sscanf() line that this is safe because strlen(name) >= strlen(line).
Sounds reasonable. Patch (which applies on top of the five patches for selftests/x86 I sent out earlier today) is attached.
Thanks, Dominik
diff --git a/tools/testing/selftests/x86/test_vdso.c b/tools/testing/selftests/x86/test_vdso.c index 558c8207e7b9..7ade625f10ed 100644 --- a/tools/testing/selftests/x86/test_vdso.c +++ b/tools/testing/selftests/x86/test_vdso.c @@ -26,6 +26,9 @@ # endif #endif +/* max length of lines in /proc/self/maps - anything longer is skipped here */ +#define MAPS_LINE_LEN 128
int nerrs = 0; typedef long (*getcpu_t)(unsigned *, unsigned *, void *); @@ -37,17 +40,19 @@ static void *vsyscall_getcpu(void) { #ifdef __x86_64__ FILE *maps;
- char line[128];
- char line[MAPS_LINE_LEN]; bool found = false;
maps = fopen("/proc/self/maps", "r"); if (!maps) /* might still be present, but ignore it here, as we test vDSO not vsyscall */ return NULL;
- while (fgets(line, sizeof(line), maps)) {
- while (fgets(line, MAPS_LINE_LEN, maps)) { char r, x; void *start, *end;
char name[128];
char name[MAPS_LINE_LEN];/* sscanf is safe here as strlen(name) >= strlen(line) */diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c index 7a744fa7b786..ee92e4727f18 100644 --- a/tools/testing/selftests/x86/test_vsyscall.c +++ b/tools/testing/selftests/x86/test_vsyscall.c @@ -33,6 +33,9 @@ # endif #endif +/* max length of lines in /proc/self/maps - anything longer is skipped here */ +#define MAPS_LINE_LEN 128
static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *), int flags) { @@ -98,7 +101,7 @@ static int init_vsys(void) #ifdef __x86_64__ int nerrs = 0; FILE *maps;
- char line[128];
- char line[MAPS_LINE_LEN]; bool found = false;
maps = fopen("/proc/self/maps", "r"); @@ -108,10 +111,12 @@ static int init_vsys(void) return 0; }
- while (fgets(line, sizeof(line), maps)) {
- while (fgets(line, MAPS_LINE_LEN, maps)) { char r, x; void *start, *end;
char name[128];
char name[MAPS_LINE_LEN];/* sscanf is safe here as strlen(name) >= strlen(line) */
-- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 64bit builds, we should not rely on int $0x80 working (it only does if CONFIG_IA32_EMULATION is enabled). Without this patch, the move test may succeed, but the int $0x80 cause a segfault, resulting in a false negative output of this selftest.
CC: Dmitry Safonov dsafonov@virtuozzo.com CC: Andy Lutomirski luto@kernel.org Signed-off-by: Dominik Brodowski linux@dominikbrodowski.net --- tools/testing/selftests/x86/test_mremap_vdso.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/x86/test_mremap_vdso.c b/tools/testing/selftests/x86/test_mremap_vdso.c index bf0d687c7db7..64f11c8d9b76 100644 --- a/tools/testing/selftests/x86/test_mremap_vdso.c +++ b/tools/testing/selftests/x86/test_mremap_vdso.c @@ -90,8 +90,12 @@ int main(int argc, char **argv, char **envp) vdso_size += PAGE_SIZE; }
+#ifdef __i386__ /* Glibc is likely to explode now - exit with raw syscall */ asm volatile ("int $0x80" : : "a" (__NR_exit), "b" (!!ret)); +#else /* __x86_64__ */ + syscall(SYS_exit, ret); +#endif } else { int status;
On 64bit builds, we should not rely on int $0x80 working (it only does if CONFIG_IA32_EMULATION is enabled). To keep the "Set TF and check int80" test running on 64bit installs with CONFIG_IA32_EMULATION enabled, build this test only if we can also build 32bit binaries (which should be a good approximation for that).
CC: Dmitry Safonov dsafonov@virtuozzo.com CC: Andy Lutomirski luto@kernel.org Signed-off-by: Dominik Brodowski linux@dominikbrodowski.net --- tools/testing/selftests/x86/Makefile | 2 ++ tools/testing/selftests/x86/single_step_syscall.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index ce2615a2d105..371ae715c506 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -40,12 +40,14 @@ endef ifeq ($(CAN_BUILD_I386),1) all: all_32 TEST_PROGS += $(BINARIES_32) +EXTRA_CFLAGS += -DCAN_BUILD_32 $(foreach t,$(TARGETS_C_32BIT_ALL),$(eval $(call gen-target-rule-32,$(t)))) endif
ifeq ($(CAN_BUILD_X86_64),1) all: all_64 TEST_PROGS += $(BINARIES_64) +EXTRA_CFLAGS += -DCAN_BUILD_64 $(foreach t,$(TARGETS_C_64BIT_ALL),$(eval $(call gen-target-rule-64,$(t)))) endif
diff --git a/tools/testing/selftests/x86/single_step_syscall.c b/tools/testing/selftests/x86/single_step_syscall.c index a48da95c18fd..ddfdd635de16 100644 --- a/tools/testing/selftests/x86/single_step_syscall.c +++ b/tools/testing/selftests/x86/single_step_syscall.c @@ -119,7 +119,9 @@ static void check_result(void)
int main() { +#ifdef CAN_BUILD_32 int tmp; +#endif
sethandler(SIGTRAP, sigtrap, 0);
@@ -139,12 +141,13 @@ int main() : : "c" (post_nop) : "r11"); check_result(); #endif - +#ifdef CAN_BUILD_32 printf("[RUN]\tSet TF and check int80\n"); set_eflags(get_eflags() | X86_EFLAGS_TF); asm volatile ("int $0x80" : "=a" (tmp) : "a" (SYS_getpid) : INT80_CLOBBERS); check_result(); +#endif
/* * This test is particularly interesting if fast syscalls use
The ldt_gdt and ptrace_syscall selftests, even in their 64bit variant, use hard-coded 32bit syscall numbers and call int $0x80. This will fail on 64bit systems with CONFIG_IA32_EMULATION disabled. Therefore, do not build these tests if we cannot build 32bit binaries (which should be a good approximation for CONFIG_IA32_EMULATION being enabled).
CC: Dmitry Safonov dsafonov@virtuozzo.com CC: Andy Lutomirski luto@kernel.org Signed-off-by: Dominik Brodowski linux@dominikbrodowski.net --- tools/testing/selftests/x86/Makefile | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index 371ae715c506..d744991c0f4f 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -5,16 +5,26 @@ include ../lib.mk
.PHONY: all all_32 all_64 warn_32bit_failure clean
-TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall test_mremap_vdso \ - check_initial_reg_state sigreturn ldt_gdt iopl mpx-mini-test ioperm \ +UNAME_M := $(shell uname -m) +CAN_BUILD_I386 := $(shell ./check_cc.sh $(CC) trivial_32bit_program.c -m32) +CAN_BUILD_X86_64 := $(shell ./check_cc.sh $(CC) trivial_64bit_program.c) + +TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \ + check_initial_reg_state sigreturn iopl mpx-mini-test ioperm \ protection_keys test_vdso test_vsyscall TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \ test_FCMOV test_FCOMI test_FISTTP \ vdso_restorer TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip +# Some selftests require 32bit support enabled also on 64bit systems +TARGETS_C_32BIT_NEEDED := ldt_gdt ptrace_syscall
-TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY) +TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY) $(TARGETS_C_32BIT_NEEDED) TARGETS_C_64BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_64BIT_ONLY) +ifeq ($(CAN_BUILD_I386)$(CAN_BUILD_X86_64),11) +TARGETS_C_64BIT_ALL += $(TARGETS_C_32BIT_NEEDED) +endif + BINARIES_32 := $(TARGETS_C_32BIT_ALL:%=%_32) BINARIES_64 := $(TARGETS_C_64BIT_ALL:%=%_64)
@@ -23,10 +33,6 @@ BINARIES_64 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_64))
CFLAGS := -O2 -g -std=gnu99 -pthread -Wall -no-pie
-UNAME_M := $(shell uname -m) -CAN_BUILD_I386 := $(shell ./check_cc.sh $(CC) trivial_32bit_program.c -m32) -CAN_BUILD_X86_64 := $(shell ./check_cc.sh $(CC) trivial_64bit_program.c) - define gen-target-rule-32 $(1) $(1)_32: $(OUTPUT)/$(1)_32 .PHONY: $(1) $(1)_32
linux-kselftest-mirror@lists.linaro.org
-
Dominik Brodowski -
Ingo Molnar -
Shuah Khan