-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpf: replace usage of supported with dedicated list iterator variable #229
Conversation
Master branch: d717e4c |
Master branch: 2975dbd |
c0e402d
to
f52ad9c
Compare
Master branch: 38608ee |
f52ad9c
to
6ed7ef7
Compare
Master branch: 11e17ae |
6ed7ef7
to
994c377
Compare
Master branch: fe4625d |
994c377
to
86887c8
Compare
Master branch: 8eb943f |
86887c8
to
99291e8
Compare
Master branch: 891663a |
99291e8
to
49d3c8f
Compare
Master branch: 891663a |
49d3c8f
to
c713178
Compare
Master branch: 891663a |
To move the list iterator variable into the list_for_each_entry_*() macro in the future it should be avoided to use the list iterator variable after the loop body. To *never* use the list iterator variable after the loop it was concluded to use a separate iterator variable instead of a found boolean [1]. This removes the need to use the found variable (existed & supported) and simply checking if the variable was set, can determine if the break/goto was hit. [1] https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ Acked-by: Yonghong Song <[email protected]> Signed-off-by: Jakob Koschel <[email protected]>
c713178
to
26faa71
Compare
At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=627769 irrelevant now. Closing PR. |
When generate a synthetic event with many params and then create a trace action for it [1], kernel panic happened [2]. It is because that in trace_action_create() 'data->n_params' is up to SYNTH_FIELDS_MAX (current value is 64), and array 'data->var_ref_idx' keeps indices into array 'hist_data->var_refs' for each synthetic event param, but the length of 'data->var_ref_idx' is TRACING_MAP_VARS_MAX (current value is 16), so out-of-bound write happened when 'data->n_params' more than 16. In this case, 'data->match_data.event' is overwritten and eventually cause the panic. To solve the issue, adjust the length of 'data->var_ref_idx' to be SYNTH_FIELDS_MAX and add sanity checks to avoid out-of-bound write. [1] # cd /sys/kernel/tracing/ # echo "my_synth_event int v1; int v2; int v3; int v4; int v5; int v6;\ int v7; int v8; int v9; int v10; int v11; int v12; int v13; int v14;\ int v15; int v16; int v17; int v18; int v19; int v20; int v21; int v22;\ int v23; int v24; int v25; int v26; int v27; int v28; int v29; int v30;\ int v31; int v32; int v33; int v34; int v35; int v36; int v37; int v38;\ int v39; int v40; int v41; int v42; int v43; int v44; int v45; int v46;\ int v47; int v48; int v49; int v50; int v51; int v52; int v53; int v54;\ int v55; int v56; int v57; int v58; int v59; int v60; int v61; int v62;\ int v63" >> synthetic_events # echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="bash"' >> \ events/sched/sched_waking/trigger # echo "hist:keys=next_pid:onmatch(sched.sched_waking).my_synth_event(\ pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,\ pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,\ pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,pid,\ pid,pid,pid,pid,pid,pid,pid,pid,pid)" >> events/sched/sched_switch/trigger [2] BUG: unable to handle page fault for address: ffff91c900000000 PGD 61001067 P4D 61001067 PUD 0 Oops: 0000 [#1] PREEMPT SMP NOPTI CPU: 2 PID: 322 Comm: bash Tainted: G W 6.1.0-rc8+ #229 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014 RIP: 0010:strcmp+0xc/0x30 Code: 75 f7 31 d2 44 0f b6 04 16 44 88 04 11 48 83 c2 01 45 84 c0 75 ee c3 cc cc cc cc 0f 1f 00 31 c0 eb 08 48 83 c0 01 84 d2 74 13 <0f> b6 14 07 3a 14 06 74 ef 19 c0 83 c8 01 c3 cc cc cc cc 31 c3 RSP: 0018:ffff9b3b00f53c48 EFLAGS: 00000246 RAX: 0000000000000000 RBX: ffffffffba958a68 RCX: 0000000000000000 RDX: 0000000000000010 RSI: ffff91c943d33a90 RDI: ffff91c900000000 RBP: ffff91c900000000 R08: 00000018d604b529 R09: 0000000000000000 R10: ffff91c9483eddb1 R11: ffff91ca483eddab R12: ffff91c946171580 R13: ffff91c9479f0538 R14: ffff91c9457c2848 R15: ffff91c9479f0538 FS: 00007f1d1cfbe740(0000) GS:ffff91c9bdc80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff91c900000000 CR3: 0000000006316000 CR4: 00000000000006e0 Call Trace: <TASK> __find_event_file+0x55/0x90 action_create+0x76c/0x1060 event_hist_trigger_parse+0x146d/0x2060 ? event_trigger_write+0x31/0xd0 trigger_process_regex+0xbb/0x110 event_trigger_write+0x6b/0xd0 vfs_write+0xc8/0x3e0 ? alloc_fd+0xc0/0x160 ? preempt_count_add+0x4d/0xa0 ? preempt_count_add+0x70/0xa0 ksys_write+0x5f/0xe0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7f1d1d0cf077 Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 RSP: 002b:00007ffcebb0e568 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000143 RCX: 00007f1d1d0cf077 RDX: 0000000000000143 RSI: 00005639265aa7e0 RDI: 0000000000000001 RBP: 00005639265aa7e0 R08: 000000000000000a R09: 0000000000000142 R10: 000056392639c017 R11: 0000000000000246 R12: 0000000000000143 R13: 00007f1d1d1ae6a0 R14: 00007f1d1d1aa4a0 R15: 00007f1d1d1a98a0 </TASK> Modules linked in: CR2: ffff91c900000000 ---[ end trace 0000000000000000 ]--- RIP: 0010:strcmp+0xc/0x30 Code: 75 f7 31 d2 44 0f b6 04 16 44 88 04 11 48 83 c2 01 45 84 c0 75 ee c3 cc cc cc cc 0f 1f 00 31 c0 eb 08 48 83 c0 01 84 d2 74 13 <0f> b6 14 07 3a 14 06 74 ef 19 c0 83 c8 01 c3 cc cc cc cc 31 c3 RSP: 0018:ffff9b3b00f53c48 EFLAGS: 00000246 RAX: 0000000000000000 RBX: ffffffffba958a68 RCX: 0000000000000000 RDX: 0000000000000010 RSI: ffff91c943d33a90 RDI: ffff91c900000000 RBP: ffff91c900000000 R08: 00000018d604b529 R09: 0000000000000000 R10: ffff91c9483eddb1 R11: ffff91ca483eddab R12: ffff91c946171580 R13: ffff91c9479f0538 R14: ffff91c9457c2848 R15: ffff91c9479f0538 FS: 00007f1d1cfbe740(0000) GS:ffff91c9bdc80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff91c900000000 CR3: 0000000006316000 CR4: 00000000000006e0 Link: https://lore.kernel.org/linux-trace-kernel/[email protected] Cc: <[email protected]> Cc: <[email protected]> Cc: [email protected] Fixes: d380dcd ("tracing: Fix now invalid var_ref_vals assumption in trace action") Signed-off-by: Zheng Yejian <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
Chris pointed out that some bonehead, *cough* me *cough*, added two mutex_locks() to the SiFive errata patching. The second was meant to have been a mutex_unlock(). This results in errors such as Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030 Oops [#1] Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 6.2.0-rc1-starlight-00079-g9493e6f3ce02 #229 Hardware name: BeagleV Starlight Beta (DT) epc : __schedule+0x42/0x500 ra : schedule+0x46/0xce epc : ffffffff8065957c ra : ffffffff80659a80 sp : ffffffff81203c80 gp : ffffffff812d50a0 tp : ffffffff8120db40 t0 : ffffffff81203d68 t1 : 0000000000000001 t2 : 4c45203a76637369 s0 : ffffffff81203cf0 s1 : ffffffff8120db40 a0 : 0000000000000000 a1 : ffffffff81213958 a2 : ffffffff81213958 a3 : 0000000000000000 a4 : 0000000000000000 a5 : ffffffff80a1bd00 a6 : 0000000000000000 a7 : 0000000052464e43 s2 : ffffffff8120db41 s3 : ffffffff80a1ad00 s4 : 0000000000000000 s5 : 0000000000000002 s6 : ffffffff81213938 s7 : 0000000000000000 s8 : 0000000000000000 s9 : 0000000000000001 s10: ffffffff812d7204 s11: ffffffff80d3c920 t3 : 0000000000000001 t4 : ffffffff812e6dd7 t5 : ffffffff812e6dd8 t6 : ffffffff81203bb8 status: 0000000200000100 badaddr: 0000000000000030 cause: 000000000000000d [<ffffffff80659a80>] schedule+0x46/0xce [<ffffffff80659dce>] schedule_preempt_disabled+0x16/0x28 [<ffffffff8065ae0c>] __mutex_lock.constprop.0+0x3fe/0x652 [<ffffffff8065b138>] __mutex_lock_slowpath+0xe/0x16 [<ffffffff8065b182>] mutex_lock+0x42/0x4c [<ffffffff8000ad94>] sifive_errata_patch_func+0xf6/0x18c [<ffffffff80002b92>] _apply_alternatives+0x74/0x76 [<ffffffff80802ee8>] apply_boot_alternatives+0x3c/0xfa [<ffffffff80803cb0>] setup_arch+0x60c/0x640 [<ffffffff80800926>] start_kernel+0x8e/0x99c ---[ end trace 0000000000000000 ]--- Reported-by: Chris Hofstaedtler <[email protected]> Fixes: 9493e6f ("RISC-V: take text_mutex during alternative patching") Signed-off-by: Conor Dooley <[email protected]> Tested-by: Geert Uytterhoeven <[email protected]> Link: https://lore.kernel.org/r/[email protected] [Palmer: pick up Geert's bug report from the thread] Signed-off-by: Palmer Dabbelt <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #224 tc_links_after:OK #225 tc_links_append:OK #226 tc_links_basic:OK #227 tc_links_before:OK #228 tc_links_both:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_first:OK #232 tc_links_invalid:OK #233 tc_links_last:OK #234 tc_links_prepend:OK #235 tc_links_replace:OK #236 tc_links_revision:OK Summary: 13/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #224 tc_links_after:OK #225 tc_links_append:OK #226 tc_links_basic:OK #227 tc_links_before:OK #228 tc_links_both:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_first:OK #232 tc_links_invalid:OK #233 tc_links_last:OK #234 tc_links_prepend:OK #235 tc_links_replace:OK #236 tc_links_revision:OK Summary: 13/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
Add a big batch of test coverage to assert all aspects of the tcx link API: # ./vmtest.sh -- ./test_progs -t tc_links [...] #225 tc_links_after:OK #226 tc_links_append:OK #227 tc_links_basic:OK #228 tc_links_before:OK #229 tc_links_chain_classic:OK #230 tc_links_dev_cleanup:OK #231 tc_links_invalid:OK #232 tc_links_prepend:OK #233 tc_links_replace:OK #234 tc_links_revision:OK Summary: 10/0 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
Pull request for series with
subject: bpf: replace usage of supported with dedicated list iterator variable
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=627769