netdev - Re: [PATCH xfrm-next v7 4/8] xfrm: add TX datapath support for IPsec packet 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]

Message-ID: <Y3YpyplG969qtYO3@unreal>
Date:   Thu, 17 Nov 2022 14:32:10 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Steffen Klassert <steffen.klassert@...unet.com>
Cc:     "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
Subject: Re: [PATCH xfrm-next v7 4/8] xfrm: add TX datapath support for IPsec
 packet offload mode

On Thu, Nov 17, 2022 at 12:59:39PM +0100, Steffen Klassert wrote:
> On Wed, Nov 09, 2022 at 02:54:32PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@...dia.com>
> 
> > @@ -2708,6 +2710,23 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
> >  	if (!dev)
> >  		goto free_dst;
> >  
> > +	dst1 = &xdst0->u.dst;
> > +	/* Packet offload: both policy and SA should be offloaded */
> > +	if ((policy->xdo.type == XFRM_DEV_OFFLOAD_PACKET &&
> > +	     dst1->xfrm->xso.type != XFRM_DEV_OFFLOAD_PACKET) ||
> > +	    (policy->xdo.type != XFRM_DEV_OFFLOAD_PACKET &&
> > +	     dst1->xfrm->xso.type == XFRM_DEV_OFFLOAD_PACKET)) {
> > +		err = -EINVAL;
> > +		goto free_dst;
> > +	}
> > +
> > +	/* Packet offload: both policy and SA should have same device */
> > +	if (policy->xdo.type == XFRM_DEV_OFFLOAD_PACKET &&
> > +	    policy->xdo.dev != dst1->xfrm->xso.dev) {
> > +		err = -EINVAL;
> > +		goto free_dst;
> > +	}
> > +
> 
> This is the wrong place for these checks. Things went already wrong
> in the lookup if policy and state do not match here.

Where do you think we should put such checks?

We need to make sure that both policy and SA are offloaded when handle
packet, It prevents various corner cases where we will mix SW and HW
paths.

xfrm_bundle_create() is called when we perform XFRM lookup to create dst_entry.

Thanks