On Wed, Dec 14, 2022 at 01:35:42PM +0100, Jiri Olsa wrote: > 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> >From an RCU viewpoint: Acked-by: Paul E. McKenney <paulmck@xxxxxxxxxx> > --- > kernel/bpf/dispatcher.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/kernel/bpf/dispatcher.c b/kernel/bpf/dispatcher.c > index c19719f48ce0..fa3e9225aedc 100644 > --- a/kernel/bpf/dispatcher.c > +++ b/kernel/bpf/dispatcher.c > @@ -125,6 +125,11 @@ static void bpf_dispatcher_update(struct bpf_dispatcher *d, int prev_num_progs) > > __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; > } > -- > 2.38.1 >
