6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Jiri Olsa <jolsa@xxxxxxxxxx>
commit 4121d4481b72501aa4d22680be4ea1096d69d133 upstream.
Hao Sun reported crash in dispatcher image [1].
Currently we don't have any sync between bpf_dispatcher_update and
bpf_dispatcher_xdp_func, so following race is possible:
cpu 0: cpu 1:
bpf_prog_run_xdp
...
bpf_dispatcher_xdp_func
in image at offset 0x0
bpf_dispatcher_update
update image at offset 0x800
bpf_dispatcher_update
update image at offset 0x0
in image at offset 0x0 -> crash
Fixing this by synchronizing dispatcher image update (which is done
in bpf_dispatcher_update function) with bpf_dispatcher_xdp_func that
reads and execute the dispatcher image.
Calling synchronize_rcu after updating and installing new image ensures
that readers leave old image before it's changed in the next dispatcher
update. The update itself is locked with dispatcher's mutex.
The bpf_prog_run_xdp is called under local_bh_disable and synchronize_rcu
will wait for it to leave [2].
[1] https://lore.kernel.org/bpf/Y5SFho7ZYXr9ifRn@krava/T/#m00c29ece654bc9f332a17df493bbca33e702896c
[2] https://lore.kernel.org/bpf/0B62D35A-E695-4B7A-A0D4-774767544C1A@xxxxxxxxx/T/#mff43e2c003ae99f4a38f353c7969be4c7162e877
Reported-by: Hao Sun <sunhao.th@xxxxxxxxx>
Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
Acked-by: Yonghong Song <yhs@xxxxxx>
Acked-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20221214123542.1389719-1-jolsa@xxxxxxxxxx
Signed-off-by: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
Reported-by: syzbot+08ba1e474d350b613604@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Sergio González Collado <sergio.collado@xxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
kernel/bpf/dispatcher.c | 5 +++++
1 file changed, 5 insertions(+)
--- a/kernel/bpf/dispatcher.c
+++ b/kernel/bpf/dispatcher.c
@@ -125,6 +125,11 @@ static void bpf_dispatcher_update(struct
__BPF_DISPATCHER_UPDATE(d, new ?: (void *)&bpf_dispatcher_nop_func);
+ /* Make sure all the callers executing the previous/old half of the
+ * image leave it, so following update call can modify it safely.
+ */
+ synchronize_rcu();
+
if (new)
d->image_off = noff;
}
