From patchwork Thu Nov 10 20:50:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 693475 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3tFFYb1BVsz9t1L for ; Fri, 11 Nov 2016 07:50:27 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="Ea6Zils5"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966285AbcKJUuW (ORCPT ); Thu, 10 Nov 2016 15:50:22 -0500 Received: from mail-pf0-f194.google.com ([209.85.192.194]:33830 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966280AbcKJUuV (ORCPT ); Thu, 10 Nov 2016 15:50:21 -0500 Received: by mail-pf0-f194.google.com with SMTP id y68so5326381pfb.1 for ; Thu, 10 Nov 2016 12:50:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=LymQtFG1jbsbh/NKC5rvS15KymubIHOTneaIa3loO9Q=; b=Ea6Zils5eBkf/eJXsl1lx9M5UeSGV4r8r+2UIpXPkSUy51Gsnn19MhXU1CpLUxzQWP iIJSPu3NK2QhrQU6kIXrjLNGYCjroeL/yfJrmVoDCf7C7NXiwzo+G39kIBvNwpgF8Hp8 f1BjQirzLsFJHqz6BvwX5eGjBx0nAj+5A2dNrrba5WNOdh7GntIsmJGjz+roDMgQ6Fhi XITn/VL+izuyc1mUJyI+1xPSdzHMi4khIOxKvR8BAIkpChbjcMOHpwT8gNpfjD1nGjCn X7sE9ndQCaa2WZG/EwJo8b9EgRXGst6QYcQzifVGuAYKXO32FiRLZ09h8FeNC99mj2N5 RG9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=LymQtFG1jbsbh/NKC5rvS15KymubIHOTneaIa3loO9Q=; b=ar3Eu6H9ozU0mLjoUaAiNpqReLMIBvMhDTXaXm+XZtdHWjJofdcY4Bx21SgJj5t0+v bafSfhngFRYpjrYEwRBwT6NJPub2xT75gyUHwAM3/8ofv1+cZ7UMTl/ZNpuMWcSE2EYI 4hggOHxQpffhXS6erGtXFaiw9OgVwHL5m0dBsAG3/jUufOoH5oRCyzrbr5oWQ8HWUtE4 zUicSIgfowV6g+rAgO7Njva8WcJPM4pVwIih4KL74Kt8cQ4yA6RuFBZIfgr32hhZ1jh6 Qkyc4TTI2cFsEgBL4h/3xkcgrGK/dmuY/A5Mj1lqJA+Q+YDPc61NyL/fRl2nKe2233nK P6cg== X-Gm-Message-State: ABUngveEiphR8uLnZJPtBlaXxwC9+eKJcvET7EiIbh2po0t9lxSmmfKe8GqgiK9eKUcA7g== X-Received: by 10.99.161.2 with SMTP id b2mr38978202pgf.5.1478811018404; Thu, 10 Nov 2016 12:50:18 -0800 (PST) Received: from ?IPv6:2620:0:1000:1704:b0c3:2d76:4ddd:2c37? ([2620:0:1000:1704:b0c3:2d76:4ddd:2c37]) by smtp.googlemail.com with ESMTPSA id c2sm9342746pal.42.2016.11.10.12.50.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 10 Nov 2016 12:50:17 -0800 (PST) Message-ID: <1478811016.8455.33.camel@edumazet-glaptop3.roam.corp.google.com> Subject: [PATCH net] tcp: take care of truncations done by sk_filter() From: Eric Dumazet To: Vladis Dronov , David Miller Cc: netdev@vger.kernel.org, Marco Grassi Date: Thu, 10 Nov 2016 12:50:16 -0800 In-Reply-To: <1478808780.8455.23.camel@edumazet-glaptop3.roam.corp.google.com> References: <1623420310.11961160.1478789246631.JavaMail.zimbra@redhat.com> <1478792652.8455.3.camel@edumazet-glaptop3.roam.corp.google.com> <1478805992.8455.20.camel@edumazet-glaptop3.roam.corp.google.com> <1478807366.8455.21.camel@edumazet-glaptop3.roam.corp.google.com> <1478808780.8455.23.camel@edumazet-glaptop3.roam.corp.google.com> X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Eric Dumazet With syzkaller help, Marco Grassi found a bug in TCP stack, crashing in tcp_collapse() Root cause is that sk_filter() can truncate the incoming skb, but TCP stack was not really expecting this to happen. It probably was expecting a simple DROP or ACCEPT behavior. We first need to make sure no part of TCP header could be removed. Then we need to adjust TCP_SKB_CB(skb)->end_seq Many thanks to syzkaller team and Marco for giving us a reproducer. Signed-off-by: Eric Dumazet Reported-by: Marco Grassi Reported-by: Vladis Dronov --- include/net/tcp.h | 1 + net/ipv4/tcp_ipv4.c | 17 ++++++++++++++++- net/ipv6/tcp_ipv6.c | 5 +++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/include/net/tcp.h b/include/net/tcp.h index 304a8e17bc87..b572f722dbdc 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -1220,6 +1220,7 @@ static inline void tcp_prequeue_init(struct tcp_sock *tp) bool tcp_prequeue(struct sock *sk, struct sk_buff *skb); bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb); +struct tcphdr *tcp_filter(struct sock *sk, struct sk_buff *skb); #undef STATE_TRACE diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 61b7be303eec..cad14d1bc5a5 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1564,6 +1564,20 @@ bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb) } EXPORT_SYMBOL(tcp_add_backlog); +struct tcphdr *tcp_filter(struct sock *sk, struct sk_buff *skb) +{ + struct tcphdr *th = (struct tcphdr *)skb->data; + unsigned int eaten = skb->len; + + if (sk_filter_trim_cap(sk, skb, th->doff * 4)) + return NULL; + eaten -= skb->len; + TCP_SKB_CB(skb)->end_seq -= eaten; + + return (struct tcphdr *)skb->data; +} +EXPORT_SYMBOL(tcp_filter); + /* * From tcp_input.c */ @@ -1676,7 +1690,8 @@ int tcp_v4_rcv(struct sk_buff *skb) nf_reset(skb); - if (sk_filter(sk, skb)) + th = tcp_filter(sk, skb); + if (!th) goto discard_and_relse; skb->dev = NULL; diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 6ca23c2e76f7..208552750550 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1229,7 +1229,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb) if (skb->protocol == htons(ETH_P_IP)) return tcp_v4_do_rcv(sk, skb); - if (sk_filter(sk, skb)) + if (!tcp_filter(sk, skb)) goto discard; /* @@ -1457,7 +1457,8 @@ static int tcp_v6_rcv(struct sk_buff *skb) if (tcp_v6_inbound_md5_hash(sk, skb)) goto discard_and_relse; - if (sk_filter(sk, skb)) + th = tcp_filter(sk, skb); + if (!th) goto discard_and_relse; skb->dev = NULL;