September 2023 - Linux-kselftest-mirror

[PATCH 2/3] Revert "bpf: Fix issue in verifying allow_ptr_leaks"

by Luis Gerhorst

This reverts commit d75e30dddf73449bc2d10bb8e2f1a2c446bc67a2. To mitigate Spectre v1, the verifier relies on static analysis to deduct constant pointer bounds, which can then be enforced by rewriting pointer arithmetic [1] or index masking [2]. This relies on the fact that every memory region to be accessed has a static upper bound and every date below that bound is accessible. The verifier can only rewrite pointer arithmetic or insert masking instructions to mitigate Spectre v1 if a static upper bound, below of which every access is valid, can be given. When allowing packet pointer comparisons, this introduces a way for the program to effectively construct an accessible pointer for which no static upper bound is known. Intuitively, this is obvious as a packet might be of any size and therefore 0 is the only statically known upper bound below of which every date is always accessible (i.e., none). To clarify, the problem is not that comparing two pointers can be used for pointer leaks in the same way in that comparing a pointer to a known scalar can be used for pointer leaks. That is because the "secret" components of the addresses cancel each other out if the pointers are into the same region. With [3] applied, the following malicious BPF program can be loaded into the kernel without CAP_PERFMON: r2 = *(u32 *)(r1 + 76) // data r3 = *(u32 *)(r1 + 80) // data_end r4 = r2 r4 += 1 if r4 > r3 goto exit r5 = *(u8 *)(r2 + 0) // speculatively read secret r5 &= 1 // choose bit to leak // ... side channel to leak secret bit exit: // ... This is jited to the following amd64 code which still contains the gadget: 0: endbr64 4: nopl 0x0(%rax,%rax,1) 9: xchg %ax,%ax b: push %rbp c: mov %rsp,%rbp f: endbr64 13: push %rbx 14: mov 0xc8(%rdi),%rsi // data 1b: mov 0x50(%rdi),%rdx // data_end 1f: mov %rsi,%rcx 22: add $0x1,%rcx 26: cmp %rdx,%rcx 29: ja 0x000000000000003f // branch to mispredict 2b: movzbq 0x0(%rsi),%r8 // speculative load of secret 30: and $0x1,%r8 // choose bit to leak 34: xor %ebx,%ebx 36: cmp %rbx,%r8 39: je 0x000000000000003f // branch based on secret 3b: imul $0x61,%r8,%r8 // leak using port contention side channel 3f: xor %eax,%eax 41: pop %rbx 42: leaveq 43: retq Here I'm using a port contention side channel because storing the secret to the stack causes the verifier to insert an lfence for unrelated reasons (SSB mitigation) which would terminate the speculation. As Daniel already pointed out to me, data_end is even attacker controlled as one could send many packets of sufficient length to train the branch prediction into assuming data_end >= data will never be true. When the attacker then sends a packet with insufficient data, the Spectre v1 gadget leaks the chosen bit of some value that lies behind data_end. To make it clear that the problem is not the pointer comparison but the missing masking instruction, it can be useful to transform the code above into the following equivalent pseudocode: r2 = data r3 = data_end r6 = ... // index to access, constant does not help r7 = data_end - data // only known at runtime, could be [0,PKT_MAX) if !(r6 < r7) goto exit // no masking of index in r6 happens r2 += r6 // addr. to access r5 = *(u8 *)(r2 + 0) // speculatively read secret // ... leak secret as above One idea to resolve this while still allowing for unprivileged packet access would be to always allocate a power of 2 for the packet data and then also pass the respective index mask in the skb structure. The verifier would then have to check that this index mask is always applied to the offset before a packet pointer is dereferenced. This patch does not implement this extension, but only reverts [3]. [1] 979d63d50c0c0f7bc537bf821e056cc9fe5abd38 ("bpf: prevent out of bounds speculation on pointer arithmetic") [2] b2157399cc9898260d6031c5bfe45fe137c1fbe7 ("bpf: prevent out-of-bounds speculation") [3] d75e30dddf73449bc2d10bb8e2f1a2c446bc67a2 ("bpf: Fix issue in verifying allow_ptr_leaks") Reported-by: Daniel Borkmann <daniel(a)iogearbox.net> Signed-off-by: Luis Gerhorst <gerhorst(a)amazon.de> Signed-off-by: Luis Gerhorst <gerhorst(a)cs.fau.de> Acked-by: Hagar Gamal Halim Hemdan <hagarhem(a)amazon.de> Cc: Puranjay Mohan <puranjay12(a)gmail.com> --- kernel/bpf/verifier.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index bb78212fa5b2..b415a81149ed 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -14050,12 +14050,6 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, return -EINVAL; } - /* check src2 operand */ - err = check_reg_arg(env, insn->dst_reg, SRC_OP); - if (err) - return err; - - dst_reg = &regs[insn->dst_reg]; if (BPF_SRC(insn->code) == BPF_X) { if (insn->imm != 0) { verbose(env, "BPF_JMP/JMP32 uses reserved fields\n"); @@ -14067,13 +14061,12 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, if (err) return err; - src_reg = &regs[insn->src_reg]; - if (!(reg_is_pkt_pointer_any(dst_reg) && reg_is_pkt_pointer_any(src_reg)) && - is_pointer_value(env, insn->src_reg)) { + if (is_pointer_value(env, insn->src_reg)) { verbose(env, "R%d pointer comparison prohibited\n", insn->src_reg); return -EACCES; } + src_reg = &regs[insn->src_reg]; } else { if (insn->src_reg != BPF_REG_0) { verbose(env, "BPF_JMP/JMP32 uses reserved fields\n"); @@ -14081,6 +14074,12 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, } } + /* check src2 operand */ + err = check_reg_arg(env, insn->dst_reg, SRC_OP); + if (err) + return err; + + dst_reg = &regs[insn->dst_reg]; is_jmp32 = BPF_CLASS(insn->code) == BPF_JMP32; if (BPF_SRC(insn->code) == BPF_K) { -- 2.40.1 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879

