Fix pining maps after reuse map fd by kernel-patches-bot · Pull Request #168 · kernel-patches/bpf

kernel-patches-bot · 2020-10-03T09:01:08Z

Pull request for series with
subject: Fix pining maps after reuse map fd
version: 1
url: https://patchwork.kernel.org/project/bpf/list/?series=359243

kernel-patches-bot · 2020-10-03T09:01:09Z

Master branch: d82a532
series: https://patchwork.kernel.org/project/bpf/list/?series=359243
version: 1

kernel-patches-bot · 2020-10-05T22:59:45Z

Master branch: d82a532
series: https://patchwork.kernel.org/project/bpf/list/?series=359243
version: 1

Previously we forgot to close the map fd if bpf_map_update_elem() failed during map slot init, which will leak map fd. Let's move map slot initialization to new function init_map_slots() to simplify the code. And close the map fd if init slot failed. Reported-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> Acked-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Say a user reuse map fd after creating a map manually and set the pin_path, then load the object via libbpf. In libbpf bpf_object__create_maps(), bpf_object__reuse_map() will return 0 if there is no pinned map in map->pin_path. Then after checking if map fd exist, we should also check if pin_path was set and do bpf_map__pin() instead of continue the loop. Fix it by creating map if fd not exist and continue checking pin_path after that. Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> Acked-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

This add a test to make sure that we can still pin maps with reused map fd. Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

kernel-patches-bot · 2020-10-06T02:21:20Z

Master branch: d82a532
series: https://patchwork.kernel.org/project/bpf/list/?series=360083
version: 1

kernel-patches-bot · 2020-10-06T18:43:33Z

At least one diff in series https://patchwork.kernel.org/project/bpf/list/?series=360083 irrelevant now. Closing PR.

The test case creates 4 threads and then pins these 4 threads in CPU 0. These 4 threads will run different bpf program through bpf_prog_test_run_opts() and these bpf program will use bpf_obj_new() and bpf_obj_drop() to allocate and free local kptrs concurrently. Under preemptible kernel, bpf_obj_new() and bpf_obj_drop() may preempt each other, bpf_obj_new() may return NULL and the test will fail before the fixes are applied as shown below: test_preempted_bpf_ma_op:PASS:open_and_load 0 nsec test_preempted_bpf_ma_op:PASS:attach 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:FAIL:ENOMEM unexpected ENOMEM: got TRUE #168 preempted_bpf_ma_op:FAIL Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED Signed-off-by: Hou Tao <houtao1@huawei.com>

The test case creates 4 threads and then pins these 4 threads in CPU 0. These 4 threads will run different bpf program through bpf_prog_test_run_opts() and these bpf program will use bpf_obj_new() and bpf_obj_drop() to allocate and free local kptrs concurrently. Under preemptible kernel, bpf_obj_new() and bpf_obj_drop() may preempt each other, bpf_obj_new() may return NULL and the test will fail before applying these fixes as shown below: test_preempted_bpf_ma_op:PASS:open_and_load 0 nsec test_preempted_bpf_ma_op:PASS:attach 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:FAIL:ENOMEM unexpected ENOMEM: got TRUE #168 preempted_bpf_ma_op:FAIL Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED Signed-off-by: Hou Tao <houtao1@huawei.com>

The test case creates 4 threads and then pins these 4 threads in CPU 0. These 4 threads will run different bpf program through bpf_prog_test_run_opts() and these bpf program will use bpf_obj_new() and bpf_obj_drop() to allocate and free local kptrs concurrently. Under preemptible kernel, bpf_obj_new() and bpf_obj_drop() may preempt each other, bpf_obj_new() may return NULL and the test will fail before applying these fixes as shown below: test_preempted_bpf_ma_op:PASS:open_and_load 0 nsec test_preempted_bpf_ma_op:PASS:attach 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:no test prog 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:pthread_create 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:PASS:run prog err 0 nsec test_preempted_bpf_ma_op:FAIL:ENOMEM unexpected ENOMEM: got TRUE #168 preempted_bpf_ma_op:FAIL Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED Signed-off-by: Hou Tao <houtao1@huawei.com> Link: https://lore.kernel.org/r/20230901111954.1804721-4-houtao@huaweicloud.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>

