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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9c0b3cef3029ea71f7802eab6214062aad48d509.camel@redhat.com>
Date:   Tue, 12 Jul 2022 12:12:28 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Lorenzo Bianconi <lorenzo@...nel.org>, netdev@...r.kernel.org
Cc:     nbd@....name, john@...ozen.org, sean.wang@...iatek.com,
        Mark-MC.Lee@...iatek.com, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, matthias.bgg@...il.com,
        linux-mediatek@...ts.infradead.org, ilias.apalodimas@...aro.org,
        lorenzo.bianconi@...hat.com, jbrouer@...hat.com
Subject: Re: [PATCH net-next 2/4] net: ethernet: mtk_eth_soc: add basic XDP
 support

On Sat, 2022-07-09 at 17:48 +0200, Lorenzo Bianconi wrote:
> Introduce basic XDP support to mtk_eth_soc driver.
> Supported XDP verdicts:
> - XDP_PASS
> - XDP_DROP
> - XDP_REDIRECT
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 146 +++++++++++++++++---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.h |   2 +
>  2 files changed, 130 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 9a92d602ebd5..3b583abb599d 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -1494,22 +1494,55 @@ static void mtk_rx_put_buff(struct mtk_rx_ring *ring, void *data, bool napi)
>  		skb_free_frag(data);
>  }
>  
> +static u32 mtk_xdp_run(struct mtk_rx_ring *ring, struct bpf_prog *prog,
> +		       struct xdp_buff *xdp, struct net_device *dev)
> +{
> +	u32 act = XDP_PASS;
> +
> +	if (!prog)
> +		return XDP_PASS;
> +
> +	act = bpf_prog_run_xdp(prog, xdp);
> +	switch (act) {
> +	case XDP_PASS:
> +		return XDP_PASS;
> +	case XDP_REDIRECT:
> +		if (unlikely(xdp_do_redirect(dev, xdp, prog)))
> +			break;
> +		return XDP_REDIRECT;
> +	default:
> +		bpf_warn_invalid_xdp_action(dev, prog, act);
> +		fallthrough;
> +	case XDP_ABORTED:
> +		trace_xdp_exception(dev, prog, act);
> +		fallthrough;
> +	case XDP_DROP:
> +		break;
> +	}
> +
> +	page_pool_put_full_page(ring->page_pool,
> +				virt_to_head_page(xdp->data), true);
> +	return XDP_DROP;
> +}
> +
>  static int mtk_poll_rx(struct napi_struct *napi, int budget,
>  		       struct mtk_eth *eth)
>  {
> +	struct bpf_prog *prog = READ_ONCE(eth->prog);
>  	struct dim_sample dim_sample = {};
>  	struct mtk_rx_ring *ring;
>  	int idx;
>  	struct sk_buff *skb;
>  	u8 *data, *new_data;
>  	struct mtk_rx_dma_v2 *rxd, trxd;
> +	bool xdp_do_redirect = false;
>  	int done = 0, bytes = 0;
>  
>  	while (done < budget) {
>  		unsigned int pktlen, *rxdcsum;
> -		u32 hash, reason, reserve_len;
>  		struct net_device *netdev;
>  		dma_addr_t dma_addr;
> +		u32 hash, reason;
>  		int mac = 0;
>  
>  		ring = mtk_get_rx_ring(eth);
> @@ -1539,8 +1572,14 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
>  		if (unlikely(test_bit(MTK_RESETTING, &eth->state)))
>  			goto release_desc;
>  
> +		pktlen = RX_DMA_GET_PLEN0(trxd.rxd2);
> +
>  		/* alloc new buffer */
>  		if (ring->page_pool) {
> +			struct page *page = virt_to_head_page(data);
> +			struct xdp_buff xdp;
> +			u32 ret;
> +
>  			new_data = mtk_page_pool_get_buff(ring->page_pool,
>  							  &dma_addr,
>  							  GFP_ATOMIC);
> @@ -1548,6 +1587,34 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
>  				netdev->stats.rx_dropped++;
>  				goto release_desc;
>  			}
> +
> +			dma_sync_single_for_cpu(eth->dma_dev,
> +				page_pool_get_dma_addr(page) + MTK_PP_HEADROOM,
> +				pktlen, page_pool_get_dma_dir(ring->page_pool));
> +
> +			xdp_init_buff(&xdp, PAGE_SIZE, &ring->xdp_q);
> +			xdp_prepare_buff(&xdp, data, MTK_PP_HEADROOM, pktlen,
> +					 false);
> +			xdp_buff_clear_frags_flag(&xdp);
> +
> +			ret = mtk_xdp_run(ring, prog, &xdp, netdev);
> +			if (ret != XDP_PASS) {
> +				if (ret == XDP_REDIRECT)
> +					xdp_do_redirect = true;
> +				goto skip_rx;
> +			}
> +
> +			skb = build_skb(data, PAGE_SIZE);
> +			if (unlikely(!skb)) {
> +				page_pool_put_full_page(ring->page_pool,
> +							page, true);
> +				netdev->stats.rx_dropped++;
> +				goto skip_rx;
> +			}
> +
> +			skb_reserve(skb, xdp.data - xdp.data_hard_start);
> +			skb_put(skb, xdp.data_end - xdp.data);
> +			skb_mark_for_recycle(skb);
>  		} else {
>  			if (ring->frag_size <= PAGE_SIZE)
>  				new_data = napi_alloc_frag(ring->frag_size);
> @@ -1571,27 +1638,20 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
>  
>  			dma_unmap_single(eth->dma_dev, trxd.rxd1,
>  					 ring->buf_size, DMA_FROM_DEVICE);
> -		}
>  
> -		/* receive data */
> -		skb = build_skb(data, ring->frag_size);
> -		if (unlikely(!skb)) {
> -			mtk_rx_put_buff(ring, data, true);
> -			netdev->stats.rx_dropped++;
> -			goto skip_rx;
> -		}
> +			skb = build_skb(data, ring->frag_size);
> +			if (unlikely(!skb)) {
> +				netdev->stats.rx_dropped++;
> +				skb_free_frag(data);
> +				goto skip_rx;
> +			}
>  
> -		if (ring->page_pool) {
> -			reserve_len = MTK_PP_HEADROOM;
> -			skb_mark_for_recycle(skb);
> -		} else {
> -			reserve_len = NET_SKB_PAD + NET_IP_ALIGN;
> +			skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
> +			skb_put(skb, pktlen);
>  		}
> -		skb_reserve(skb, reserve_len);
>  
> -		pktlen = RX_DMA_GET_PLEN0(trxd.rxd2);
>  		skb->dev = netdev;
> -		skb_put(skb, pktlen);
> +		bytes += skb->len;
>  
>  		if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2))
>  			rxdcsum = &trxd.rxd3;
> @@ -1603,7 +1663,6 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
>  		else
>  			skb_checksum_none_assert(skb);
>  		skb->protocol = eth_type_trans(skb, netdev);
> -		bytes += pktlen;
>  
>  		hash = trxd.rxd4 & MTK_RXD4_FOE_ENTRY;
>  		if (hash != MTK_RXD4_FOE_ENTRY) {
> @@ -1666,6 +1725,9 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
>  			  &dim_sample);
>  	net_dim(&eth->rx_dim, dim_sample);
>  
> +	if (prog && xdp_do_redirect)
> +		xdp_do_flush_map();
> +
>  	return done;
>  }
>  
> @@ -2750,6 +2812,48 @@ static int mtk_stop(struct net_device *dev)
>  	return 0;
>  }
>  
> +static int mtk_xdp_setup(struct net_device *dev, struct bpf_prog *prog,
> +			 struct netlink_ext_ack *extack)
> +{
> +	struct mtk_mac *mac = netdev_priv(dev);
> +	struct mtk_eth *eth = mac->hw;
> +	struct bpf_prog *old_prog;
> +	bool need_update;
> +
> +	if (eth->hwlro) {
> +		NL_SET_ERR_MSG_MOD(extack, "XDP not supported with HWLRO");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (dev->mtu > MTK_PP_MAX_BUF_SIZE) {
> +		NL_SET_ERR_MSG_MOD(extack, "MTU too large for XDP");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	need_update = !!eth->prog != !!prog;
> +	if (netif_running(dev) && need_update)
> +		mtk_stop(dev);
> +
> +	old_prog = xchg(&eth->prog, prog);
> +	if (old_prog)
> +		bpf_prog_put(old_prog);
> +
> +	if (netif_running(dev) && need_update)
> +		return mtk_open(dev);
> +
> +	return 0;
> +}
> +
> +static int mtk_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> +{
> +	switch (xdp->command) {
> +	case XDP_SETUP_PROG:
> +		return mtk_xdp_setup(dev, xdp->prog, xdp->extack);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static void ethsys_reset(struct mtk_eth *eth, u32 reset_bits)
>  {
>  	regmap_update_bits(eth->ethsys, ETHSYS_RSTCTRL,
> @@ -3045,6 +3149,11 @@ static int mtk_change_mtu(struct net_device *dev, int new_mtu)
>  	struct mtk_eth *eth = mac->hw;
>  	u32 mcr_cur, mcr_new;
>  
> +	if (eth->prog && length > MTK_PP_MAX_BUF_SIZE) {
> +		netdev_err(dev, "Invalid MTU for XDP mode\n");
> +		return -EINVAL;
> +	}
> +
>  	if (!MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628)) {
>  		mcr_cur = mtk_r32(mac->hw, MTK_MAC_MCR(mac->id));
>  		mcr_new = mcr_cur & ~MAC_MCR_MAX_RX_MASK;
> @@ -3372,6 +3481,7 @@ static const struct net_device_ops mtk_netdev_ops = {
>  	.ndo_poll_controller	= mtk_poll_controller,
>  #endif
>  	.ndo_setup_tc		= mtk_eth_setup_tc,
> +	.ndo_bpf		= mtk_xdp,
>  };
>  
>  static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 26c019319055..a1cea93300c1 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -1088,6 +1088,8 @@ struct mtk_eth {
>  
>  	struct mtk_ppe			*ppe;
>  	struct rhashtable		flow_table;
> +
> +	struct bpf_prog			*prog;

The XDP program is apparently under an RCU protection schema (otherwise
you will get UaF when replacing it). Why don't you have explicit RCU
annotations? (here, and where the 'prog' field is touched).

Thanks!

Paolo

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