7 months, 3 weeks

5
10
0 0

[PATCH] kunit: string-stream: Allow ERR_PTR to be passed to string_stream_destroy()

by Richard Fitzgerald

Check the stream pointer passed to string_stream_destroy() for IS_ERR_OR_NULL() instead of only NULL. Whatever alloc_string_stream() returns should be safe to pass to string_stream_destroy(), and that will be an ERR_PTR. It's obviously good practise and generally helpful to also check for NULL pointers so that client cleanup code can call string_stream_destroy() unconditionally - which could include pointers that have never been set to anything and so are NULL. Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> --- lib/kunit/string-stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c index a6f3616c2048..54f4fdcbfac8 100644 --- a/lib/kunit/string-stream.c +++ b/lib/kunit/string-stream.c @@ -173,7 +173,7 @@ void string_stream_destroy(struct string_stream *stream) { KUNIT_STATIC_STUB_REDIRECT(string_stream_destroy, stream); - if (!stream) + if (IS_ERR_OR_NULL(stream)) return; string_stream_clear(stream); -- 2.30.2

7 months, 3 weeks

1
0
0 0

[PATCH v2] kunit: debugfs: Handle errors from alloc_string_stream()

by Richard Fitzgerald

In kunit_debugfs_create_suite() give up and skip creating the debugfs file if any of the alloc_string_stream() calls return an error or NULL. Only put a value in the log pointer of kunit_suite and kunit_test if it is a valid pointer to a log. This prevents the potential invalid dereference reported by smatch: lib/kunit/debugfs.c:115 kunit_debugfs_create_suite() error: 'suite->log' dereferencing possible ERR_PTR() lib/kunit/debugfs.c:119 kunit_debugfs_create_suite() error: 'test_case->log' dereferencing possible ERR_PTR() Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> Reported-by: Dan Carpenter <dan.carpenter(a)linaro.org> Fixes: 05e2006ce493 ("kunit: Use string_stream for test log") Reviewed-by: Rae Moar <rmoar(a)google.com> --- Changes from V1: - If the alloc_string_stream() for the suite->log fails just return. Nothing has been created at this point so there's nothing to clean up. - Re-word the explanation of why the log pointers are only set if they point to a valid log. As these changes are trivial I've carried Rae Moar's Reviewed-by from V1. --- lib/kunit/debugfs.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c index 270d185737e6..9d167adfa746 100644 --- a/lib/kunit/debugfs.c +++ b/lib/kunit/debugfs.c @@ -109,14 +109,28 @@ static const struct file_operations debugfs_results_fops = { void kunit_debugfs_create_suite(struct kunit_suite *suite) { struct kunit_case *test_case; + struct string_stream *stream; - /* Allocate logs before creating debugfs representation. */ - suite->log = alloc_string_stream(GFP_KERNEL); - string_stream_set_append_newlines(suite->log, true); + /* + * Allocate logs before creating debugfs representation. + * The suite->log and test_case->log pointer are expected to be NULL + * if there isn't a log, so only set it if the log stream was created + * successfully. + */ + stream = alloc_string_stream(GFP_KERNEL); + if (IS_ERR_OR_NULL(stream)) + return; + + string_stream_set_append_newlines(stream, true); + suite->log = stream; kunit_suite_for_each_test_case(suite, test_case) { - test_case->log = alloc_string_stream(GFP_KERNEL); - string_stream_set_append_newlines(test_case->log, true); + stream = alloc_string_stream(GFP_KERNEL); + if (IS_ERR_OR_NULL(stream)) + goto err; + + string_stream_set_append_newlines(stream, true); + test_case->log = stream; } suite->debugfs = debugfs_create_dir(suite->name, debugfs_rootdir); @@ -124,6 +138,12 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite) debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444, suite->debugfs, suite, &debugfs_results_fops); + return; + +err: + string_stream_destroy(suite->log); + kunit_suite_for_each_test_case(suite, test_case) + string_stream_destroy(test_case->log); } void kunit_debugfs_destroy_suite(struct kunit_suite *suite) -- 2.30.2

