netdev - Re: [PATCH xfrm-next v7 6/8] xfrm: speed-up lookup of HW policies

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: <20221122131002.GN704954@gauss3.secunet.de>
Date:   Tue, 22 Nov 2022 14:10:02 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Leon Romanovsky <leon@...nel.org>
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 6/8] xfrm: speed-up lookup of HW policies

On Mon, Nov 21, 2022 at 03:01:42PM +0200, Leon Romanovsky wrote:
> On Mon, Nov 21, 2022 at 01:43:49PM +0100, Steffen Klassert wrote:
> > On Mon, Nov 21, 2022 at 02:02:52PM +0200, Leon Romanovsky wrote:
> > > 
> > > I think that something like this will do the trick.
> > > 
> > > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> > > index 5076f9d7a752..d1c9ef857755 100644
> > > --- a/net/xfrm/xfrm_state.c
> > > +++ b/net/xfrm/xfrm_state.c
> > > @@ -1090,6 +1090,28 @@ static void xfrm_state_look_at(struct xfrm_policy *pol, struct xfrm_state *x,
> > >         }
> > >  }
> > > 
> > > +static bool xfrm_state_and_policy_mixed(struct xfrm_state *x,
> > > +                                       struct xfrm_policy *p)
> > > +{
> > > +       /* Packet offload: both policy and SA should be offloaded */
> > > +       if (p->xdo.type == XFRM_DEV_OFFLOAD_PACKET &&
> > > +           x->xso.type != XFRM_DEV_OFFLOAD_PACKET)
> > > +               return true;
> > > +
> > > +       if (p->xdo.type != XFRM_DEV_OFFLOAD_PACKET &&
> > > +           x->xso.type == XFRM_DEV_OFFLOAD_PACKET)
> > > +               return true;
> > > +
> > > +       if (p->xdo.type != XFRM_DEV_OFFLOAD_PACKET)
> > > +               return false;
> > > +
> > > +       /* Packet offload: both policy and SA should have same device */
> > > +       if (p->xdo.dev != x->xso.dev)
> > > +               return true;
> > > +
> > > +       return false;
> > > +}
> > > +
> > >  struct xfrm_state *
> > >  xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
> > >                 const struct flowi *fl, struct xfrm_tmpl *tmpl,
> > > @@ -1147,7 +1169,8 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
> > > 
> > >  found:
> > >         x = best;
> > > -       if (!x && !error && !acquire_in_progress) {
> > > +       if (!x && !error && !acquire_in_progress &&
> > > +           pol->xdo.type != XFRM_DEV_OFFLOAD_PACKET) {
> > >                 if (tmpl->id.spi &&
> > >                     (x0 = __xfrm_state_lookup(net, mark, daddr, tmpl->id.spi,
> > >                                               tmpl->id.proto, encap_family)) != NULL) {
> > > @@ -1228,7 +1251,14 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
> > >                         *err = -EAGAIN;
> > >                         x = NULL;
> > >                 }
> > > +               if (x && xfrm_state_and_policy_mixed(x, pol)) {
> > > +                       *err = -EINVAL;
> > > +                       x = NULL;
> > 
> > If policy and state do not match here, this means the lookup
> > returned the wrong state. The correct state might still sit
> > in the database. At this point, you should either have found
> > a matching state, or no state at all.
> 
> I check for "x" because of "x = NULL" above.

This does not change the fact that the lookup returned the wrong state.