lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 6 Dec 2019 10:03:09 +0100 From: Björn Töpel <bjorn.topel@...il.com> To: Maxim Mikityanskiy <maximmi@...lanox.com> Cc: Björn Töpel <bjorn.topel@...el.com>, Magnus Karlsson <magnus.karlsson@...el.com>, Jeff Kirsher <jeffrey.t.kirsher@...el.com>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>, "David S. Miller" <davem@...emloft.net>, Saeed Mahameed <saeedm@...lanox.com>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Jakub Kicinski <jakub.kicinski@...ronome.com>, Jesper Dangaard Brouer <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>, Jonathan Lemon <jonathan.lemon@...il.com> Subject: Re: [PATCH bpf 4/4] net/ixgbe: Fix concurrency issues between config flow and XSK On Thu, 5 Dec 2019 at 16:52, Maxim Mikityanskiy <maximmi@...lanox.com> wrote: > > Use synchronize_rcu to wait until the XSK wakeup function finishes > before destroying the resources it uses: > > 1. ixgbe_down already calls synchronize_rcu after setting __IXGBE_DOWN. > > 2. After switching the XDP program, call synchronize_rcu to let > ixgbe_xsk_async_xmit exit before the XDP program is freed. > > 3. Changing the number of channels brings the interface down. > > 4. Disabling UMEM sets __IXGBE_TX_DISABLED before closing hardware > resources and resetting xsk_umem. Check that bit in ixgbe_xsk_async_xmit > to avoid using the XDP ring when it's already destroyed. synchronize_rcu > is called from ixgbe_txrx_ring_disable. > > Signed-off-by: Maxim Mikityanskiy <maximmi@...lanox.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 +++++- > drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 8 ++++++-- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index 25c097cd8100..60503318c7e5 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -10273,8 +10273,12 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog) > adapter->xdp_prog); > } > > - if (old_prog) > + if (old_prog) { > + /* Wait until ndo_xsk_async_xmit completes. */ > + synchronize_rcu(); > + > bpf_prog_put(old_prog); > + } > > /* Kick start the NAPI context if there is an AF_XDP socket open > * on that queue id. This so that receiving will start. > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c > index d6feaacfbf89..b43be9f14105 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c > @@ -709,10 +709,14 @@ int ixgbe_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags) > if (qid >= adapter->num_xdp_queues) > return -ENXIO; > > - if (!adapter->xdp_ring[qid]->xsk_umem) > + ring = adapter->xdp_ring[qid]; > + > + if (test_bit(__IXGBE_TX_DISABLED, &ring->state)) > + return -ENETDOWN; > + > + if (!ring->xsk_umem) Pretty much same comment as in i40e. The synchronize_rcu() is not needed, but the additional test is! > return -ENXIO; > > - ring = adapter->xdp_ring[qid]; > if (!napi_if_scheduled_mark_missed(&ring->q_vector->napi)) { > u64 eics = BIT_ULL(ring->q_vector->v_idx); > > -- > 2.20.1 >
Powered by blists - more mailing lists