On Mon, 2016-11-28 at 11:40 -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Nov 28, 2016 at 06:26:49AM -0800, Eric Dumazet escreveu: > > From: Eric Dumazet <eduma...@google.com> > > > > pskb_may_pull() can reallocate skb->head, we need to reload dh pointer > > in dccp_invalid_packet() or risk use after free. > > > > Bug found by Andrey Konovalov using syzkaller. > > > > Signed-off-by: Eric Dumazet <eduma...@google.com> > > Reported-by: Andrey Konovalov <andreyk...@google.com> > > Acked-by: Arnaldo Carvalho de Melo <a...@redhat.com> > > I was about to send exactly this patch, and while looking at it I think > the patch below needs to go in as well, no? To follow the advice of that > Warning line there :-) > > From: Arnaldo Carvalho de Melo <a...@redhat.com> > > pskb_may_pull() can reallocate skb->head, so we can't access > iph->frag_off or risk use after free, save it to a variable and us that > later. > > Cc: Andrey Konovalov <andreyk...@google.com> > Cc: Eric Dumazet <eduma...@google.com> > Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com> > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > index 5ddf5cda07f4..9462070561a3 100644 > --- a/net/ipv4/af_inet.c > +++ b/net/ipv4/af_inet.c > @@ -1198,6 +1198,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, > struct iphdr *iph; > int proto, tot_len; > int nhoff; > + u16 frag_off; > int ihl; > int id; > > @@ -1213,6 +1214,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, > > id = ntohs(iph->id); > proto = iph->protocol; > + frag_off = iph->frag_off; > > /* Warning: after this point, iph might be no longer valid */ > if (unlikely(!pskb_may_pull(skb, ihl))) > @@ -1233,7 +1235,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, > fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID); > > /* fixed ID is invalid if DF bit is not set */ > - if (fixedid && !(iph->frag_off & htons(IP_DF))) > + if (fixedid && !(frag_off & htons(IP_DF))) > goto out; > } >
I do not see why this patch would be needed ?
