[V2 PATCH net-next 2/2] xdp: catch invalid XDP_REDIRECT API usage
From: Jesper Dangaard Brouer <hidden>
Date: 2017-09-07 12:33:21
Subsystem:
bpf [core], bpf [general] (safe dynamic programs and tools), bpf [networking] (tcx & tc bpf, sock_addr), networking [general], the rest · Maintainers:
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
Catch different invalid XDP_REDIRECT and bpf_redirect_map API usage.
It is fairly easy to create a dangling redirect_info->map pointer,
which (until John or Daniel fix this) can crash the kernel.
The intended usage of the BPF helper bpf_redirect_map(), is to return
XDP_REDIRECT action after invoking it, but there is nothing stopping
the bpf_prog to return anything else. When XDP_REDIRECT isn't
returned, then a dangling ->map pointer is left behind, as
xdp_do_redirect() isn't called.
This also happens for drivers not implementing XDP_REDIRECT, as they
are not aware of this new XDP_REDIRECT return code, they leave the map
pointer dangling.
The simply solution to check for a dangling ->map pointer after each
driver napi->poll() invocation, see xdp_do_map_check().
This patch also add a check for a dangling ->map_to_flush pointer.
This should be considered a driver bug, as the driver contract is that
a pair of xdp_do_redirect and xdp_do_flush_map MUST be called in the
same cpu context.
Note, we need to check after each drivers napi->poll call, as:
1. DevA poll call bpf_redirect_map() but not xdp_do_redirect()
2. DevB bpf_prog uses bpf_redirect() and call xdp_do_redirect()
which now use map from DevA
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
include/linux/filter.h | 1 +
net/core/dev.c | 3 +++
net/core/filter.c | 25 +++++++++++++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index d29e58fde364..0c48941e0022 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h@@ -724,6 +724,7 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp, struct bpf_prog *prog); void xdp_do_flush_map(void); +void xdp_do_map_check(struct napi_struct *napi); void bpf_warn_invalid_xdp_action(u32 act); void bpf_warn_invalid_xdp_redirect(u32 ifindex);
diff --git a/net/core/dev.c b/net/core/dev.c
index 6f845e4fec17..7eac642b469f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c@@ -5320,6 +5320,7 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock) */ rc = napi->poll(napi, BUSY_POLL_BUDGET); trace_napi_poll(napi, rc, BUSY_POLL_BUDGET); + xdp_do_map_check(napi); netpoll_poll_unlock(have_poll_lock); if (rc == BUSY_POLL_BUDGET) __napi_schedule(napi);
@@ -5367,6 +5368,7 @@ void napi_busy_loop(unsigned int napi_id, } work = napi_poll(napi, BUSY_POLL_BUDGET); trace_napi_poll(napi, work, BUSY_POLL_BUDGET); + xdp_do_map_check(napi); count: if (work > 0) __NET_ADD_STATS(dev_net(napi->dev),
@@ -5529,6 +5531,7 @@ static int napi_poll(struct napi_struct *n, struct list_head *repoll) if (test_bit(NAPI_STATE_SCHED, &n->state)) { work = n->poll(n, weight); trace_napi_poll(n, work, weight); + xdp_do_map_check(n); } WARN_ON_ONCE(work > weight);
diff --git a/net/core/filter.c b/net/core/filter.c
index 3767470cab6c..f0e1135eeb9d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c@@ -2500,6 +2500,31 @@ void xdp_do_flush_map(void) } EXPORT_SYMBOL_GPL(xdp_do_flush_map); +void xdp_do_map_check(struct napi_struct *napi) +{ + struct redirect_info *ri = this_cpu_ptr(&redirect_info); + + /* XDP drivers (and XDP-generic) must invoke xdp_do_redirect() + * when bpf_prog use helper bpf_redirect_map(), else the map + * pointer can be left dangling. Catch this invalid API + * usage, instead of potentially crashing. + */ + if (ri->map) { + ri->map = NULL; + net_err_ratelimited("%s: caught invalid XDP bpf_redirect_map\n", + napi->dev->name); + trace_xdp_exception(napi->dev, NULL, XDP_REDIRECT); + } + if (ri->map_to_flush) { /* Driver bug */ + net_err_ratelimited("%s: XDP driver miss xdp_do_flush_map\n", + napi->dev->name); + trace_xdp_exception(napi->dev, NULL, XDP_REDIRECT); + /* Flush map, else pkts can be stuck on XDP TXq */ + xdp_do_flush_map(); + } +} +EXPORT_SYMBOL_GPL(xdp_do_map_check); + static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, struct bpf_prog *xdp_prog) {