From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750945AbdL3Lzh (ORCPT ); Sat, 30 Dec 2017 06:55:37 -0500 Received: from mail-ot0-f173.google.com ([74.125.82.173]:33844 "EHLO mail-ot0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750806AbdL3Lzf (ORCPT ); Sat, 30 Dec 2017 06:55:35 -0500 X-Google-Smtp-Source: ACJfBotvscF7A2iUD8TKNycNFJtdG11oQfWmYUTq7afV+RzG3Gh++vc5MVTYKokKS5IzmFOPuMCF2w2evIRvhtgStGw= MIME-Version: 1.0 In-Reply-To: References: <001a1137452496ffc305617e5fe0@google.com> From: Willem de Bruijn Date: Sat, 30 Dec 2017 12:54:54 +0100 Message-ID: Subject: Re: general protection fault in skb_segment To: syzbot Cc: David Miller , LKML , linux-sctp@vger.kernel.org, Network Development , nhorman@tuxdriver.com, syzkaller-bugs@googlegroups.com, Vladislav Yasevich , Marcelo Ricardo Leitner Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > So this is a packet socket writing something that apparently looks > like an SCTP packet, is only 42 bytes long, but has GSO set in its > virtio_net_hdr struct. > > It crashes in skb_segment seemingly on a NULL list_skb. > > (gdb) list *(skb_segment+0x2a4) > 0xffffffff8167cc24 is in skb_segment (net/core/skbuff.c:3566). > 3561 if (hsize < 0) > 3562 hsize = 0; > 3563 if (hsize > len || !sg) > 3564 hsize = len; > 3565 > 3566 if (!hsize && i >= nfrags && skb_headlen(list_skb) && > 3567 (skb_headlen(list_skb) == len || sg)) { > 3568 BUG_ON(skb_headlen(list_skb) > len); > 3569 > 3570 i = 0; It appears to be a packet that consists only of an sctp header. sctp_gso_segment pulls the header before calling skb_segment, after which hsize == skb_headlen(head_skb) == 0 and nfrags == 0. This check avoids the crash, but still triggers an skb_warn_bad_offload on return in __skb_gso_segment @@ -45,6 +45,13 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff *skb, struct sk_buff *segs = ERR_PTR(-EINVAL); struct sctphdr *sh; + if (!skb_has_frag_list(skb)) + goto out; A gso packet shorter than mss should perhaps just be dropped. The stack does not generate these. tcp_gso_segment does have a test if (unlikely(skb->len <= mss)) goto out; but as mss is derived from gso_size, which a packet socket controls, this may not be sufficient for this purpose. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Willem de Bruijn Date: Sat, 30 Dec 2017 11:54:54 +0000 Subject: Re: general protection fault in skb_segment Message-Id: List-Id: References: <001a1137452496ffc305617e5fe0@google.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: syzbot Cc: David Miller , LKML , linux-sctp@vger.kernel.org, Network Development , nhorman@tuxdriver.com, syzkaller-bugs@googlegroups.com, Vladislav Yasevich , Marcelo Ricardo Leitner > So this is a packet socket writing something that apparently looks > like an SCTP packet, is only 42 bytes long, but has GSO set in its > virtio_net_hdr struct. > > It crashes in skb_segment seemingly on a NULL list_skb. > > (gdb) list *(skb_segment+0x2a4) > 0xffffffff8167cc24 is in skb_segment (net/core/skbuff.c:3566). > 3561 if (hsize < 0) > 3562 hsize = 0; > 3563 if (hsize > len || !sg) > 3564 hsize = len; > 3565 > 3566 if (!hsize && i >= nfrags && skb_headlen(list_skb) && > 3567 (skb_headlen(list_skb) = len || sg)) { > 3568 BUG_ON(skb_headlen(list_skb) > len); > 3569 > 3570 i = 0; It appears to be a packet that consists only of an sctp header. sctp_gso_segment pulls the header before calling skb_segment, after which hsize = skb_headlen(head_skb) = 0 and nfrags = 0. This check avoids the crash, but still triggers an skb_warn_bad_offload on return in __skb_gso_segment @@ -45,6 +45,13 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff *skb, struct sk_buff *segs = ERR_PTR(-EINVAL); struct sctphdr *sh; + if (!skb_has_frag_list(skb)) + goto out; A gso packet shorter than mss should perhaps just be dropped. The stack does not generate these. tcp_gso_segment does have a test if (unlikely(skb->len <= mss)) goto out; but as mss is derived from gso_size, which a packet socket controls, this may not be sufficient for this purpose.