7 months, 3 weeks

1
0
0 0

[PATCH 0/3] selftests: timers: Convert some more of the timers tests to KTAP

by Mark Brown

KTAP is the standard output format for selftests, providing a method for systems running the selftests to get results from individual tests rather than just a pass/fail for the test program as a whole. While many of the timers tests use KTAP some have custom output formats, let's convert a few more to KTAP to make them work better in automation. The posix_timers test made use of perror(), I've added a generic helper to kselftest.h for that since it seems like it'll be useful elsewhere. There are more tests that don't use KTAP, several of them just run a single test so don't really benefit from KTAP and there were a couple where the conversion was a bit more complex so I've left them for now. Signed-off-by: Mark Brown <broonie(a)kernel.org> --- Mark Brown (3): kselftest: Add a ksft_perror() helper selftests: timers: Convert posix_timers test to generate KTAP output selftests: timers: Convert nsleep-lat test to generate KTAP output tools/testing/selftests/kselftest.h | 14 +++++ tools/testing/selftests/timers/nsleep-lat.c | 26 ++++----- tools/testing/selftests/timers/posix_timers.c | 81 ++++++++++++++------------- 3 files changed, 67 insertions(+), 54 deletions(-) --- base-commit: 6465e260f48790807eef06b583b38ca9789b6072 change-id: 20230926-ktap-posix-timers-67e978466185 Best regards, -- Mark Brown <broonie(a)kernel.org>

7 months, 3 weeks

2
4
0 0

[PATCH] kunit: debugfs: Handle errors from alloc_string_stream()

by Richard Fitzgerald

In kunit_debugfs_create_suite() give up and skip creating the debugfs file if any of the alloc_string_stream() calls return an error or NULL. Only put a value in the log pointer of kunit_suite and kunit_test if it is a valid pointer to a log. This prevents the potential invalid dereference reported by smatch: lib/kunit/debugfs.c:115 kunit_debugfs_create_suite() error: 'suite->log' dereferencing possible ERR_PTR() lib/kunit/debugfs.c:119 kunit_debugfs_create_suite() error: 'test_case->log' dereferencing possible ERR_PTR() Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> Reported-by: Dan Carpenter <dan.carpenter(a)linaro.org> Fixes: 05e2006ce493 ("kunit: Use string_stream for test log") --- lib/kunit/debugfs.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c index 270d185737e6..73075ca6e88c 100644 --- a/lib/kunit/debugfs.c +++ b/lib/kunit/debugfs.c @@ -109,14 +109,27 @@ static const struct file_operations debugfs_results_fops = { void kunit_debugfs_create_suite(struct kunit_suite *suite) { struct kunit_case *test_case; + struct string_stream *stream; - /* Allocate logs before creating debugfs representation. */ - suite->log = alloc_string_stream(GFP_KERNEL); - string_stream_set_append_newlines(suite->log, true); + /* + * Allocate logs before creating debugfs representation. + * The log pointer must be NULL if there isn't a log so only + * set it if the log stream was created successfully. + */ + stream = alloc_string_stream(GFP_KERNEL); + if (IS_ERR_OR_NULL(stream)) + goto err; + + string_stream_set_append_newlines(stream, true); + suite->log = stream; kunit_suite_for_each_test_case(suite, test_case) { - test_case->log = alloc_string_stream(GFP_KERNEL); - string_stream_set_append_newlines(test_case->log, true); + stream = alloc_string_stream(GFP_KERNEL); + if (IS_ERR_OR_NULL(stream)) + goto err; + + string_stream_set_append_newlines(stream, true); + test_case->log = stream; } suite->debugfs = debugfs_create_dir(suite->name, debugfs_rootdir); @@ -124,6 +137,12 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite) debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444, suite->debugfs, suite, &debugfs_results_fops); + return; + +err: + string_stream_destroy(suite->log); + kunit_suite_for_each_test_case(suite, test_case) + string_stream_destroy(test_case->log); } void kunit_debugfs_destroy_suite(struct kunit_suite *suite) -- 2.30.2

