* [PATCH bpf-next] bpf: Fix trampoline usage in preempt
@ 2020-01-21 3:22 Alexei Starovoitov
2020-01-21 4:54 ` Paul E. McKenney
2020-01-22 15:55 ` Daniel Borkmann
0 siblings, 2 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2020-01-21 3:22 UTC (permalink / raw)
To: davem; +Cc: daniel, jannh, paulmck, netdev, bpf, kernel-team
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 <jannh@google.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/trampoline.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 79a04417050d..7657ede7aee2 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -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);
--
2.23.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Fix trampoline usage in preempt
2020-01-21 3:22 [PATCH bpf-next] bpf: Fix trampoline usage in preempt Alexei Starovoitov
@ 2020-01-21 4:54 ` Paul E. McKenney
2020-01-22 15:55 ` Daniel Borkmann
1 sibling, 0 replies; 3+ messages in thread
From: Paul E. McKenney @ 2020-01-21 4:54 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: davem, daniel, jannh, netdev, bpf, kernel-team
On Mon, Jan 20, 2020 at 07:22:31PM -0800, Alexei Starovoitov wrote:
> 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 <jannh@google.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> kernel/bpf/trampoline.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 79a04417050d..7657ede7aee2 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -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:
Acked-by: Paul E. McKenney <paulmck@kernel.org>
> 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);
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Fix trampoline usage in preempt
2020-01-21 3:22 [PATCH bpf-next] bpf: Fix trampoline usage in preempt Alexei Starovoitov
2020-01-21 4:54 ` Paul E. McKenney
@ 2020-01-22 15:55 ` Daniel Borkmann
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2020-01-22 15:55 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: jannh, paulmck, netdev, bpf, kernel-team
On 1/21/20 4:22 AM, Alexei Starovoitov wrote:
> 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 <jannh@google.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Applied, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-01-22 15:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-21 3:22 [PATCH bpf-next] bpf: Fix trampoline usage in preempt Alexei Starovoitov
2020-01-21 4:54 ` Paul E. McKenney
2020-01-22 15:55 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).