| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231215171020.687342-18-bigeasy@linutronix.de>
Date: Fri, 15 Dec 2023 18:07:36 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: linux-kernel@...r.kernel.org,
netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
Boqun Feng <boqun.feng@...il.com>,
Daniel Borkmann <daniel@...earbox.net>,
Eric Dumazet <edumazet@...gle.com>,
Frederic Weisbecker <frederic@...nel.org>,
Ingo Molnar <mingo@...hat.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Waiman Long <longman@...hat.com>,
Will Deacon <will@...nel.org>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Alexei Starovoitov <ast@...nel.org>,
Arthur Kiyanovski <akiyano@...zon.com>,
David Arinzon <darinzon@...zon.com>,
Igor Russkikh <irusskikh@...vell.com>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
Michael Chan <michael.chan@...adcom.com>,
Noam Dagan <ndagan@...zon.com>,
Saeed Bishara <saeedb@...zon.com>,
Shay Agroskin <shayagr@...zon.com>,
Sunil Goutham <sgoutham@...vell.com>
Subject: [PATCH net-next 17/24] net: amazon, aquanti, broadcom, cavium, engleder: Use nested-BH locking for XDP redirect.
The per-CPU variables used during bpf_prog_run_xdp() invocation and
later during xdp_do_redirect() rely on disabled BH for their protection.
Without locking in local_bh_disable() on PREEMPT_RT these data structure
require explicit locking.
This is a follow-up on the previous change which introduced
bpf_run_lock.redirect_lock and uses it now within drivers.
The simple way is to acquire the lock before bpf_prog_run_xdp() is
invoked and hold it until the end of function.
This does not always work because some drivers (cpsw, atlantic) invoke
xdp_do_flush() in the same context.
Acquiring the lock in bpf_prog_run_xdp() and dropping in
xdp_do_redirect() (without touching drivers) does not work because not
all driver, which use bpf_prog_run_xdp(), do support XDP_REDIRECT (and
invoke xdp_do_redirect()).
Ideally the minimal locking scope would be bpf_prog_run_xdp() +
xdp_do_redirect() and everything else (error recovery, DMA unmapping,
free/ alloc of memory, …) would happen outside of the locked section.
Cc: Alexei Starovoitov <ast@...nel.org>
Cc: Arthur Kiyanovski <akiyano@...zon.com>
Cc: David Arinzon <darinzon@...zon.com>
Cc: Igor Russkikh <irusskikh@...vell.com>
Cc: Jesper Dangaard Brouer <hawk@...nel.org>
Cc: John Fastabend <john.fastabend@...il.com>
Cc: Michael Chan <michael.chan@...adcom.com>
Cc: Noam Dagan <ndagan@...zon.com>
Cc: Saeed Bishara <saeedb@...zon.com>
Cc: Shay Agroskin <shayagr@...zon.com>
Cc: Sunil Goutham <sgoutham@...vell.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 +
.../net/ethernet/aquantia/atlantic/aq_ring.c | 26 ++++++++++++-------
drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 1 +
.../net/ethernet/cavium/thunder/nicvf_main.c | 3 ++-
drivers/net/ethernet/engleder/tsnep_main.c | 17 +++++++-----
5 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index b5bca48148309..cf075bc5e2b13 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -385,6 +385,7 @@ static int ena_xdp_execute(struct ena_ring *rx_ring, struct xdp_buff *xdp)
if (!xdp_prog)
goto out;
+ guard(local_lock_nested_bh)(&bpf_run_lock.redirect_lock);
verdict = bpf_prog_run_xdp(xdp_prog, xdp);
switch (verdict) {
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 694daeaf3e615..5d33d478d5109 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -458,7 +458,24 @@ static struct sk_buff *aq_xdp_run_prog(struct aq_nic_s *aq_nic,
if (xdp_buff_has_frags(xdp) && !prog->aux->xdp_has_frags)
goto out_aborted;
+ local_lock_nested_bh(&bpf_run_lock.redirect_lock);
act = bpf_prog_run_xdp(prog, xdp);
+ if (act == XDP_REDIRECT) {
+ if (xdp_do_redirect(aq_nic->ndev, xdp, prog) < 0) {
+ local_unlock_nested_bh(&bpf_run_lock.redirect_lock);
+ goto out_aborted;
+ }
+ local_unlock_nested_bh(&bpf_run_lock.redirect_lock);
+
+ xdp_do_flush();
+ u64_stats_update_begin(&rx_ring->stats.rx.syncp);
+ ++rx_ring->stats.rx.xdp_redirect;
+ u64_stats_update_end(&rx_ring->stats.rx.syncp);
+ aq_get_rxpages_xdp(buff, xdp);
+ } else {
+ local_unlock_nested_bh(&bpf_run_lock.redirect_lock);
+ }
+
switch (act) {
case XDP_PASS:
skb = aq_xdp_build_skb(xdp, aq_nic->ndev, buff);
@@ -481,15 +498,6 @@ static struct sk_buff *aq_xdp_run_prog(struct aq_nic_s *aq_nic,
u64_stats_update_end(&rx_ring->stats.rx.syncp);
aq_get_rxpages_xdp(buff, xdp);
break;
- case XDP_REDIRECT:
- if (xdp_do_redirect(aq_nic->ndev, xdp, prog) < 0)
- goto out_aborted;
- xdp_do_flush();
- u64_stats_update_begin(&rx_ring->stats.rx.syncp);
- ++rx_ring->stats.rx.xdp_redirect;
- u64_stats_update_end(&rx_ring->stats.rx.syncp);
- aq_get_rxpages_xdp(buff, xdp);
- break;
default:
fallthrough;
case XDP_ABORTED:
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
index 96f5ca778c67d..c4d989da7fade 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
@@ -253,6 +253,7 @@ bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons,
/* BNXT_RX_PAGE_MODE(bp) when XDP enabled */
orig_data = xdp.data;
+ guard(local_lock_nested_bh)(&bpf_run_lock.redirect_lock);
act = bpf_prog_run_xdp(xdp_prog, &xdp);
tx_avail = bnxt_tx_avail(bp, txr);
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index eff350e0bc2a8..8e1406101f71b 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -554,7 +554,8 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,
xdp_prepare_buff(&xdp, hard_start, data - hard_start, len, false);
orig_data = xdp.data;
- action = bpf_prog_run_xdp(prog, &xdp);
+ scoped_guard(local_lock_nested_bh, &bpf_run_lock.redirect_lock)
+ action = bpf_prog_run_xdp(prog, &xdp);
len = xdp.data_end - xdp.data;
/* Check if XDP program has changed headers */
diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index df40c720e7b23..acda3502d274f 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -1268,6 +1268,7 @@ static bool tsnep_xdp_run_prog(struct tsnep_rx *rx, struct bpf_prog *prog,
length = xdp->data_end - xdp->data_hard_start - XDP_PACKET_HEADROOM;
+ guard(local_lock_nested_bh)(&bpf_run_lock.redirect_lock);
act = bpf_prog_run_xdp(prog, xdp);
switch (act) {
case XDP_PASS:
@@ -1309,14 +1310,16 @@ static bool tsnep_xdp_run_prog_zc(struct tsnep_rx *rx, struct bpf_prog *prog,
{
u32 act;
- act = bpf_prog_run_xdp(prog, xdp);
+ scoped_guard(local_lock_nested_bh, &bpf_run_lock.redirect_lock) {
+ act = bpf_prog_run_xdp(prog, xdp);
- /* XDP_REDIRECT is the main action for zero-copy */
- if (likely(act == XDP_REDIRECT)) {
- if (xdp_do_redirect(rx->adapter->netdev, xdp, prog) < 0)
- goto out_failure;
- *status |= TSNEP_XDP_REDIRECT;
- return true;
+ /* XDP_REDIRECT is the main action for zero-copy */
+ if (likely(act == XDP_REDIRECT)) {
+ if (xdp_do_redirect(rx->adapter->netdev, xdp, prog) < 0)
+ goto out_failure;
+ *status |= TSNEP_XDP_REDIRECT;
+ return true;
+ }
}
switch (act) {
--
2.43.0
Powered by blists - more mailing lists