- Linux-kselftest-mirror - lists.linaro.org

[PATCH 00/13] Add arm64/signal initial kselftest support

by Cristian Marussi

Hi this patch aims to add the initial arch-specific arm64 support to kselftest starting with signals-related test-cases. A common internal test-case layout is proposed which then it is anyway wired-up to the toplevel kselftest Makefile, so that it should be possible at the end to run it on an arm64 target in the usual way with KSFT. ~/linux# make TARGETS=arm64 kselftest Notes: ----- - further details in the included READMEs - more tests still to be written (current strategy is going through the related Kernel signal-handling code and write a test for each possible and sensible code-path) - a bit of overlap around KSFT arm64/ Makefiles is expected with this recently proposed patch-series: http://lists.infradead.org/pipermail/linux-arm-kernel/2019-June/659432.html Cristian Marussi (13): kselftest: arm64: introduce new boilerplate code kselftest: arm64: adds arm64/signal support code kselftest: arm64: mangle_sp_misaligned kselftest: arm64: mangle_pc_invalid kselftest: arm64: mangle_pstate_invalid_daif_bits kselftest: arm64: mangle_pstate_invalid_state_toggle kselftest: arm64: mangle_pstate_invalid_mode_el? kselftest: arm64: mangle_pstate_ssbs_regs kselftest: arm64: fake_sigreturn_misaligned kselftest: arm64: fake_sigreturn_bad_magic kselftest: arm64: fake_sigreturn_bad_size kselftest: arm64: fake_sigreturn_bad_size_for_magic0 kselftest: arm64: fake_sigreturn_overflow_reserved tools/testing/selftests/Makefile | 1 + tools/testing/selftests/arm64/Makefile | 51 ++++ tools/testing/selftests/arm64/README | 44 +++ .../testing/selftests/arm64/signal/.gitignore | 5 + tools/testing/selftests/arm64/signal/Makefile | 86 ++++++ tools/testing/selftests/arm64/signal/README | 56 ++++ .../testing/selftests/arm64/signal/signals.S | 64 +++++ .../arm64/signal/test_arm64_signals.sh | 44 +++ .../selftests/arm64/signal/test_signals.c | 30 ++ .../selftests/arm64/signal/test_signals.h | 136 ++++++++++ .../arm64/signal/test_signals_utils.c | 256 ++++++++++++++++++ .../arm64/signal/test_signals_utils.h | 110 ++++++++ .../arm64/signal/testcases/.gitignore | 13 + .../testcases/fake_sigreturn_bad_magic.c | 42 +++ .../testcases/fake_sigreturn_bad_size.c | 40 +++ .../fake_sigreturn_bad_size_for_magic0.c | 44 +++ .../testcases/fake_sigreturn_misaligned.c | 30 ++ .../fake_sigreturn_overflow_reserved.c | 48 ++++ .../signal/testcases/mangle_pc_invalid.c | 24 ++ .../mangle_pstate_invalid_daif_bits.c | 25 ++ .../mangle_pstate_invalid_mode_el1.c | 25 ++ .../mangle_pstate_invalid_mode_el2.c | 25 ++ .../mangle_pstate_invalid_mode_el3.c | 25 ++ .../mangle_pstate_invalid_state_toggle.c | 25 ++ .../testcases/mangle_pstate_ssbs_regs.c | 41 +++ .../signal/testcases/mangle_sp_misaligned.c | 24 ++ .../arm64/signal/testcases/testcases.c | 123 +++++++++ .../arm64/signal/testcases/testcases.h | 85 ++++++ 28 files changed, 1522 insertions(+) create mode 100644 tools/testing/selftests/arm64/Makefile create mode 100644 tools/testing/selftests/arm64/README create mode 100644 tools/testing/selftests/arm64/signal/.gitignore create mode 100644 tools/testing/selftests/arm64/signal/Makefile create mode 100644 tools/testing/selftests/arm64/signal/README create mode 100644 tools/testing/selftests/arm64/signal/signals.S create mode 100755 tools/testing/selftests/arm64/signal/test_arm64_signals.sh create mode 100644 tools/testing/selftests/arm64/signal/test_signals.c create mode 100644 tools/testing/selftests/arm64/signal/test_signals.h create mode 100644 tools/testing/selftests/arm64/signal/test_signals_utils.c create mode 100644 tools/testing/selftests/arm64/signal/test_signals_utils.h create mode 100644 tools/testing/selftests/arm64/signal/testcases/.gitignore create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_misaligned.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_overflow_reserved.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pc_invalid.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_state_toggle.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_sp_misaligned.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/testcases.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/testcases.h -- 2.17.1

5 years, 9 months

3
36
0 0

[PATCH v2 00/10] Add arm64/signal initial kselftest support

by Cristian Marussi

Hi this patchset aims to add the initial arch-specific arm64 support to kselftest starting with signals-related test-cases. A common internal test-case layout is proposed which then it is anyway wired-up to the toplevel kselftest Makefile, so that it should be possible at the end to run it on an arm64 target in the usual way with KSFT. ~/linux# make TARGETS=arm64 kselftest New KSFT arm64 testcases live inside tools/testing/selftests/arm64 grouped by family inside subdirectories: arm64/signal is the first family proposed with this series. arm64/signal tests can be run via KSFT or standalone. Thanks Cristian Notes: ----- - further details in the included READMEs - more tests still to be written (current strategy is going through the related Kernel signal-handling code and write a test for each possible and sensible code-path) - a bit of overlap around KSFT arm64/ Makefiles is expected with this recently proposed patch-series: http://lists.infradead.org/pipermail/linux-arm-kernel/2019-June/659432.html Changes: -------- v1-->v2: - rebased on 5.2-rc7 - various makefile's cleanups - mixed READMEs fixes - fixed test_arm64_signals.sh runner script - cleaned up assembly code in signal.S - improved get_current_context() logic - fixed SAFE_WRITE() - common support code splitted into more chunks, each one introduced when needed by some new testcases - fixed some headers validation routines in testcases.c - removed some still broken/immature tests: + fake_sigreturn_misaligned + fake_sigreturn_overflow_reserved + mangle_pc_invalid + mangle_sp_misaligned - fixed some other testcases: + mangle_pstate_ssbs_regs: better checks of SSBS bit when feature unsupported + mangle_pstate_invalid_compat_toggle: name fix + mangle_pstate_invalid_mode_el[1-3]: precautionary zeroing PSTATE.MODE + fake_sigreturn_bad_magic, fake_sigreturn_bad_size, fake_sigreturn_bad_size_for_magic0: - accounting for available space...dropping extra when needed - keeping alignent - new testcases on FPSMID context: + fake_sigreturn_missing_fpsimd + fake_sigreturn_duplicated_fpsimd Cristian Marussi (10): kselftest: arm64: introduce new boilerplate code kselftest: arm64: adds first test and common utils kselftest: arm64: mangle_pstate_invalid_daif_bits kselftest: arm64: mangle_pstate_invalid_mode_el kselftest: arm64: mangle_pstate_ssbs_regs kselftest: arm64: fake_sigreturn_bad_magic kselftest: arm64: fake_sigreturn_bad_size_for_magic0 kselftest: arm64: fake_sigreturn_missing_fpsimd kselftest: arm64: fake_sigreturn_duplicated_fpsimd kselftest: arm64: fake_sigreturn_bad_size tools/testing/selftests/Makefile | 1 + tools/testing/selftests/arm64/Makefile | 51 +++ tools/testing/selftests/arm64/README | 43 +++ .../testing/selftests/arm64/signal/.gitignore | 5 + tools/testing/selftests/arm64/signal/Makefile | 86 +++++ tools/testing/selftests/arm64/signal/README | 59 ++++ .../testing/selftests/arm64/signal/signals.S | 64 ++++ .../arm64/signal/test_arm64_signals.sh | 55 +++ .../selftests/arm64/signal/test_signals.c | 26 ++ .../selftests/arm64/signal/test_signals.h | 141 ++++++++ .../arm64/signal/test_signals_utils.c | 331 ++++++++++++++++++ .../arm64/signal/test_signals_utils.h | 31 ++ .../arm64/signal/testcases/.gitignore | 11 + .../testcases/fake_sigreturn_bad_magic.c | 63 ++++ .../testcases/fake_sigreturn_bad_size.c | 85 +++++ .../fake_sigreturn_bad_size_for_magic0.c | 57 +++ .../fake_sigreturn_duplicated_fpsimd.c | 62 ++++ .../testcases/fake_sigreturn_missing_fpsimd.c | 44 +++ .../mangle_pstate_invalid_compat_toggle.c | 25 ++ .../mangle_pstate_invalid_daif_bits.c | 28 ++ .../mangle_pstate_invalid_mode_el1.c | 29 ++ .../mangle_pstate_invalid_mode_el2.c | 29 ++ .../mangle_pstate_invalid_mode_el3.c | 29 ++ .../testcases/mangle_pstate_ssbs_regs.c | 56 +++ .../arm64/signal/testcases/testcases.c | 150 ++++++++ .../arm64/signal/testcases/testcases.h | 83 +++++ 26 files changed, 1644 insertions(+) create mode 100644 tools/testing/selftests/arm64/Makefile create mode 100644 tools/testing/selftests/arm64/README create mode 100644 tools/testing/selftests/arm64/signal/.gitignore create mode 100644 tools/testing/selftests/arm64/signal/Makefile create mode 100644 tools/testing/selftests/arm64/signal/README create mode 100644 tools/testing/selftests/arm64/signal/signals.S create mode 100755 tools/testing/selftests/arm64/signal/test_arm64_signals.sh create mode 100644 tools/testing/selftests/arm64/signal/test_signals.c create mode 100644 tools/testing/selftests/arm64/signal/test_signals.h create mode 100644 tools/testing/selftests/arm64/signal/test_signals_utils.c create mode 100644 tools/testing/selftests/arm64/signal/test_signals_utils.h create mode 100644 tools/testing/selftests/arm64/signal/testcases/.gitignore create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/testcases.c create mode 100644 tools/testing/selftests/arm64/signal/testcases/testcases.h -- 2.17.1

5 years, 9 months

1
10
0 0

[PATCH bpf 1/3] bpf, x32: Fix bug with ALU64 {LSH, RSH, ARSH} BPF_X shift by 0

by Luke Nelson

