On Wed, Jun 21, 2017 at 11:24:05AM +0000, Yossi Kuperman wrote: > > > > -----Original Message----- > > From: Steffen Klassert [mailto:steffen.klass...@secunet.com] > > Sent: Wednesday, June 21, 2017 8:37 AM > > To: Yossi Kuperman <yoss...@mellanox.com> > > Cc: netdev@vger.kernel.org; Herbert Xu <herb...@gondor.apana.org.au>; > > Yevgeny Kliteynik <klit...@mellanox.com>; Boris Pismenny > > <bor...@mellanox.com>; Ilan Tayari <il...@mellanox.com> > > Subject: Re: [PATCH net 1/2] xfrm6: Fix IPv6 payload_len in > > xfrm6_transport_finish > > > > On Mon, Jun 19, 2017 at 11:33:20AM +0300, yoss...@mellanox.com wrote: > > > From: Yossi Kuperman <yoss...@mellanox.com> > > > > > > IPv6 payload length indicates the size of the payload, including any > > > extension headers. In xfrm6_transport_finish, > > > ipv6_hdr(skb)->payload_len is set to the payload size only, regardless > > > of the presence of any extension headers. > > > > > > After ESP GRO transport mode decapsulation, ipv6_rcv trims the packet > > > according to the wrong payload_len, thus corrupting the packet. > > > > > > Set payload_len to account for extension headers as well. > > > > > > Fixes: 716062fd4c2f ("[IPSEC]: Merge most of the input path") > > > Signed-off-by: Yossi Kuperman <yoss...@mellanox.com> > > > --- > > > net/ipv6/xfrm6_input.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c index > > > 08a807b..3ef5d91 100644 > > > --- a/net/ipv6/xfrm6_input.c > > > +++ b/net/ipv6/xfrm6_input.c > > > @@ -43,8 +43,8 @@ int xfrm6_transport_finish(struct sk_buff *skb, int > > async) > > > return 1; > > > #endif > > > > > > - ipv6_hdr(skb)->payload_len = htons(skb->len); > > > __skb_push(skb, skb->data - skb_network_header(skb)); > > > + ipv6_hdr(skb)->payload_len = htons(skb->len - sizeof(struct > > > +ipv6hdr)); > > > > You mentioned that the bug happens with ESP GRO. Does this bug also > > happen in the standard codepath? If not, you might better move the above > > line into the 'if' section below. > > > > Yes, it happens only with ESP GRO.
In this case your Fixes tag is wrong. > Normally, ipv6_rcv happens first which trims the packet correctly and then > comes xfrm6_transport_finish. > Now, with ESP GRO introduced, xfrm6_transport_finish, which comes first, > modifies the packet's payload_len, > and followed by a call to ipv6_rcv which trims it wrongly. > > If I understand you right, you suggest to move the new line under the "if" > section and restore the removed line? I just want to make sure that the standard codepath works with this cange. Did you test this with the standard codepath too?