7 months, 3 weeks

3
3
0 0

[PATCH v4 0/4] kunit: Fix some bugs in kunit

by Jinjie Ruan

The test_cases is not freed in kunit_free_suite_set(). And the copy pointer may be moved in kunit_filter_suites(). The filtered_suite and filtered_suite->test_cases allocated in the last kunit_filter_attr_tests() in last inner for loop may be leaked if kunit_filter_suites() fails. If kunit_filter_suites() succeeds, not only copy but also filtered_suite and filtered_suite->test_cases should be freed. Changes in v4: - Make free_suite_set() take a void * for the 4th patch. - Add Suggested-by and Reviewed-by. - Correct the fix tag. Changes in v3: - Update the kfree_at_end() to use kunit_free_suite_set() for 4th patch. - Update the commit message for the 4th patch. Changes in v2: - Add Reviewed-by. - Add the memory leak backtrace for the 4th patch. - Remove the unused func kernel test robot noticed for the 4th patch. - Update the commit message for the 4th patch. Jinjie Ruan (4): kunit: Fix missed memory release in kunit_free_suite_set() kunit: Fix the wrong kfree of copy for kunit_filter_suites() kunit: Fix possible memory leak in kunit_filter_suites() kunit: test: Fix the possible memory leak in executor_test lib/kunit/executor.c | 23 +++++++++++++++++------ lib/kunit/executor_test.c | 36 ++++++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 20 deletions(-) -- 2.34.1

7 months, 3 weeks

2
5
0 0

[RFC 0/3] vfio-pci support pasid attach/detach

by Yi Liu

This adds the pasid attach/detach uAPIs for userspace to attach/detach a PASID of a device to/from a given ioas/hwpt. Only vfio-pci driver is enabled in this series. After this series, PASID-capable devices bound with vfio-pci can report PASID capability to userspace and VM to enable PASID usages like Shared Virtual Addressing (SVA). This series first adds the helpers for pasid attach in vfio core and then add the device cdev ioctls for pasid attach/detach, finally exposes the device PASID capability to user. It depends on iommufd pasid attach/detach series [1]. Complete code can be found at [2] [1] https://lore.kernel.org/linux-iommu/20230926092651.17041-1-yi.l.liu@intel.c… [2] https://github.com/yiliu1765/iommufd/tree/iommufd_pasid Regards, Yi Liu Kevin Tian (1): vfio-iommufd: Support pasid [at|de]tach for physical VFIO devices Yi Liu (2): vfio: Add VFIO_DEVICE_PASID_[AT|DE]TACH_IOMMUFD_PT vfio/pci: Expose PCIe PASID capability to userspace drivers/vfio/device_cdev.c | 45 ++++++++++++++++++++++++ drivers/vfio/iommufd.c | 48 ++++++++++++++++++++++++++ drivers/vfio/pci/vfio_pci.c | 2 ++ drivers/vfio/pci/vfio_pci_config.c | 2 +- drivers/vfio/vfio.h | 4 +++ drivers/vfio/vfio_main.c | 8 +++++ include/linux/vfio.h | 11 ++++++ include/uapi/linux/vfio.h | 55 ++++++++++++++++++++++++++++++ 8 files changed, 174 insertions(+), 1 deletion(-) -- 2.34.1

7 months, 3 weeks

3
6
0 0

[RFC PATCH] kunit: run test suites only after module initialization completes

by Marco Pagani

