netdev - Re: [PATCH 1/3] xfrm: Use the XFRM_GRO to indicate a GRO call on input.

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: <CAHsH6GtgcffudgDzyTv8f8CZC13ZKMeU886DLHuuj0Tu73Ppfg@mail.gmail.com>
Date:   Fri, 20 Jan 2023 13:05:04 +0200
From:   Eyal Birger <eyal.birger@...il.com>
To:     antony.antony@...unet.com
Cc:     Steffen Klassert <steffen.klassert@...unet.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        netdev@...r.kernel.org
Subject: Re: [PATCH 1/3] xfrm: Use the XFRM_GRO to indicate a GRO call on input.

Hi,

On Thu, Jan 19, 2023 at 9:59 PM Antony Antony <antony.antony@...unet.com> wrote:
>
> From: Steffen Klassert <steffen.klassert@...unet.com>
>
> This is needed to support GRO for ESP in UDP encapsulation.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@...unet.com>
> Signed-off-by: Antony Antony <antony.antony@...unet.com>
> ---
>  net/ipv4/esp4_offload.c |  2 +-
>  net/ipv6/esp6_offload.c |  2 +-
>  net/xfrm/xfrm_input.c   | 75 +++++++++++++++++++++++------------------
>  3 files changed, 44 insertions(+), 35 deletions(-)
>
> diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
> index 3969fa805679..77bb01032667 100644
> --- a/net/ipv4/esp4_offload.c
> +++ b/net/ipv4/esp4_offload.c
> @@ -76,7 +76,7 @@ static struct sk_buff *esp4_gro_receive(struct list_head *head,
>
>         /* We don't need to handle errors from xfrm_input, it does all
>          * the error handling and frees the resources on error. */
> -       xfrm_input(skb, IPPROTO_ESP, spi, -2);
> +       xfrm_input(skb, IPPROTO_ESP, spi, 0);
>
>         return ERR_PTR(-EINPROGRESS);
>  out_reset:
> diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
> index 75c02992c520..ee5f5abdb503 100644
> --- a/net/ipv6/esp6_offload.c
> +++ b/net/ipv6/esp6_offload.c
> @@ -103,7 +103,7 @@ static struct sk_buff *esp6_gro_receive(struct list_head *head,
>
>         /* We don't need to handle errors from xfrm_input, it does all
>          * the error handling and frees the resources on error. */
> -       xfrm_input(skb, IPPROTO_ESP, spi, -2);
> +       xfrm_input(skb, IPPROTO_ESP, spi, 0);
>
>         return ERR_PTR(-EINPROGRESS);
>  out_reset:
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index c06e54a10540..ffd62ad58207 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -458,6 +458,35 @@ static int xfrm_inner_mode_input(struct xfrm_state *x,
>         return -EOPNOTSUPP;
>  }
>
> +static int xfrm_input_check_offload(struct net *net, struct sk_buff *skb,
> +                                   struct xfrm_state *x,
> +                                   struct xfrm_offload *xo)
> +{
> +       if (!(xo->status & CRYPTO_SUCCESS)) {
> +               if (xo->status &
> +                   (CRYPTO_TRANSPORT_AH_AUTH_FAILED |
> +                    CRYPTO_TRANSPORT_ESP_AUTH_FAILED |
> +                    CRYPTO_TUNNEL_AH_AUTH_FAILED |
> +                    CRYPTO_TUNNEL_ESP_AUTH_FAILED)) {
> +                       xfrm_audit_state_icvfail(x, skb,
> +                                                x->type->proto);
> +                       x->stats.integrity_failed++;
> +                       XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEPROTOERROR);
> +                       return -EINVAL;
> +               }
> +
> +               if (xo->status & CRYPTO_INVALID_PROTOCOL) {
> +                       XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEPROTOERROR);
> +                       return -EINVAL;
> +               }
> +
> +               XFRM_INC_STATS(net, LINUX_MIB_XFRMINBUFFERERROR);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
>  int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
>  {
>         const struct xfrm_state_afinfo *afinfo;
> @@ -477,7 +506,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
>         struct xfrm_offload *xo = xfrm_offload(skb);
>         struct sec_path *sp;
>
> -       if (encap_type < 0) {
> +       if (encap_type < 0 || (xo && xo->flags & XFRM_GRO)) {
>                 x = xfrm_input_state(skb);
>
>                 if (unlikely(x->km.state != XFRM_STATE_VALID)) {
> @@ -495,46 +524,26 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
>                 family = x->outer_mode.family;
>
>                 /* An encap_type of -1 indicates async resumption. */
> -               if (encap_type == -1) {
> +               if (encap_type  < 0) {

Why is this specific line change needed? I see that now -2 is not sent
anymore, so how is this related?
If it is needed, maybe the comment above also needs updating?

nit, a cover letter would've been handy so that the series could be
fetched as a whole.

Eyal.