bpf: Fix trampoline usage in preempt
Subsystems (4)
Alexei Starovoitov
maintainerPATCH 1/1 bpf: Fix trampoline usage in preempt
Though the second half of trampoline page is unused a task could be preempted in the middle of the first half of trampoline and two updates to trampoline would change the code from underneath the preempted task. Hence wait for tasks to voluntarily schedule or go to userspace.
Add similar wait before freeing the trampoline.
--- kernel/bpf/trampoline.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
kernel/bpf/trampoline.c +10 -0
@@ -160,6 +160,14 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr)if (fexit_cnt) flags =BPF_TRAMP_F_CALL_ORIG |BPF_TRAMP_F_SKIP_FRAME ; +/* Though the second half of trampoline page is unused a task could be +* preempted in the middle of the first half of trampoline and two +* updates to trampoline would change the code from underneath the +* preempted task. Hence wait for tasks to voluntarily schedule or go +* to userspace. +*/ +synchronize_rcu_tasks (); + err = arch_prepare_bpf_trampoline(new_image, new_image + PAGE_SIZE / 2, &tr->func.model, flags, fentry, fentry_cnt,
@@ -251,6 +259,8 @@ void bpf_trampoline_put(struct bpf_trampoline *tr) goto out;if (WARN_ON_ONCE (!hlist_empty (&tr->progs_hlist[BPF_TRAMP_FEXIT ]))) goto out; +/* wait for tasks to get out of trampoline before freeing it */ +synchronize_rcu_tasks ();bpf_jit_free_exec (tr->image);hlist_del (&tr->hlist);kfree (tr);
Signature · 1 line
2.23.0
Quoted 19 lines · Alexei Starovoitov
Though the second half of trampoline page is unused a task could be preempted in the middle of the first half of trampoline and two updates to trampoline would change the code from underneath the preempted task. Hence wait for tasks to voluntarily schedule or go to userspace.
Add similar wait before freeing the trampoline.Fixes fec56f5890d9 ("bpf: Introduce BPF trampoline")Reported-by Jann Horn <[email protected]>Signed-off-by Alexei Starovoitov <[email protected]>--- kernel/bpf/trampoline.c | 10 ++++++++++ 1 file changed, 10 insertions(+)kernel/bpf/trampoline.c +0
-0
kernel/bpf/trampoline.c static int bpf_trampoline_update(struct bpf_trampoline *tr) · line 169
Quoted 10 lines
kernel/bpf/trampoline.c +7
-0@@ -160,6 +160,14 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr)if (fexit_cnt) flags =BPF_TRAMP_F_CALL_ORIG |BPF_TRAMP_F_SKIP_FRAME ; +/* Though the second half of trampoline page is unused a task could be +* preempted in the middle of the first half of trampoline and two +* updates to trampoline would change the code from underneath the +* preempted task. Hence wait for tasks to voluntarily schedule or go +* to userspace. +*/ +synchronize_rcu_tasks ();
So in this case, although the trampoline is not freed, it is reused. And we need to clear everyone off of the trampoline before reusing it.
If this states the situation correctly:
Quoted 15 lines
kernel/bpf/trampoline.c +2
-0err = arch_prepare_bpf_trampoline(new_image, new_image + PAGE_SIZE / 2, &tr->func.model, flags, fentry, fentry_cnt,@@ -251,6 +259,8 @@ void bpf_trampoline_put(struct bpf_trampoline *tr) goto out;if (WARN_ON_ONCE (!hlist_empty (&tr->progs_hlist[BPF_TRAMP_FEXIT ]))) goto out; +/* wait for tasks to get out of trampoline before freeing it */ +synchronize_rcu_tasks ();bpf_jit_free_exec (tr->image);hlist_del (&tr->hlist);kfree (tr);Signature · 1 line
2.23.0
Quoted 10 lines · Alexei Starovoitov
Though the second half of trampoline page is unused a task could be preempted in the middle of the first half of trampoline and two updates to trampoline would change the code from underneath the preempted task. Hence wait for tasks to voluntarily schedule or go to userspace.
Add similar wait before freeing the trampoline.Fixes fec56f5890d9 ("bpf: Introduce BPF trampoline")Reported-by Jann Horn <[email protected]>Signed-off-by Alexei Starovoitov <[email protected]>
Applied, thanks!