Commit 2810c1e99867 ("kunit: Fix wild-memory-access bug in kunit_free_suite_set()") causes test suites to run while the test module is still in MODULE_STATE_COMING. In that state, the module is not fully initialized, lacking sysfs, module_memory, args, init function. This behavior can cause all sorts of problems while using fake devices. This RFC patch restores the normal execution flow, waiting for module initialization to complete before running the tests, and uses the refcount to avoid calling kunit_free_suite_set() if load_module() fails. Fixes: 2810c1e99867 ("kunit: Fix wild-memory-access bug in kunit_free_suite_set()") Signed-off-by: Marco Pagani <marpagan(a)redhat.com> --- lib/kunit/test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 421f13981412..242f26ad387a 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -784,13 +784,13 @@ static int kunit_module_notify(struct notifier_block *nb, unsigned long val, switch (val) { case MODULE_STATE_LIVE: + kunit_module_init(mod); break; case MODULE_STATE_GOING: - kunit_module_exit(mod); + if (module_refcount(mod) == -1) + kunit_module_exit(mod); break; case MODULE_STATE_COMING: - kunit_module_init(mod); - break; case MODULE_STATE_UNFORMED: break; } -- 2.41.0

7 months, 3 weeks

1
0
0 0

[PATCH v2 0/9] KVM RISC-V Conditional Operations

by Anup Patel

This series extends KVM RISC-V to allow Guest/VM discover and use conditional operations related ISA extensions (namely XVentanaCondOps and Zicond). To try these patches, use KVMTOOL from riscv_zbx_zicntr_smstateen_condops_v1 branch at: https://github.com/avpatel/kvmtool.git These patches are based upon the latest riscv_kvm_queue and can also be found in the riscv_kvm_condops_v2 branch at: https://github.com/avpatel/linux.git Changes since v1: - Rebased the series on riscv_kvm_queue - Split PATCH1 and PATCH2 of v1 series into two patches - Added separate test configs for XVentanaCondOps and Zicond in PATCH7 of v1 series. Anup Patel (9): dt-bindings: riscv: Add XVentanaCondOps extension entry RISC-V: Detect XVentanaCondOps from ISA string dt-bindings: riscv: Add Zicond extension entry RISC-V: Detect Zicond from ISA string RISC-V: KVM: Allow XVentanaCondOps extension for Guest/VM RISC-V: KVM: Allow Zicond extension for Guest/VM KVM: riscv: selftests: Add senvcfg register to get-reg-list test KVM: riscv: selftests: Add smstateen registers to get-reg-list test KVM: riscv: selftests: Add condops extensions to get-reg-list test .../devicetree/bindings/riscv/extensions.yaml | 13 ++++ arch/riscv/include/asm/hwcap.h | 2 + arch/riscv/include/uapi/asm/kvm.h | 2 + arch/riscv/kernel/cpufeature.c | 2 + arch/riscv/kvm/vcpu_onereg.c | 4 ++ .../selftests/kvm/riscv/get-reg-list.c | 71 +++++++++++++++++++ 6 files changed, 94 insertions(+) -- 2.34.1

7 months, 3 weeks

5
24
0 0

[PATCH] drivers: base: test: Make property entry API test modular

by Geert Uytterhoeven

There is no reason why the KUnit Tests for the property entry API can only be built-in. Add support for building these tests as a loadable module, like is supported by most other tests. Signed-off-by: Geert Uytterhoeven <geert+renesas(a)glider.be> --- drivers/base/test/Kconfig | 4 ++-- drivers/base/test/property-entry-test.c | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/base/test/Kconfig b/drivers/base/test/Kconfig index 9d42051f8f8e715e..5c7fac80611ce8bc 100644 --- a/drivers/base/test/Kconfig +++ b/drivers/base/test/Kconfig @@ -14,6 +14,6 @@ config DM_KUNIT_TEST depends on KUNIT config DRIVER_PE_KUNIT_TEST - bool "KUnit Tests for property entry API" if !KUNIT_ALL_TESTS - depends on KUNIT=y + tristate "KUnit Tests for property entry API" if !KUNIT_ALL_TESTS + depends on KUNIT default KUNIT_ALL_TESTS diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c index dd2b606d76a3f546..a8657eb06f94e934 100644 --- a/drivers/base/test/property-entry-test.c +++ b/drivers/base/test/property-entry-test.c @@ -506,3 +506,7 @@ static struct kunit_suite property_entry_test_suite = { }; kunit_test_suite(property_entry_test_suite); + +MODULE_DESCRIPTION("Test module for the property entry API"); +MODULE_AUTHOR("Dmitry Torokhov <dtor(a)chromium.org>"); +MODULE_LICENSE("GPL"); -- 2.34.1

7 months, 3 weeks

1
0
0 0

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror September 2023