The current x32 BPF JIT for shift operations is not correct when the shift amount in a register is 0. The expected behavior is a no-op, whereas the current implementation changes bits in the destination register. The following example demonstrates the bug. The expected result of this program is 1, but the current JITed code returns 2. r0 = 1 r1 = 1 r2 = 0 r1 <<= r2 if r1 == 1 goto end r0 = 2 end: exit The bug is caused by an incorrect assumption by the JIT that a shift by 32 clear the register. On x32 however, shifts use the lower 5 bits of the source, making a shift by 32 equivalent to a shift by 0. This patch fixes the bug using double-precision shifts, which also simplifies the code. Fixes: 03f5781be2c7 ("bpf, x86_32: add eBPF JIT compiler for ia32") Co-developed-by: Xi Wang <xi.wang(a)gmail.com> Signed-off-by: Xi Wang <xi.wang(a)gmail.com> Signed-off-by: Luke Nelson <luke.r.nels(a)gmail.com> --- arch/x86/net/bpf_jit_comp32.c | 221 ++++------------------------------ 1 file changed, 23 insertions(+), 198 deletions(-) diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c index b29e82f190c7..f34ef513f4f9 100644 --- a/arch/x86/net/bpf_jit_comp32.c +++ b/arch/x86/net/bpf_jit_comp32.c @@ -724,9 +724,6 @@ static inline void emit_ia32_lsh_r64(const u8 dst[], const u8 src[], { u8 *prog = *pprog; int cnt = 0; - static int jmp_label1 = -1; - static int jmp_label2 = -1; - static int jmp_label3 = -1; u8 dreg_lo = dstk ? IA32_EAX : dst_lo; u8 dreg_hi = dstk ? IA32_EDX : dst_hi; @@ -745,79 +742,23 @@ static inline void emit_ia32_lsh_r64(const u8 dst[], const u8 src[], /* mov ecx,src_lo */ EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_ECX)); - /* cmp ecx,32 */ - EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32); - /* Jumps when >= 32 */ - if (is_imm8(jmp_label(jmp_label1, 2))) - EMIT2(IA32_JAE, jmp_label(jmp_label1, 2)); - else - EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label1, 6)); - - /* < 32 */ - /* shl dreg_hi,cl */ - EMIT2(0xD3, add_1reg(0xE0, dreg_hi)); - /* mov ebx,dreg_lo */ - EMIT2(0x8B, add_2reg(0xC0, dreg_lo, IA32_EBX)); + /* shld dreg_hi,dreg_lo,cl */ + EMIT3(0x0F, 0xA5, add_2reg(0xC0, dreg_hi, dreg_lo)); /* shl dreg_lo,cl */ EMIT2(0xD3, add_1reg(0xE0, dreg_lo)); - /* IA32_ECX = -IA32_ECX + 32 */ - /* neg ecx */ - EMIT2(0xF7, add_1reg(0xD8, IA32_ECX)); - /* add ecx,32 */ - EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32); - - /* shr ebx,cl */ - EMIT2(0xD3, add_1reg(0xE8, IA32_EBX)); - /* or dreg_hi,ebx */ - EMIT2(0x09, add_2reg(0xC0, dreg_hi, IA32_EBX)); - - /* goto out; */ - if (is_imm8(jmp_label(jmp_label3, 2))) - EMIT2(0xEB, jmp_label(jmp_label3, 2)); - else - EMIT1_off32(0xE9, jmp_label(jmp_label3, 5)); - - /* >= 32 */ - if (jmp_label1 == -1) - jmp_label1 = cnt; + /* if ecx >= 32, mov dreg_lo into dreg_hi and clear dreg_lo */ - /* cmp ecx,64 */ - EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 64); - /* Jumps when >= 64 */ - if (is_imm8(jmp_label(jmp_label2, 2))) - EMIT2(IA32_JAE, jmp_label(jmp_label2, 2)); - else - EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label2, 6)); + /* cmp ecx,32 */ + EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32); + /* skip the next two instructions (4 bytes) when < 32 */ + EMIT2(IA32_JB, 4); - /* >= 32 && < 64 */ - /* sub ecx,32 */ - EMIT3(0x83, add_1reg(0xE8, IA32_ECX), 32); - /* shl dreg_lo,cl */ - EMIT2(0xD3, add_1reg(0xE0, dreg_lo)); /* mov dreg_hi,dreg_lo */ EMIT2(0x89, add_2reg(0xC0, dreg_hi, dreg_lo)); - /* xor dreg_lo,dreg_lo */ EMIT2(0x33, add_2reg(0xC0, dreg_lo, dreg_lo)); - /* goto out; */ - if (is_imm8(jmp_label(jmp_label3, 2))) - EMIT2(0xEB, jmp_label(jmp_label3, 2)); - else - EMIT1_off32(0xE9, jmp_label(jmp_label3, 5)); - - /* >= 64 */ - if (jmp_label2 == -1) - jmp_label2 = cnt; - /* xor dreg_lo,dreg_lo */ - EMIT2(0x33, add_2reg(0xC0, dreg_lo, dreg_lo)); - /* xor dreg_hi,dreg_hi */ - EMIT2(0x33, add_2reg(0xC0, dreg_hi, dreg_hi)); - - if (jmp_label3 == -1) - jmp_label3 = cnt; - if (dstk) { /* mov dword ptr [ebp+off],dreg_lo */ EMIT3(0x89, add_2reg(0x40, IA32_EBP, dreg_lo), @@ -836,9 +777,6 @@ static inline void emit_ia32_arsh_r64(const u8 dst[], const u8 src[], { u8 *prog = *pprog; int cnt = 0; - static int jmp_label1 = -1; - static int jmp_label2 = -1; - static int jmp_label3 = -1; u8 dreg_lo = dstk ? IA32_EAX : dst_lo; u8 dreg_hi = dstk ? IA32_EDX : dst_hi; @@ -857,78 +795,22 @@ static inline void emit_ia32_arsh_r64(const u8 dst[], const u8 src[], /* mov ecx,src_lo */ EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_ECX)); - /* cmp ecx,32 */ - EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32); - /* Jumps when >= 32 */ - if (is_imm8(jmp_label(jmp_label1, 2))) - EMIT2(IA32_JAE, jmp_label(jmp_label1, 2)); - else - EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label1, 6)); - - /* < 32 */ - /* lshr dreg_lo,cl */ - EMIT2(0xD3, add_1reg(0xE8, dreg_lo)); - /* mov ebx,dreg_hi */ - EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX)); - /* ashr dreg_hi,cl */ + /* shrd dreg_lo,dreg_hi,cl */ + EMIT3(0x0F, 0xAD, add_2reg(0xC0, dreg_lo, dreg_hi)); + /* sar dreg_hi,cl */ EMIT2(0xD3, add_1reg(0xF8, dreg_hi)); - /* IA32_ECX = -IA32_ECX + 32 */ - /* neg ecx */ - EMIT2(0xF7, add_1reg(0xD8, IA32_ECX)); - /* add ecx,32 */ - EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32); - - /* shl ebx,cl */ - EMIT2(0xD3, add_1reg(0xE0, IA32_EBX)); - /* or dreg_lo,ebx */ - EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX)); - - /* goto out; */ - if (is_imm8(jmp_label(jmp_label3, 2))) - EMIT2(0xEB, jmp_label(jmp_label3, 2)); - else - EMIT1_off32(0xE9, jmp_label(jmp_label3, 5)); - - /* >= 32 */ - if (jmp_label1 == -1) - jmp_label1 = cnt; + /* if ecx >= 32, mov dreg_hi to dreg_lo and set/clear dreg_hi depending on sign */ - /* cmp ecx,64 */ - EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 64); - /* Jumps when >= 64 */ - if (is_imm8(jmp_label(jmp_label2, 2))) - EMIT2(IA32_JAE, jmp_label(jmp_label2, 2)); - else - EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label2, 6)); + /* cmp ecx,32 */ + EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32); + /* skip the next two instructions (5 bytes) when < 32 */ + EMIT2(IA32_JB, 5); - /* >= 32 && < 64 */ - /* sub ecx,32 */ - EMIT3(0x83, add_1reg(0xE8, IA32_ECX), 32); - /* ashr dreg_hi,cl */ - EMIT2(0xD3, add_1reg(0xF8, dreg_hi)); /* mov dreg_lo,dreg_hi */ EMIT2(0x89, add_2reg(0xC0, dreg_lo, dreg_hi)); - - /* ashr dreg_hi,imm8 */ - EMIT3(0xC1, add_1reg(0xF8, dreg_hi), 31); - - /* goto out; */ - if (is_imm8(jmp_label(jmp_label3, 2))) - EMIT2(0xEB, jmp_label(jmp_label3, 2)); - else - EMIT1_off32(0xE9, jmp_label(jmp_label3, 5)); - - /* >= 64 */ - if (jmp_label2 == -1) - jmp_label2 = cnt; - /* ashr dreg_hi,imm8 */ + /* sar dreg_hi,31 */ EMIT3(0xC1, add_1reg(0xF8, dreg_hi), 31); - /* mov dreg_lo,dreg_hi */ - EMIT2(0x89, add_2reg(0xC0, dreg_lo, dreg_hi)); - - if (jmp_label3 == -1) - jmp_label3 = cnt; if (dstk) { /* mov dword ptr [ebp+off],dreg_lo */ @@ -948,9 +830,6 @@ static inline void emit_ia32_rsh_r64(const u8 dst[], const u8 src[], bool dstk, { u8 *prog = *pprog; int cnt = 0; - static int jmp_label1 = -1; - static int jmp_label2 = -1; - static int jmp_label3 = -1; u8 dreg_lo = dstk ? IA32_EAX : dst_lo; u8 dreg_hi = dstk ? IA32_EDX : dst_hi; @@ -969,77 +848,23 @@ static inline void emit_ia32_rsh_r64(const u8 dst[], const u8 src[], bool dstk, /* mov ecx,src_lo */ EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_ECX)); - /* cmp ecx,32 */ - EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32); - /* Jumps when >= 32 */ - if (is_imm8(jmp_label(jmp_label1, 2))) - EMIT2(IA32_JAE, jmp_label(jmp_label1, 2)); - else - EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label1, 6)); - - /* < 32 */ - /* lshr dreg_lo,cl */ - EMIT2(0xD3, add_1reg(0xE8, dreg_lo)); - /* mov ebx,dreg_hi */ - EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX)); + /* shrd dreg_lo,dreg_hi,cl */ + EMIT3(0x0F, 0xAD, add_2reg(0xC0, dreg_lo, dreg_hi)); /* shr dreg_hi,cl */ EMIT2(0xD3, add_1reg(0xE8, dreg_hi)); - /* IA32_ECX = -IA32_ECX + 32 */ - /* neg ecx */ - EMIT2(0xF7, add_1reg(0xD8, IA32_ECX)); - /* add ecx,32 */ - EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32); - - /* shl ebx,cl */ - EMIT2(0xD3, add_1reg(0xE0, IA32_EBX)); - /* or dreg_lo,ebx */ - EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX)); + /* if ecx >= 32, mov dreg_hi to dreg_lo and clear dreg_hi */ - /* goto out; */ - if (is_imm8(jmp_label(jmp_label3, 2))) - EMIT2(0xEB, jmp_label(jmp_label3, 2)); - else - EMIT1_off32(0xE9, jmp_label(jmp_label3, 5)); - - /* >= 32 */ - if (jmp_label1 == -1) - jmp_label1 = cnt; - /* cmp ecx,64 */ - EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 64); - /* Jumps when >= 64 */ - if (is_imm8(jmp_label(jmp_label2, 2))) - EMIT2(IA32_JAE, jmp_label(jmp_label2, 2)); - else - EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label2, 6)); + /* cmp ecx,32 */ + EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32); + /* skip the next two instructions (4 bytes) when < 32 */ + EMIT2(IA32_JB, 4); - /* >= 32 && < 64 */ - /* sub ecx,32 */ - EMIT3(0x83, add_1reg(0xE8, IA32_ECX), 32); - /* shr dreg_hi,cl */ - EMIT2(0xD3, add_1reg(0xE8, dreg_hi)); /* mov dreg_lo,dreg_hi */ EMIT2(0x89, add_2reg(0xC0, dreg_lo, dreg_hi)); /* xor dreg_hi,dreg_hi */ EMIT2(0x33, add_2reg(0xC0, dreg_hi, dreg_hi)); - /* goto out; */ - if (is_imm8(jmp_label(jmp_label3, 2))) - EMIT2(0xEB, jmp_label(jmp_label3, 2)); - else - EMIT1_off32(0xE9, jmp_label(jmp_label3, 5)); - - /* >= 64 */ - if (jmp_label2 == -1) - jmp_label2 = cnt; - /* xor dreg_lo,dreg_lo */ - EMIT2(0x33, add_2reg(0xC0, dreg_lo, dreg_lo)); - /* xor dreg_hi,dreg_hi */ - EMIT2(0x33, add_2reg(0xC0, dreg_hi, dreg_hi)); - - if (jmp_label3 == -1) - jmp_label3 = cnt; - if (dstk) { /* mov dword ptr [ebp+off],dreg_lo */ EMIT3(0x89, add_2reg(0x40, IA32_EBP, dreg_lo), -- 2.20.1

5 years, 9 months

2
3
0 0

[PATCH v5 0/2] clocksource/drivers: Create new Hyper-V clocksource driver

by Michael Kelley

This patch series moves Hyper-V clock/timer code to a separate Hyper-V clocksource driver. Previously, Hyper-V clock/timer code and data structures were mixed in with other Hyper-V code in the ISA independent drivers/hv code as well as in ISA dependent code. The new Hyper-V clocksource driver is ISA agnostic, with a just few dependencies on ISA specific functions. The patch series does not change any behavior or functionality -- it only reorganizes the existing code and fixes up the linkages. A few places outside of Hyper-V code are fixed up to use the new #include file structure. This restructuring is in response to Marc Zyngier's review comments on supporting Hyper-V running on ARM64, and is a good idea in general. It increases the amount of code shared between the x86 and ARM64 architectures, and reduces the size of the new code for supporting Hyper-V on ARM64. A new version of the Hyper-V on ARM64 patches will follow once this clocksource restructuring is accepted. The code is diff'ed against the upstream tip tree: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/vdso Changes in v5: * Revised commit summaries [Thomas Gleixner] * Removed call to clockevents_unbind_device() [Thomas Gleixner] * Restructured hv_init_clocksource() [Thomas Gleixner] * Various other small code cleanups [Thomas Gleixner] Changes in v4: * Revised commit messages * Rebased to upstream tip tree Changes in v3: * Removed boolean argument to hv_init_clocksource(). Always call sched_clock_register, which is needed on ARM64 but a no-op on x86. * Removed separate cpuhp setup in hv_stimer_alloc() and instead directly call hv_stimer_init() and hv_stimer_cleanup() from corresponding VMbus functions. This more closely matches original code and avoids clocksource stop/restart problems on ARM64 when VMbus code denies CPU offlining request. Changes in v2: * Revised commit short descriptions so the distinction between the first and second patches is clearer [GregKH] * Renamed new clocksource driver files and functions to use existing "timer" and "stimer" names instead of introducing "syntimer". [Vitaly Kuznetsov] * Introduced CONFIG_HYPER_TIMER to fix build problem when CONFIG_HYPERV=m [Vitaly Kuznetsov] * Added "Suggested-by: Marc Zyngier" Michael Kelley (2): clocksource/drivers: Make Hyper-V clocksource ISA agnostic clocksource/drivers: Continue making Hyper-V clocksource ISA agnostic MAINTAINERS | 2 + arch/x86/entry/vdso/vma.c | 2 +- arch/x86/hyperv/hv_init.c | 91 +-------- arch/x86/include/asm/hyperv-tlfs.h | 6 + arch/x86/include/asm/mshyperv.h | 81 +------- arch/x86/include/asm/vdso/gettimeofday.h | 2 +- arch/x86/kernel/cpu/mshyperv.c | 4 +- arch/x86/kvm/x86.c | 1 + drivers/clocksource/Makefile | 1 + drivers/clocksource/hyperv_timer.c | 339 +++++++++++++++++++++++++++++++ drivers/hv/Kconfig | 3 + drivers/hv/hv.c | 156 +------------- drivers/hv/hv_util.c | 1 + drivers/hv/hyperv_vmbus.h | 3 - drivers/hv/vmbus_drv.c | 42 ++-- include/clocksource/hyperv_timer.h | 105 ++++++++++ 16 files changed, 503 insertions(+), 336 deletions(-) create mode 100644 drivers/clocksource/hyperv_timer.c create mode 100644 include/clocksource/hyperv_timer.h -- 1.8.3.1

5 years, 9 months

2
4
0 0

[PATCH AUTOSEL 5.1 09/39] selftests/powerpc: Add test of fork with mapping above 512TB

by Sasha Levin

From: Michael Ellerman <mpe(a)ellerman.id.au> [ Upstream commit 16391bfc862342f285195013b73c1394fab28b97 ] This tests that when a process with a mapping above 512TB forks we correctly separate the parent and child address spaces. This exercises the bug in the context id handling fixed in the previous commit. Signed-off-by: Michael Ellerman <mpe(a)ellerman.id.au> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/powerpc/mm/.gitignore | 3 +- tools/testing/selftests/powerpc/mm/Makefile | 4 +- .../powerpc/mm/large_vm_fork_separation.c | 87 +++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/powerpc/mm/large_vm_fork_separation.c diff --git a/tools/testing/selftests/powerpc/mm/.gitignore b/tools/testing/selftests/powerpc/mm/.gitignore index ba919308fe30..d503b8764a8e 100644 --- a/tools/testing/selftests/powerpc/mm/.gitignore +++ b/tools/testing/selftests/powerpc/mm/.gitignore @@ -3,4 +3,5 @@ subpage_prot tempfile prot_sao segv_errors -wild_bctr \ No newline at end of file +wild_bctr +large_vm_fork_separation \ No newline at end of file diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile index 43d68420e363..f1fbc15800c4 100644 --- a/tools/testing/selftests/powerpc/mm/Makefile +++ b/tools/testing/selftests/powerpc/mm/Makefile @@ -2,7 +2,8 @@ noarg: $(MAKE) -C ../ -TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr +TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr \ + large_vm_fork_separation TEST_GEN_FILES := tempfile top_srcdir = ../../../../.. @@ -13,6 +14,7 @@ $(TEST_GEN_PROGS): ../harness.c $(OUTPUT)/prot_sao: ../utils.c $(OUTPUT)/wild_bctr: CFLAGS += -m64 +$(OUTPUT)/large_vm_fork_separation: CFLAGS += -m64 $(OUTPUT)/tempfile: dd if=/dev/zero of=$@ bs=64k count=1 diff --git a/tools/testing/selftests/powerpc/mm/large_vm_fork_separation.c b/tools/testing/selftests/powerpc/mm/large_vm_fork_separation.c new file mode 100644 index 000000000000..2363a7f3ab0d --- /dev/null +++ b/tools/testing/selftests/powerpc/mm/large_vm_fork_separation.c @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-2.0+ +// +// Copyright 2019, Michael Ellerman, IBM Corp. +// +// Test that allocating memory beyond the memory limit and then forking is +// handled correctly, ie. the child is able to access the mappings beyond the +// memory limit and the child's writes are not visible to the parent. + +#include <stdio.h> +#include <stdlib.h> +#include <sys/mman.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <unistd.h> + +#include "utils.h" + + +#ifndef MAP_FIXED_NOREPLACE +#define MAP_FIXED_NOREPLACE MAP_FIXED // "Should be safe" above 512TB +#endif + + +static int test(void) +{ + int p2c[2], c2p[2], rc, status, c, *p; + unsigned long page_size; + pid_t pid; + + page_size = sysconf(_SC_PAGESIZE); + SKIP_IF(page_size != 65536); + + // Create a mapping at 512TB to allocate an extended_id + p = mmap((void *)(512ul << 40), page_size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED_NOREPLACE, -1, 0); + if (p == MAP_FAILED) { + perror("mmap"); + printf("Error: couldn't mmap(), confirm kernel has 4TB support?\n"); + return 1; + } + + printf("parent writing %p = 1\n", p); + *p = 1; + + FAIL_IF(pipe(p2c) == -1 || pipe(c2p) == -1); + + pid = fork(); + if (pid == 0) { + FAIL_IF(read(p2c[0], &c, 1) != 1); + + pid = getpid(); + printf("child writing %p = %d\n", p, pid); + *p = pid; + + FAIL_IF(write(c2p[1], &c, 1) != 1); + FAIL_IF(read(p2c[0], &c, 1) != 1); + exit(0); + } + + c = 0; + FAIL_IF(write(p2c[1], &c, 1) != 1); + FAIL_IF(read(c2p[0], &c, 1) != 1); + + // Prevent compiler optimisation + barrier(); + + rc = 0; + printf("parent reading %p = %d\n", p, *p); + if (*p != 1) { + printf("Error: BUG! parent saw child's write! *p = %d\n", *p); + rc = 1; + } + + FAIL_IF(write(p2c[1], &c, 1) != 1); + FAIL_IF(waitpid(pid, &status, 0) == -1); + FAIL_IF(!WIFEXITED(status) || WEXITSTATUS(status)); + + if (rc == 0) + printf("success: test completed OK\n"); + + return rc; +} + +int main(void) +{ + return test_harness(test, "large_vm_fork_separation"); +} -- 2.20.1

5 years, 9 months

1
0
0 0

[PATCHv2] selftests/net: skip psock_tpacket test if KALLSYMS was not enabled

by Po-Hsu Lin

The psock_tpacket test will need to access /proc/kallsyms, this would require the kernel config CONFIG_KALLSYMS to be enabled first. Apart from adding CONFIG_KALLSYMS to the net/config file here, check the file existence to determine if we can run this test will be helpful to avoid a false-positive test result when testing it directly with the following commad against a kernel that have CONFIG_KALLSYMS disabled: make -C tools/testing/selftests TARGETS=net run_tests Signed-off-by: Po-Hsu Lin <po-hsu.lin(a)canonical.com> --- tools/testing/selftests/net/config | 1 + tools/testing/selftests/net/run_afpackettests | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config index 4740404..3dea2cb 100644 --- a/tools/testing/selftests/net/config +++ b/tools/testing/selftests/net/config @@ -25,3 +25,4 @@ CONFIG_NF_TABLES_IPV6=y CONFIG_NF_TABLES_IPV4=y CONFIG_NFT_CHAIN_NAT_IPV6=m CONFIG_NFT_CHAIN_NAT_IPV4=m +CONFIG_KALLSYMS=y diff --git a/tools/testing/selftests/net/run_afpackettests b/tools/testing/selftests/net/run_afpackettests index ea5938e..8b42e8b 100755 --- a/tools/testing/selftests/net/run_afpackettests +++ b/tools/testing/selftests/net/run_afpackettests @@ -21,12 +21,16 @@ fi echo "--------------------" echo "running psock_tpacket test" echo "--------------------" -./in_netns.sh ./psock_tpacket -if [ $? -ne 0 ]; then - echo "[FAIL]" - ret=1 +if [ -f /proc/kallsyms ]; then + ./in_netns.sh ./psock_tpacket + if [ $? -ne 0 ]; then + echo "[FAIL]" + ret=1 + else + echo "[PASS]" + fi else - echo "[PASS]" + echo "[SKIP] CONFIG_KALLSYMS not enabled" fi echo "--------------------" -- 2.7.4

5 years, 9 months

3
2
0 0

[PATCH][next] selftests/x86: fix spelling mistake "FAILT" -> "FAIL"

by Colin King

From: Colin Ian King <colin.king(a)canonical.com> There is an spelling mistake in an a test error message. Fix it. Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- tools/testing/selftests/x86/test_vsyscall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c index 4602326b8f5b..a4f4d4cf22c3 100644 --- a/tools/testing/selftests/x86/test_vsyscall.c +++ b/tools/testing/selftests/x86/test_vsyscall.c @@ -451,7 +451,7 @@ static int test_vsys_x(void) printf("[OK]\tExecuting the vsyscall page failed: #PF(0x%lx)\n", segv_err); } else { - printf("[FAILT]\tExecution failed with the wrong error: #PF(0x%lx)\n", + printf("[FAIL]\tExecution failed with the wrong error: #PF(0x%lx)\n", segv_err); return 1; } -- 2.20.1

5 years, 9 months

5
8
0 0

[PATCH] selftests/kselftest/runner.sh: Add 30 second timeout per test

by Anders Roxell

Commit a745f7af3cbd ("selftests/harness: Add 30 second timeout per test") solves that binary tests doesn't hang forever. However, scripts can still hang forever, this adds an timeout to each test script run. This assumes that an individual test doesn't take longer than 30 seconds. Signed-off-by: Anders Roxell <anders.roxell(a)linaro.org> --- tools/testing/selftests/kselftest/runner.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index 00c9020bdda8..cff7d2d83648 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -5,6 +5,7 @@ export skip_rc=4 export logfile=/dev/stdout export per_test_logging= +export TEST_TIMEOUT_DEFAULT=30 # There isn't a shell-agnostic way to find the path of a sourced file, # so we must rely on BASE_DIR being set to find other tools. @@ -24,6 +25,14 @@ tap_prefix() fi } +tap_timeout() +{ + if [ -x /usr/bin/timeout ] && [ -x "$BASENAME_TEST" ] \ + && file $BASENAME_TEST |grep -q "shell script"; then + echo -n "timeout $TEST_TIMEOUT_DEFAULT" + fi +} + run_one() { DIR="$1" @@ -44,7 +53,7 @@ run_one() echo "not ok $test_num $TEST_HDR_MSG" else cd `dirname $TEST` > /dev/null - (((((./$BASENAME_TEST 2>&1; echo $? >&3) | + ((((( tap_timeout ./$BASENAME_TEST 2>&1; echo $? >&3) | tap_prefix >&4) 3>&1) | (read xs; exit $xs)) 4>>"$logfile" && echo "ok $test_num $TEST_HDR_MSG") || -- 2.11.0

5 years, 9 months

2
1
0 0

[RFC 1/3] rcu: Expedite the rcu quiescent state reporting if help needed

by Joel Fernandes (Google)

The t->rcu_read_unlock_special union's need_qs bit can be set by the scheduler tick (in rcu_flavor_sched_clock_irq) to indicate that help is needed from the rcu_read_unlock path. When this help arrives however, we can do better to speed up the quiescent state reporting which if rcu_read_unlock_special::need_qs is set might be quite urgent. Make use of this information in deciding when to do heavy-weight softirq raising where possible. Signed-off-by: Joel Fernandes (Google) <joel(a)joelfernandes.org> --- kernel/rcu/tree_plugin.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index c588ef98efd3..bff6410fac06 100644 --- a/kernel/rcu/tree_plugin.h +++ b/kernel/rcu/tree_plugin.h @@ -622,7 +622,8 @@ static void rcu_read_unlock_special(struct task_struct *t) t->rcu_read_unlock_special.b.exp_hint = false; exp = (t->rcu_blocked_node && t->rcu_blocked_node->exp_tasks) || (rdp->grpmask & rnp->expmask) || - tick_nohz_full_cpu(rdp->cpu); + tick_nohz_full_cpu(rdp->cpu) || + t->rcu_read_unlock_special.b.need_qs; // Need to defer quiescent state until everything is enabled. if (irqs_were_disabled && use_softirq && (in_interrupt() || -- 2.22.0.410.gd8fdbe21b5-goog

5 years, 9 months

5
11
0 0

[RFC] arm64: Detecting tagged addresses

by Andrew Murray

Hello, The proposed introduction of a relaxed ARM64 ABI [1] will allow tagged memory addresses to be passed through the user-kernel syscall ABI boundary. Tagged memory addresses are those which contain a non-zero top byte (the hardware has always ignored this top byte due to TCR_EL1.TBI0) and may be useful for features such as HWASan. To permit this relaxation a proposed patchset [2] strips the top byte (tag) from user provided memory addresses prior to use in kernel functions which require untagged addresses (for example comparasion/arithmetic of addresses). The author of this patchset relied on a variety of techniques [2] (such as grep, BUG_ON, sparse etc) to identify as many instances of possible where tags need to be stipped. To support this effort and to catch future regressions (e.g. in new syscalls or ioctls), I've devised an additional approach for detecting the use of tagged addresses in functions that do not want them. This approach makes use of Smatch [3] and is outlined in this RFC. Due to the ability of Smatch to do flow analysis I believe we can annotate the kernel in fewer places than a similar approach in sparse. I'm keen for feedback on the likely usefulness of this approach. We first add some new annotations that are exclusively consumed by Smatch: --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -19,6 +19,7 @@ # define __cond_lock(x,c) ((c) ? ({ __acquire(x); 1; }) : 0) # define __percpu __attribute__((noderef, address_space(3))) # define __rcu __attribute__((noderef, address_space(4))) +# define __untagged __attribute__((address_space(5))) # define __private __attribute__((noderef)) extern void __chk_user_ptr(const volatile void __user *); extern void __chk_io_ptr(const volatile void __iomem *); @@ -45,6 +46,7 @@ extern void __chk_io_ptr(const volatile void __iomem *); ... The purpose of this annotation is to indicate in function prototypes that a given argument must not be a user tagged memory address. (The address space number isn't significant here and could be replaced with any other annotation that we get Smatch to understand). An example of how we use this annotation is as follows: --- a/mm/mmap.c +++ b/mm/mmap.c @@ -2224,7 +2224,7 @@ get_unmapped_area(struct file *file, unsigned long addr, unsigned long len, EXPORT_SYMBOL(get_unmapped_area); /* Look up the first VMA which satisfies addr < vm_end, NULL if none. */ -struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr) +struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long __untagged addr) { struct rb_node *rb_node; struct vm_area_struct *vma; Thus our intent here is to flag up whenever addr is tagged. Specifically modifications I've made to Smatch to test this will flag an issue where all the following conditions are met: 1. the parameter is used in the function 2. the data in the parameter has originated or been derived from userspace (this relies on existing Smatch functionality to detect where data has come from) 3. the data's top byte is non-zero (via flow analysis to determine the range of values that it may be given the known call-tree). Due to the use of smatch and its flow-analysis we don't need to propogate the __untagged annotation up the call chain to the callers and their callers - thus allowing us to only annotate functions that actually does something with the address and it's only necessary if the function has the potential to receive user data. I.e. we only need to tag find_vma to catch an issue with mmap_region (because it calls count_vma_pages_range which calls find_vma_intersection which calls find_vma). Due to condition 3 above, the use of the existing untagged_addr macro (or anything that does something similar) will prevent smatch from producing a warning. Using a vanilla (v5.2-rc2) kernel, and a single find_vma annotation, smatch will produce the following warnings: mm/mmap.c:2227 find_vma() warn: Variable addr looks like a tagged address - it is not allowed here mm/mmap.c:2227 find_vma() warn: Variable addr looks like a tagged address - it is not allowed here mm/mmap.c:2227 find_vma() warn: Variable addr looks like a tagged address - it is not allowed here ... The warning is printed for each call site that calls find_vma with a tagged address from userspace. After 6 runs of smatch, 24 warnings are produced. The warnings are helpful in detecting issues, but not useful in providing enough information to debug the issue and find the offending functions higher up the call stack that call find_vma. Smatch is good at determining the ranges of values that can be passed to a function, but it doesn't keep track of how it determined those ranges - this makes it difficult to determine the offending function. However even this level of limited functionality is helpful - as once the kernel is initially sanitized of tagged addresses, then the use of smatch here can spot regressions and offending code identified via git aiaiai/bisect. Smatch builds a database which includes a table of functions, who they are called by and with what range of parameters. Smatch also provides a bunch of perl and python scripts which can be used to extract helpful information for example to produce a call tree for a given function. I've adapted these scripts such that for a given function (e.g. find_vma) it will show you the call tree where callers pass user data and where that data is tagged addresses. The output looks something like this: find_vma() - 0-u64max (1) kvm_arch_prepare_memory_region() - 0-u64max (1) __kvm_set_memory_region() - 0-u64max (1) kvm_set_memory_region() - 0-u64max (1) kvm_vm_ioctl_set_memory_region() - 0-u64max (1) hugetlb_get_unmapped_area() - 0-u64max (1) shm_get_unmapped_area() - 0-u64max (1) shm_get_unmapped_area() - 32785,40977,98321,106513,2097151-u64max[c] (1) ... In summary the following are found, note this currently unhelpfuly includes functions inbetween find_vma and the leaf functions: $ cat find_vma_tree_orig | sed -e 's/^[ \t]*//' | cut -d ' ' -f 1 | sort | uniq call_mmap() check_and_migrate_cma_pages() compat_ipv6_getsockopt() compat_sock_common_getsockopt() compat_tcp_getsockopt() count_vma_pages_range() __do_compat_sys_get_mempolicy() do_get_mempolicy() do_ioctl() do_mincore() do_mlock() do_mmap() do_mmap_pgoff() ... As you can see, this gives a good point in the right direction for hunting down callers of find_vma with tagged addresses. This can be further improved - the problem here is that for a given function, e.g. find_vma we look for callers where *any* of the parameters passed to find_vma are tagged addresses from userspace - i.e. not *just* the annotated parameter. This is also true for find_vma's callers' callers'. This results in the call tree having false positives. It *is* possible to track parameters (e.g. find_vma arg 1 comes from arg 3 of do_pages_stat_array etc), but this is limited as if functions modify the data then the tracking is stopped (however this can be fixed). After apply the patchset ([PATCH v16 00/16] arm64: untag user pointers passed to the kernel)[2] which untags user addresses, smatch indicates 11 issues. The call tree is reduced. After grep'ing the call tree output, there are some valid instances where untagging is needed, e.g: gntdev_ioctl_get_offset_for_vaddr() kvm_vm_ioctl_set_memory_region() privcmd_ioctl_mmap_batch() privcmd_ioctl_mmap_resource() __se_sys_brk() __se_sys_mremap() __se_sys_munmap() __se_sys_remap_file_pages() __se_sys_shmat() __se_sys_shmdt() __vm_munmap() ... An example of a false positve is do_mlock. We untag the address and pass that to apply_vma_lock_flags - however we also pass a length - because the length came from userspace and could have the top bits set - it's flagged. However with improved parameter tracking we can remove this false positive and similar. Prior to smatch I attempted a similar approach with sparse - however it seemed necessary to propogate the __untagged annotation in every function up the call tree, and resulted in adding the __untagged annotation to functions that would never get near user provided data. This leads to a littering of __untagged all over the kernel which doesn't seem appealing. Smatch is more capable, however it almost certainly won't pick up 100% of issues due to the difficulity of making flow analysis understand everything a compiler can. Is it likely to be acceptable to use the __untagged annotation in user-path functions that require untagged addresses across the kernel? Thanks, Andrew Murray [1] https://lkml.org/lkml/2019/6/13/534 [2] https://patchwork.kernel.org/cover/10989517/ [3] http://smatch.sourceforge.net/

5 years, 10 months

3
3
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror