From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> To: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Cc: syzbot <syzbot+fee64147a25aecd48055@syzkaller.appspotmail.com>, David Miller <davem@davemloft.net>, LKML <linux-kernel@vger.kernel.org>, linux-sctp@vger.kernel.org, Network Development <netdev@vger.kernel.org>, nhorman@tuxdriver.com, syzkaller-bugs@googlegroups.com, vyasevich@gmail.com Subject: Re: general protection fault in skb_segment Date: Sun, 31 Dec 2017 00:25:24 -0200 [thread overview] Message-ID: <20171231022524.GE22042@localhost.localdomain> (raw) In-Reply-To: <20171231005220.GD22042@localhost.localdomain> On Sat, Dec 30, 2017 at 10:52:20PM -0200, Marcelo Ricardo Leitner wrote: > On Sat, Dec 30, 2017 at 08:42:41AM +0100, Willem de Bruijn wrote: [...] > > Somewhat tangential, but any PF_PACKET socket can set this > > magic gso_size value in its virtio_net_hdr, so if it is assumed to > > be an SCTP GSO specific option, setting it for a TCP GSO packet > > may also cause unexpected results. > > It seems virtio_net could use more sanity checks. When PACKET_VNET_HDR > is used, it will end up calling: > tpacket_rcv() { > ... > if (do_vnet) { > if (virtio_net_hdr_from_skb(skb, h.raw + macoff - > sizeof(struct virtio_net_hdr), > vio_le(), true)) { > spin_lock(&sk->sk_receive_queue.lock); > goto drop_n_account; > } > } > > and virtio_net_hdr_from_skb does: > if (skb_is_gso(skb)) { > ... > if (sinfo->gso_type & SKB_GSO_TCPV4) > hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4; > else if (sinfo->gso_type & SKB_GSO_TCPV6) > hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6; > else > return -EINVAL; > > Meaning that any gso_type other than TCP would be rejected, but this > SCTP one got through. Seems the header contains a sctp header, but the > gso_type set was actually pointing to TCP (otherwise it would have > been rejected). AFAICT if this packet had an ESP header, for example, > it could have hit esp4_gso_segment. Can you please confirm this? I added: --- a/net/sctp/offload.c +++ b/net/sctp/offload.c @@ -44,6 +44,18 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff *skb, { struct sk_buff *segs = ERR_PTR(-EINVAL); struct sctphdr *sh; + int fail = 0; + + if (!(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP)) { + printk("Bogus gso_type: %x\n", skb_shinfo(skb)->gso_type); + fail = 1; + } + if (skb_shinfo(skb)->gso_size != GSO_BY_FRAGS) { + printk("Bogus gso_size: %u\n", skb_shinfo(skb)->gso_size); + fail = 1; + } + if (fail) + goto out; sh = sctp_hdr(skb); if (!pskb_may_pull(skb, sizeof(*sh))) and with the reproducer, got: [ 54.255469] Bogus gso_type: 7 [ 54.258801] Bogus gso_size: 63464 [ 54.262532] ------------[ cut here ]------------ [ 54.267703] syz0: caps=(0x00000800000058c1, 0x0000000000000000) len=32 data_len=0 gso_size=63464 gso_type=7 ip_summed0 [ 54.279777] WARNING: CPU: 1 PID: 13005 at /root/linux/net/core/dev.c:2600 skb_warn_bad_offload+0xd6/0xec gso_type 7 = SKB_GSO_TCPV4 | SKB_GSO_DODGY | SKB_GSO_TCP_ECN as the warn indicated too. Once this gets to sctp_gso_segment, it's too late to avoid the warning. Would be nice if we could somehow filter this earlier in the process. Marcelo
WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> To: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Cc: syzbot <syzbot+fee64147a25aecd48055@syzkaller.appspotmail.com>, David Miller <davem@davemloft.net>, LKML <linux-kernel@vger.kernel.org>, linux-sctp@vger.kernel.org, Network Development <netdev@vger.kernel.org>, nhorman@tuxdriver.com, syzkaller-bugs@googlegroups.com, vyasevich@gmail.com Subject: Re: general protection fault in skb_segment Date: Sun, 31 Dec 2017 02:25:24 +0000 [thread overview] Message-ID: <20171231022524.GE22042@localhost.localdomain> (raw) In-Reply-To: <20171231005220.GD22042@localhost.localdomain> On Sat, Dec 30, 2017 at 10:52:20PM -0200, Marcelo Ricardo Leitner wrote: > On Sat, Dec 30, 2017 at 08:42:41AM +0100, Willem de Bruijn wrote: [...] > > Somewhat tangential, but any PF_PACKET socket can set this > > magic gso_size value in its virtio_net_hdr, so if it is assumed to > > be an SCTP GSO specific option, setting it for a TCP GSO packet > > may also cause unexpected results. > > It seems virtio_net could use more sanity checks. When PACKET_VNET_HDR > is used, it will end up calling: > tpacket_rcv() { > ... > if (do_vnet) { > if (virtio_net_hdr_from_skb(skb, h.raw + macoff - > sizeof(struct virtio_net_hdr), > vio_le(), true)) { > spin_lock(&sk->sk_receive_queue.lock); > goto drop_n_account; > } > } > > and virtio_net_hdr_from_skb does: > if (skb_is_gso(skb)) { > ... > if (sinfo->gso_type & SKB_GSO_TCPV4) > hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4; > else if (sinfo->gso_type & SKB_GSO_TCPV6) > hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6; > else > return -EINVAL; > > Meaning that any gso_type other than TCP would be rejected, but this > SCTP one got through. Seems the header contains a sctp header, but the > gso_type set was actually pointing to TCP (otherwise it would have > been rejected). AFAICT if this packet had an ESP header, for example, > it could have hit esp4_gso_segment. Can you please confirm this? I added: --- a/net/sctp/offload.c +++ b/net/sctp/offload.c @@ -44,6 +44,18 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff *skb, { struct sk_buff *segs = ERR_PTR(-EINVAL); struct sctphdr *sh; + int fail = 0; + + if (!(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP)) { + printk("Bogus gso_type: %x\n", skb_shinfo(skb)->gso_type); + fail = 1; + } + if (skb_shinfo(skb)->gso_size != GSO_BY_FRAGS) { + printk("Bogus gso_size: %u\n", skb_shinfo(skb)->gso_size); + fail = 1; + } + if (fail) + goto out; sh = sctp_hdr(skb); if (!pskb_may_pull(skb, sizeof(*sh))) and with the reproducer, got: [ 54.255469] Bogus gso_type: 7 [ 54.258801] Bogus gso_size: 63464 [ 54.262532] ------------[ cut here ]------------ [ 54.267703] syz0: caps=(0x00000800000058c1, 0x0000000000000000) len2 data_len=0 gso_sizec464 gso_type=7 ip_summed0 [ 54.279777] WARNING: CPU: 1 PID: 13005 at /root/linux/net/core/dev.c:2600 skb_warn_bad_offload+0xd6/0xec gso_type 7 = SKB_GSO_TCPV4 | SKB_GSO_DODGY | SKB_GSO_TCP_ECN as the warn indicated too. Once this gets to sctp_gso_segment, it's too late to avoid the warning. Would be nice if we could somehow filter this earlier in the process. Marcelo
next prev parent reply other threads:[~2017-12-31 2:25 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-12-29 17:58 general protection fault in skb_segment syzbot 2017-12-30 7:42 ` Willem de Bruijn 2017-12-30 7:42 ` Willem de Bruijn 2017-12-30 11:54 ` Willem de Bruijn 2017-12-30 11:54 ` Willem de Bruijn 2017-12-30 13:51 ` Xin Long 2017-12-30 13:51 ` Xin Long 2017-12-31 0:52 ` Marcelo Ricardo Leitner 2017-12-31 0:52 ` Marcelo Ricardo Leitner 2017-12-31 2:25 ` Marcelo Ricardo Leitner [this message] 2017-12-31 2:25 ` Marcelo Ricardo Leitner 2017-12-31 8:41 ` Xin Long 2017-12-31 8:41 ` Xin Long 2017-12-31 9:17 ` Willem de Bruijn 2017-12-31 9:17 ` Willem de Bruijn 2017-12-31 9:52 ` Willem de Bruijn 2017-12-31 9:52 ` Willem de Bruijn 2018-01-02 15:48 ` Willem de Bruijn 2018-01-02 15:48 ` Willem de Bruijn 2018-01-02 17:23 ` Willem de Bruijn 2018-01-02 17:23 ` Willem de Bruijn 2018-01-16 20:32 ` Willem de Bruijn 2018-01-16 20:32 ` Willem de Bruijn
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171231022524.GE22042@localhost.localdomain \ --to=marcelo.leitner@gmail.com \ --cc=davem@davemloft.net \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-sctp@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=nhorman@tuxdriver.com \ --cc=syzbot+fee64147a25aecd48055@syzkaller.appspotmail.com \ --cc=syzkaller-bugs@googlegroups.com \ --cc=vyasevich@gmail.com \ --cc=willemdebruijn.kernel@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.