On 3/19/21 7:47 PM, Alexei Starovoitov wrote:
On Thu, Mar 18, 2021 at 08:12:36PM -0700, Yonghong Song wrote:
-static inline void bpf_cgroup_storage_set(struct bpf_cgroup_storage
- *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
+static inline int bpf_cgroup_storage_set(struct bpf_cgroup_storage
+ *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
{
enum bpf_cgroup_storage_type stype;
+ int i;
+
+ preempt_disable();
+ for (i = 0; i < BPF_CGROUP_STORAGE_NEST_MAX; i++) {
+ if (unlikely(this_cpu_read(bpf_cgroup_storage_info[i].task) != NULL))
+ continue;
+
+ this_cpu_write(bpf_cgroup_storage_info[i].task, current);
+ for_each_cgroup_storage_type(stype)
+ this_cpu_write(bpf_cgroup_storage_info[i].storage[stype],
+ storage[stype]);
+ break;
+ }
+ preempt_enable();
+
+ if (i == BPF_CGROUP_STORAGE_NEST_MAX) {
+ WARN_ON_ONCE(1);
+ return -EBUSY;
+ }
+ return 0;
The extra 'if' probably will be optimized by the compiler,
but could you write it like this instead:
+ int err = 0;
..
+ for_each_cgroup_storage_type(stype)
+ this_cpu_write(bpf_cgroup_storage_info[i].storage[stype],
+ storage[stype]);
+ goto out;
+ }
+ err = -EBUSY;
+ WARN_ON_ONCE(1);
+ out:
+ preempt_enable();
+ return err;
okay.
Also patch 2 should be squashed into patch 1,
since patch 1 alone makes bpf_prog_test_run() broken.
(The WARN_ON_ONCE should trigger right away on test_run without patch 2).
You are right. Will fold this into one patch. My original intention is
to apply patch 1 to bpf tree. Looks like folding one patch is necessary.
We can create a different patch for bpf tree if needed.
Another nit:
Is title of the patch "fix NULL pointer dereference" actually correct?
It surely was correct before accidental tracing overwrite was fixed.
But the fix is already in bpf tree.
Do you still see it as NULL deref with that 3 min reproducer?
Yes, I do. I just double checked and run again with latest bpf-next +
bpf_local_storage kprobe/tracepoint fix.
with gcc 8.4.1, kasan is enabled. I hit
[ 806.571378] BUG: KASAN: null-ptr-deref in
bpf_get_local_storage+0x29/0x70
[ 806.572393] Read of size 8 at addr 0000000000000000 by task
test_progs/16069
[ 806.573487]
[ 806.573747] CPU: 1 PID: 16069 Comm: test_progs Tainted: G O
5.12.0-rc2+ #59
[ 806.574964] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[ 806.576627] Call Trace:
[ 806.577045] dump_stack+0xa4/0xe5
[ 806.577572] ? bpf_get_local_storage+0x29/0x70
[ 806.578257] ? bpf_get_local_storage+0x29/0x70
[ 806.578929] kasan_report.cold.13+0x5f/0xd8
[ 806.579595] ? bpf_get_local_storage+0x29/0x70
[ 806.580300] bpf_get_local_storage+0x29/0x70
[ 806.580970]
bpf_prog_b06a218bf1bb5278_bpf_sping_lock_test+0x2af/0xdc8
[ 806.581976] bpf_test_run+0x268/0x420
[ 806.582602] ? bpf_test_timer_continue+0x1c0/0x1c0
[ 806.583338] ? __build_skb+0x20/0x50
[ 806.583871] ? rcu_read_lock_sched_held+0xa1/0xd0
[ 806.584562] ? rcu_read_lock_bh_held+0xb0/0xb0
[ 806.585235] ? static_obj+0x32/0x80
[ 806.585801] ? eth_gro_receive+0x3b0/0x3b0
[ 806.586440] ? __build_skb+0x45/0x50
[ 806.587006] bpf_prog_test_run_skb+0x69c/0xc10
[ 806.587721] ? bpf_prog_test_run_raw_tp+0x2e0/0x2e0
[ 806.588500] ? fput_many+0x1a/0xc0
[ 806.589052] __do_sys_bpf+0x1025/0x2d30
[ 806.589637] ? check_chain_key+0x1ea/0x2f0
[ 806.590277] ? bpf_link_get_from_fd+0x80/0x80
[ 806.590974] ? __lock_acquire+0x921/0x2f80
[ 806.591633] ? register_lock_class+0x950/0x950
[ 806.592354] ? pvclock_clocksource_read+0xdc/0x180
[ 806.593160] ? rcu_read_lock_sched_held+0xa1/0xd0
[ 806.593940] ? syscall_enter_from_user_mode+0x1c/0x40
[ 806.594696] do_syscall_64+0x33/0x40
[ 806.595244] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 806.594696] do_syscall_64+0x33/0x40
[ 806.595244] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 806.596044] RIP: 0033:0x7f7a155e67f9
[ 806.596599] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00
48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f
05 <48> 3d 01 f0 ff ff 73 01 c3 488
[ 806.599404] RSP: 002b:00007f7a144ffe68 EFLAGS: 00000202 ORIG_RAX:
0000000000000141
[ 806.600552] RAX: ffffffffffffffda RBX: 00007f7a144fff2c RCX:
00007f7a155e67f9
[ 806.601627] RDX: 0000000000000078 RSI: 00007f7a144ffe70 RDI:
000000000000000a
[ 806.602694] RBP: 00007f7a144fff28 R08: 0000000000000000 R09:
0000000000000008
[ 806.603795] R10: 0000000000000000 R11: 0000000000000202 R12:
0000000000000000
[ 806.604833] R13: 00007ffd8b062d9f R14: 0000000000000003 R15:
0000000000000000
