netdev - Re: [PATCH RFC xfrm-next v3 4/8] xfrm: add TX datapath support for IPsec full offload mode

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]

Date:   Sun, 25 Sep 2022 11:16:03 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Leon Romanovsky <leon@...nel.org>
CC:     Leon Romanovsky <leonro@...dia.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Jakub Kicinski <kuba@...nel.org>, <netdev@...r.kernel.org>,
        Paolo Abeni <pabeni@...hat.com>, Raed Salem <raeds@...dia.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Bharat Bhushan <bbhushan2@...vell.com>
Subject: Re: [PATCH RFC xfrm-next v3 4/8] xfrm: add TX datapath support for
 IPsec full offload mode

On Sun, Sep 04, 2022 at 04:15:38PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@...dia.com>
> 
> In IPsec full mode, the device is going to encrypt and encapsulate
> packets that are associated with offloaded policy. After successful
> policy lookup to indicate if packets should be offloaded or not,
> the stack forwards packets to the device to do the magic.
> 
> Signed-off-by: Raed Salem <raeds@...dia.com>
> Signed-off-by: Huy Nguyen <huyn@...dia.com>
> Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> ---
>  net/xfrm/xfrm_device.c | 15 +++++++++++++--
>  net/xfrm/xfrm_output.c | 12 +++++++++++-
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index 1cc482e9c87d..2d37bb86914a 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -120,6 +120,16 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
>  	if (xo->flags & XFRM_GRO || x->xso.dir == XFRM_DEV_OFFLOAD_IN)
>  		return skb;
>  
> +	/* The packet was sent to HW IPsec full offload engine,
> +	 * but to wrong device. Drop the packet, so it won't skip
> +	 * XFRM stack.
> +	 */
> +	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL && x->xso.dev != dev) {
> +		kfree_skb(skb);
> +		dev_core_stats_tx_dropped_inc(dev);
> +		return NULL;
> +	}
> +
>  	/* This skb was already validated on the upper/virtual dev */
>  	if ((x->xso.dev != dev) && (x->xso.real_dev == dev))
>  		return skb;
> @@ -369,8 +379,9 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
>  	if (!x->type_offload || x->encap)
>  		return false;
>  
> -	if ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
> -	    (!xdst->child->xfrm)) {
> +	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL ||
> +	    ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
> +	     !xdst->child->xfrm)) {
>  		mtu = xfrm_state_mtu(x, xdst->child_mtu_cached);
>  		if (skb->len <= mtu)
>  			goto ok;
> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> index 9a5e79a38c67..dde009be8463 100644
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -494,7 +494,7 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
>  	struct xfrm_state *x = dst->xfrm;
>  	struct net *net = xs_net(x);
>  
> -	if (err <= 0)
> +	if (err <= 0 || x->xso.type == XFRM_DEV_OFFLOAD_FULL)
>  		goto resume;

You check here that the state is marked as 'full offload' before
you skip the SW xfrm handling, but I don't see where you check
that the policy that led to this state is offloaded too. Also,
we have to make sure that both, policy and state is offloaded to
the same device. Looks like this part is missing.