linux-kernel - Re: [PATCH net v2] gso: do not skip outer ip header in case of ipip and net

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] [day] [month] [year] [list]

Date:   Fri, 18 Feb 2022 20:35:06 +0800
From:   Tao Liu <thomas.liu@...oud.cn>
To:     willemdebruijn.kernel@...il.com, Tao Liu <thomas.liu@...oud.cn>
Cc:     davem@...emloft.net, dsahern@...nel.org, edumazet@...gle.com,
        kuba@...nel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, sridhar.samudrala@...el.com,
        yoshfuji@...ux-ipv6.org
Subject: Re: [PATCH net v2] gso: do not skip outer ip header in case of ipip and net_failover

From: Willem de Bruijn <willemdebruijn.kernel@...il.com>

Sorry for late reply.

> 
> I think the message could be rewritten to point out that the issue is
> specific with the rare combination of SKB_GSO_DODGY and a tunnel
> device that adds an SKB_GSO_ tunnel option.
> 
Will do.

> > This patch also includes ipv6_gso_segment(), considering SIT, etc.
> >
> > Fixes: cb32f511a70b ("ipip: add GSO/TSO support")
> > Fixes: cfc80d9a1163 ("net: Introduce net_failover driver")
> 
> This is not a net_failover issue.
> 
Will remove it.

> I'm not sure whether the issue existed at the time tunnel support was
> added, or introduced later. It's reasonable to assume that it was
> always there, but it might be worth a quick code inspection.
> 
> > Signed-off-by: Tao Liu <thomas.liu@...oud.cn>
> > ---
> >  net/ipv4/af_inet.c     | 5 ++++-
> >  net/ipv6/ip6_offload.c | 2 ++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index 9c465ba..72fde28 100644
> > --- a/net/ipv4/af_inet.c
> > +++ b/net/ipv4/af_inet.c
> > @@ -1376,8 +1376,11 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> >         }
> >
> >         ops = rcu_dereference(inet_offloads[proto]);
> > -       if (likely(ops && ops->callbacks.gso_segment))
> > +       if (likely(ops && ops->callbacks.gso_segment)) {
> >                 segs = ops->callbacks.gso_segment(skb, features);
> > +               if (!segs)
> > +                       skb->network_header = skb_mac_header(skb) + nhoff - skb->head;
> > +       }
> >
> >         if (IS_ERR_OR_NULL(segs))
> >                 goto out;
> 
> It's unfortunate that we have to add a branch in the common path. But
> I also don't immediately see a cleaner option.
> 
Yes, it is.

Thanks.