> -----Original Message-----
> From: Steffen Klassert [mailto:[email protected]]
> Sent: Wednesday, June 21, 2017 8:37 AM
> To: Yossi Kuperman <[email protected]>
> Cc: [email protected]; Herbert Xu <[email protected]>;
> Yevgeny Kliteynik <[email protected]>; Boris Pismenny
> <[email protected]>; Ilan Tayari <[email protected]>
> 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, [email protected] wrote:
> > From: Yossi Kuperman <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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.
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?
> >
> > if (xo && (xo->flags & XFRM_GRO)) {
> > skb_mac_header_rebuild(skb);
> > --
> > 2.8.1