There are still some "ENOTSUPP" (-524) errors left when running BPF selftests on a Loongarch platform since ASSERT_GE() are used there to check the return values, not ASSERT_OK(): ''' test_bpf_cookie:PASS:skel_open 0 nsec #17/1 bpf_cookie/kprobe:OK #17/2 bpf_cookie/multi_kprobe_link_api:OK #17/3 bpf_cookie/multi_kprobe_attach_api:OK #17/4 bpf_cookie/uprobe:OK #17/5 bpf_cookie/multi_uprobe_attach_api:OK #17/6 bpf_cookie/tracepoint:OK #17/7 bpf_cookie/perf_event:OK tracing_subtest:FAIL:fentry.link_create unexpected fentry.link_create: \ actual -524 < expected 0 #17/8 bpf_cookie/trampoline:FAIL lsm_subtest:FAIL:lsm.link_create unexpected lsm.link_create: \ actual -524 < expected 0 #17/9 bpf_cookie/lsm:FAIL #17/10 bpf_cookie/tp_btf:OK #17/11 bpf_cookie/raw_tp:OK #17 bpf_cookie:FAIL ... ... test_module_fentry_shadow:PASS:load_vmlinux_btf 0 nsec test_module_fentry_shadow:PASS:get_bpf_testmod_btf_fd 0 nsec test_module_fentry_shadow:PASS:btf_get_from_fd 0 nsec test_module_fentry_shadow:PASS:btf_find_by_name 0 nsec test_module_fentry_shadow:PASS:btf_find_by_name 0 nsec test_module_fentry_shadow:PASS:bpf_prog_load 0 nsec test_module_fentry_shadow:FAIL:bpf_link_create unexpected \ bpf_link_create: actual -524 < expected 0 #168 module_fentry_shadow:FAIL ''' Just like in ASSERT_OK(), this patch skips ENOTSUPP (524) and ENOTSUP (95) in ASSERT_GT() too. With this change, the new output of these selftests look like: ''' #17/1 bpf_cookie/kprobe:OK #17/2 bpf_cookie/multi_kprobe_link_api:OK #17/3 bpf_cookie/multi_kprobe_attach_api:OK #17/4 bpf_cookie/uprobe:OK #17/5 bpf_cookie/multi_uprobe_attach_api:OK #17/6 bpf_cookie/tracepoint:OK #17/7 bpf_cookie/perf_event:OK #17/8 bpf_cookie/trampoline:SKIP #17/9 bpf_cookie/lsm:SKIP #17/10 bpf_cookie/tp_btf:SKIP #17/11 bpf_cookie/raw_tp:SKIP #17 bpf_cookie:OK (SKIP: 4/11) ... ... #168 module_fentry_shadow:SKIP ''' Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>

There are still some "ENOTSUPP" (-524) errors left when running BPF selftests on a Loongarch platform since ASSERT_GE() are used there to check the return values, not ASSERT_OK(): ''' test_bpf_cookie:PASS:skel_open 0 nsec #17/1 bpf_cookie/kprobe:OK #17/2 bpf_cookie/multi_kprobe_link_api:OK #17/3 bpf_cookie/multi_kprobe_attach_api:OK #17/4 bpf_cookie/uprobe:OK #17/5 bpf_cookie/multi_uprobe_attach_api:OK #17/6 bpf_cookie/tracepoint:OK #17/7 bpf_cookie/perf_event:OK tracing_subtest:FAIL:fentry.link_create unexpected fentry.link_create: \ actual -524 < expected 0 #17/8 bpf_cookie/trampoline:FAIL lsm_subtest:FAIL:lsm.link_create unexpected lsm.link_create: \ actual -524 < expected 0 #17/9 bpf_cookie/lsm:FAIL #17/10 bpf_cookie/tp_btf:OK #17/11 bpf_cookie/raw_tp:OK #17 bpf_cookie:FAIL ... ... test_module_fentry_shadow:PASS:load_vmlinux_btf 0 nsec test_module_fentry_shadow:PASS:get_bpf_testmod_btf_fd 0 nsec test_module_fentry_shadow:PASS:btf_get_from_fd 0 nsec test_module_fentry_shadow:PASS:btf_find_by_name 0 nsec test_module_fentry_shadow:PASS:btf_find_by_name 0 nsec test_module_fentry_shadow:PASS:bpf_prog_load 0 nsec test_module_fentry_shadow:FAIL:bpf_link_create unexpected \ bpf_link_create: actual -524 < expected 0 #168 module_fentry_shadow:FAIL ''' Just like in ASSERT_OK(), this patch uses test_progs_get_error() helper to skip ENOTSUPP (524) and ENOTSUP (95) in ASSERT_GT() too. With this change, the new output of these selftests look like: ''' #17/1 bpf_cookie/kprobe:OK #17/2 bpf_cookie/multi_kprobe_link_api:OK #17/3 bpf_cookie/multi_kprobe_attach_api:OK #17/4 bpf_cookie/uprobe:OK #17/5 bpf_cookie/multi_uprobe_attach_api:OK #17/6 bpf_cookie/tracepoint:OK #17/7 bpf_cookie/perf_event:OK #17/8 bpf_cookie/trampoline:SKIP #17/9 bpf_cookie/lsm:SKIP #17/10 bpf_cookie/tp_btf:SKIP #17/11 bpf_cookie/raw_tp:SKIP #17 bpf_cookie:OK (SKIP: 4/11) ... ... #168 module_fentry_shadow:SKIP ''' Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>

When loading guest XSAVE state via KVM_SET_XSAVE, and when updating XFD in response to a guest WRMSR, clear XFD-disabled features in the saved (or to be restored) XSTATE_BV to ensure KVM doesn't attempt to load state for features that are disabled via the guest's XFD. Because the kernel executes XRSTOR with the guest's XFD, saving XSTATE_BV[i]=1 with XFD[i]=1 will cause XRSTOR to #NM and panic the kernel. E.g. if fpu_update_guest_xfd() sets XFD without clearing XSTATE_BV: ------------[ cut here ]------------ WARNING: arch/x86/kernel/traps.c:1524 at exc_device_not_available+0x101/0x110, CPU#29: amx_test/848 Modules linked in: kvm_intel kvm irqbypass CPU: 29 UID: 1000 PID: 848 Comm: amx_test Not tainted 6.19.0-rc2-ffa07f7fd437-x86_amx_nm_xfd_non_init-vm #171 NONE Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:exc_device_not_available+0x101/0x110 Call Trace: <TASK> asm_exc_device_not_available+0x1a/0x20 RIP: 0010:restore_fpregs_from_fpstate+0x36/0x90 switch_fpu_return+0x4a/0xb0 kvm_arch_vcpu_ioctl_run+0x1245/0x1e40 [kvm] kvm_vcpu_ioctl+0x2c3/0x8f0 [kvm] __x64_sys_ioctl+0x8f/0xd0 do_syscall_64+0x62/0x940 entry_SYSCALL_64_after_hwframe+0x4b/0x53 </TASK> ---[ end trace 0000000000000000 ]--- This can happen if the guest executes WRMSR(MSR_IA32_XFD) to set XFD[18] = 1, and a host IRQ triggers kernel_fpu_begin() prior to the vmexit handler's call to fpu_update_guest_xfd(). and if userspace stuffs XSTATE_BV[i]=1 via KVM_SET_XSAVE: ------------[ cut here ]------------ WARNING: arch/x86/kernel/traps.c:1524 at exc_device_not_available+0x101/0x110, CPU#14: amx_test/867 Modules linked in: kvm_intel kvm irqbypass CPU: 14 UID: 1000 PID: 867 Comm: amx_test Not tainted 6.19.0-rc2-2dace9faccd6-x86_amx_nm_xfd_non_init-vm #168 NONE Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:exc_device_not_available+0x101/0x110 Call Trace: <TASK> asm_exc_device_not_available+0x1a/0x20 RIP: 0010:restore_fpregs_from_fpstate+0x36/0x90 fpu_swap_kvm_fpstate+0x6b/0x120 kvm_load_guest_fpu+0x30/0x80 [kvm] kvm_arch_vcpu_ioctl_run+0x85/0x1e40 [kvm] kvm_vcpu_ioctl+0x2c3/0x8f0 [kvm] __x64_sys_ioctl+0x8f/0xd0 do_syscall_64+0x62/0x940 entry_SYSCALL_64_after_hwframe+0x4b/0x53 </TASK> ---[ end trace 0000000000000000 ]--- The new behavior is consistent with the AMX architecture. Per Intel's SDM, XSAVE saves XSTATE_BV as '0' for components that are disabled via XFD (and non-compacted XSAVE saves the initial configuration of the state component): If XSAVE, XSAVEC, XSAVEOPT, or XSAVES is saving the state component i, the instruction does not generate #NM when XCR0[i] = IA32_XFD[i] = 1; instead, it operates as if XINUSE[i] = 0 (and the state component was in its initial state): it saves bit i of XSTATE_BV field of the XSAVE header as 0; in addition, XSAVE saves the initial configuration of the state component (the other instructions do not save state component i). Alternatively, KVM could always do XRSTOR with XFD=0, e.g. by using a constant XFD based on the set of enabled features when XSAVEing for a struct fpu_guest. However, having XSTATE_BV[i]=1 for XFD-disabled features can only happen in the above interrupt case, or in similar scenarios involving preemption on preemptible kernels, because fpu_swap_kvm_fpstate()'s call to save_fpregs_to_fpstate() saves the outgoing FPU state with the current XFD; and that is (on all but the first WRMSR to XFD) the guest XFD. Therefore, XFD can only go out of sync with XSTATE_BV in the above interrupt case, or in similar scenarios involving preemption on preemptible kernels, and it we can consider it (de facto) part of KVM ABI that KVM_GET_XSAVE returns XSTATE_BV[i]=0 for XFD-disabled features. Reported-by: Paolo Bonzini <pbonzini@redhat.com> Cc: stable@vger.kernel.org Fixes: 820a6ee ("kvm: x86: Add emulation for IA32_XFD", 2022-01-14) Signed-off-by: Sean Christopherson <seanjc@google.com> [Move clearing of XSTATE_BV from fpu_copy_uabi_to_guest_fpstate to kvm_vcpu_ioctl_x86_set_xsave. - Paolo] Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

kernel-patches-bot added bpf new V1 labels Oct 3, 2020

kernel-patches-bot force-pushed the series/359243=>bpf branch from 5b565cc to d18d9ac Compare October 5, 2020 22:59

kernel-patches-bot and others added 4 commits October 5, 2020 19:21

adding ci files

d5c6cd8

selftest/bpf: test pinning map with reused map fd

09cd53f

This add a test to make sure that we can still pin maps with reused map fd. Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

kernel-patches-bot force-pushed the series/359243=>bpf branch from d18d9ac to 09cd53f Compare October 6, 2020 02:21

kernel-patches-bot added accepted and removed new labels Oct 6, 2020

kernel-patches-bot closed this Oct 6, 2020

kernel-patches-bot deleted the series/359243=>bpf branch October 7, 2020 01:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix pining maps after reuse map fd#168

Fix pining maps after reuse map fd#168
kernel-patches-bot wants to merge 4 commits intobpffrom
series/359243=>bpf

kernel-patches-bot commented Oct 3, 2020

Uh oh!

kernel-patches-bot commented Oct 3, 2020

Uh oh!

kernel-patches-bot commented Oct 5, 2020

Uh oh!

kernel-patches-bot commented Oct 6, 2020

Uh oh!

kernel-patches-bot commented Oct 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kernel-patches-bot commented Oct 3, 2020

Uh oh!

kernel-patches-bot commented Oct 3, 2020

Uh oh!

kernel-patches-bot commented Oct 5, 2020

Uh oh!

kernel-patches-bot commented Oct 6, 2020

Uh oh!

kernel-patches-bot commented Oct 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants