Commit 96c5508e authored by Jesper Dangaard Brouer's avatar Jesper Dangaard Brouer Committed by David S. Miller
Browse files

xdp: implement xdp_redirect_map for generic XDP 



Using bpf_redirect_map is allowed for generic XDP programs, but the
appropriate map lookup was never performed in xdp_do_generic_redirect().

Instead the map-index is directly used as the ifindex.  For the
xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
sending on ifindex 0 which isn't valid, resulting in getting SKB
packets dropped.  Thus, the reported performance numbers are wrong in
commit 24251c26 ("samples/bpf: add option for native and skb mode
for redirect apps") for the 'xdp_redirect_map -S' case.

Before commit 109980b8 ("bpf: don't select potentially stale
ri->map from buggy xdp progs") it could crash the kernel.  Like this
commit also check that the map_owner owner is correct before
dereferencing the map pointer.  But make sure that this API misusage
can be caught by a tracepoint. Thus, allowing userspace via
tracepoints to detect misbehaving bpf_progs.

Fixes: 6103aa96 ("net: implement XDP_REDIRECT for xdp generic")
Fixes: 24251c26 ("samples/bpf: add option for native and skb mode for redirect apps")
Signed-off-by: default avatarJesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 609320c8

include/trace/events/xdp.h

+2 −2
Original line number Diff line number Diff line
@@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err, 


#define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx)		\

	 trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0,	\

				0, map, idx);

				0, map, idx)



#define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err)	\

	 trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0,	\

				    err, map, idx);

				    err, map, idx)



#endif /* _TRACE_XDP_H */

net/core/filter.c

+26 −12
Original line number Diff line number Diff line
@@ -2506,21 +2506,19 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, 
	struct redirect_info *ri = this_cpu_ptr(&redirect_info);

	const struct bpf_prog *map_owner = ri->map_owner;

	struct bpf_map *map = ri->map;

	struct net_device *fwd = NULL;

	u32 index = ri->ifindex;

	struct net_device *fwd;

	int err;



	ri->ifindex = 0;

	ri->map = NULL;

	ri->map_owner = NULL;



	/* This is really only caused by a deliberately crappy

	 * BPF program, normally we would never hit that case,

	 * so no need to inform someone via tracepoints either,

	 * just bail out.

	 */

	if (unlikely(map_owner != xdp_prog))

		return -EINVAL;

	if (unlikely(map_owner != xdp_prog)) {

		err = -EFAULT;

		map = NULL;

		goto err;

	}



	fwd = __dev_map_lookup_elem(map, index);

	if (!fwd) {
@@ -2576,13 +2574,27 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb, 
			    struct bpf_prog *xdp_prog)

{

	struct redirect_info *ri = this_cpu_ptr(&redirect_info);

	const struct bpf_prog *map_owner = ri->map_owner;

	struct bpf_map *map = ri->map;

	struct net_device *fwd = NULL;

	u32 index = ri->ifindex;

	struct net_device *fwd;

	unsigned int len;

	int err = 0;



	fwd = dev_get_by_index_rcu(dev_net(dev), index);

	ri->ifindex = 0;

	ri->map = NULL;

	ri->map_owner = NULL;



	if (map) {

		if (unlikely(map_owner != xdp_prog)) {

			err = -EFAULT;

			map = NULL;

			goto err;

		}

		fwd = __dev_map_lookup_elem(map, index);

	} else {

		fwd = dev_get_by_index_rcu(dev_net(dev), index);

	}

	if (unlikely(!fwd)) {

		err = -EINVAL;

		goto err;
@@ -2600,10 +2612,12 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb, 
	}



	skb->dev = fwd;

	_trace_xdp_redirect(dev, xdp_prog, index);

	map ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)

		: _trace_xdp_redirect(dev, xdp_prog, index);

	return 0;

err:

	_trace_xdp_redirect_err(dev, xdp_prog, index, err);

	map ? _trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err)

		: _trace_xdp_redirect_err(dev, xdp_prog, index, err);

	return err;

}

EXPORT_SYMBOL_GPL(xdp_do_generic_redirect);

CRA Git | Maintained and supported by SUSTech CRA and CCSE