Re: [PATCH] net: geneve: modify IP header check in geneve6_xmit_skb

From: Phillip Potter <phil@philpotter.co.uk>
To: Florian Westphal <fw@strlen.de>
Cc: davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, edumazet@google.com
Subject: Re: [PATCH] net: geneve: modify IP header check in geneve6_xmit_skb
Date: Thu, 22 Apr 2021 01:56:54 +0100	[thread overview]
Message-ID: <YIDJ1npZ7h4Vt0mi@equinox> (raw)
In-Reply-To: <20210422003942.GF4841@breakpoint.cc>

On Thu, Apr 22, 2021 at 02:39:42AM +0200, Florian Westphal wrote:
> Phillip Potter <phil@philpotter.co.uk> wrote:
> > Modify the check in geneve6_xmit_skb to use the size of a struct iphdr
> > rather than struct ipv6hdr. This fixes two kernel selftest failures
> > introduced by commit 6628ddfec758
> > ("net: geneve: check skb is large enough for IPv4/IPv6 header"), without
> > diminishing the fix provided by that commit.
> 
> What errors?
> 
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
> > ---
> >  drivers/net/geneve.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> > index 42f31c681846..a57a5e6f614f 100644
> > --- a/drivers/net/geneve.c
> > +++ b/drivers/net/geneve.c
> > @@ -988,7 +988,7 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> >  	__be16 sport;
> >  	int err;
> >  
> > -	if (!pskb_network_may_pull(skb, sizeof(struct ipv6hdr)))
> > +	if (!pskb_network_may_pull(skb, sizeof(struct iphdr)))
> >  		return -EINVAL;
> 
> Seems this is papering over some bug, this change makes no sense to
> me.  Can you please explain this?

Dear Florian,

I made the change to fix failures in the following tests:
IPv4 over geneve6: PMTU exceptions
IPv4 over geneve6: PMTU exceptions - nexthop objects

Error for both tests was:
PMTU exception wasn't created after exceeding link layer MTU on geneve interface

I was notified by the kernel test reobot due to the failures being caused by my
previous patch to this file. Sorry if I've done this the wrong way, just didn't
want to hold anyone up. I also tested the patch with syzbot again to make sure
it still fixed the original problem from my last commit.

Regards,
Phil