From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5568C282C3 for ; Tue, 22 Jan 2019 17:43:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A36FB20879 for ; Tue, 22 Jan 2019 17:43:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="G9CNSBF7" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726134AbfAVRnw (ORCPT ); Tue, 22 Jan 2019 12:43:52 -0500 Received: from mail-pf1-f194.google.com ([209.85.210.194]:36627 "EHLO mail-pf1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725896AbfAVRnw (ORCPT ); Tue, 22 Jan 2019 12:43:52 -0500 Received: by mail-pf1-f194.google.com with SMTP id b85so12110917pfc.3 for ; Tue, 22 Jan 2019 09:43:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=bJgilm9lW1AP9Ot32a2pKROuBji6bMvzYVwVJOCXqko=; b=G9CNSBF787VxWzrhM3zRqT5J69SgAO2ljAdM/8orF9QJatMNT/M7fccbs321BF1bra b3JoaseOWYaWkPrNpflZFu5KhBbnGTklx8JLUehNeVSUHxlHpjXbhZByOoiuEJzMngme MRdEUucowGJFh+1EXdxp62CQ7uX0EdBvw3bBE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=bJgilm9lW1AP9Ot32a2pKROuBji6bMvzYVwVJOCXqko=; b=r9Ko2a2sqQn18Ydcufwo9ZqEXrhJLdALdGUKrQzqAEVUIKDcckKY41AY36OqA7Cx1L RPaTgCM4LCKz4EYjE2qjJHFXRRosho4WmoT2X6MB4A2yetzge4AuIES9sDN7Ts0UHSn5 D/dG16/nwDkFVmd5pX+7nDAg8Se/IwgWhE4QLLXvJtO0NAdTxrc/UWlGLD9S5/EEa4YV mRluzzP/LTkmpnALhq1Kw3Ia48Hi/eXixuN19XeHGKKktB5mJ6iM4r68iL9PtQGmZTlg KZCuE41Kfno1XLbL4YGVs2EjnnN95W80eJAcJ2ODlIoMkBBFjH5eHIzgsATX5G+4eiBF nUlg== X-Gm-Message-State: AJcUukeeZFiUoc8ZLRGQUTSvgw8FGHeIgY0tLG3g6Q81rIlLA4sI9KQA XVBDKWAjrRUvMuN9uBzAKGor6WKKX3oSeQ== X-Google-Smtp-Source: ALg8bN7m+EhjdZAFD7l19r/5FPgdzD5/Z5Uzdlc4vUKyfEBtNL5w1e0n8hP5gmQ8DbS2vM/2SAwd3w== X-Received: by 2002:a63:cd4c:: with SMTP id a12mr33535608pgj.252.1548179030706; Tue, 22 Jan 2019 09:43:50 -0800 (PST) Received: from zsm-linux.mtv.corp.google.com ([2620:15c:202:201:49ea:b78f:4f04:4d25]) by smtp.googlemail.com with ESMTPSA id c4sm35079918pfm.151.2019.01.22.09.43.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 22 Jan 2019 09:43:49 -0800 (PST) From: Zubin Mithra To: netdev@vger.kernel.org Cc: gregkh@linuxfoundation.org, posk@google.com, edumazet@google.com, willemb@google.com, davem@davemloft.net, zsm@chromium.org Subject: [PATCH v4.19.y] ip: fail fast on IP defrag errors Date: Tue, 22 Jan 2019 09:43:44 -0800 Message-Id: <20190122174344.112456-1-zsm@chromium.org> X-Mailer: git-send-email 2.20.1.321.g9e740568ce-goog MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Peter Oskolkov commit 0ff89efb524631ac9901b81446b453c29711c376 upstream The current behavior of IP defragmentation is inconsistent: - some overlapping/wrong length fragments are dropped without affecting the queue; - most overlapping fragments cause the whole frag queue to be dropped. This patch brings consistency: if a bad fragment is detected, the whole frag queue is dropped. Two major benefits: - fail fast: corrupted frag queues are cleared immediately, instead of by timeout; - testing of overlapping fragments is now much easier: any kind of random fragment length mutation now leads to the frag queue being discarded (IP packet dropped); before this patch, some overlaps were "corrected", with tests not seeing expected packet drops. Note that in one case (see "if (end&7)" conditional) the current behavior is preserved as there are concerns that this could be legitimate padding. Signed-off-by: Peter Oskolkov Reviewed-by: Eric Dumazet Reviewed-by: Willem de Bruijn Signed-off-by: David S. Miller Signed-off-by: Zubin Mithra --- Backport Note: - Syzkaller reported a UAF, as 0ff89efb5246 ("ip: fail fast on IP defrag errors") was not applied prior to applying d5f9565c8d5a ("net: ipv4: do not handle duplicate fragments as overlapping"). Conflicts occur when 0ff89efb5246 is now applied onto 4.14.y/4.19.y, which this patch addresses. - An alternative to this patch would be to do the following :- - revert "net: ipv4: do not handle duplicate fragments as overlapping" (d5f9565c8d5ad on 4.19.y, 95b4b711444a on 4.14.y) - apply "ip: fail fast on IP defrag errors" (0ff89efb5246) - apply "net: ipv4: do not handle duplicate fragments as overlapping" (ade446403bfb) net/ipv4/ip_fragment.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c index f8bbd693c19c2..03576ff7557e0 100644 --- a/net/ipv4/ip_fragment.c +++ b/net/ipv4/ip_fragment.c @@ -382,7 +382,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) */ if (end < qp->q.len || ((qp->q.flags & INET_FRAG_LAST_IN) && end != qp->q.len)) - goto err; + goto discard_qp; qp->q.flags |= INET_FRAG_LAST_IN; qp->q.len = end; } else { @@ -394,20 +394,20 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) if (end > qp->q.len) { /* Some bits beyond end -> corruption. */ if (qp->q.flags & INET_FRAG_LAST_IN) - goto err; + goto discard_qp; qp->q.len = end; } } if (end == offset) - goto err; + goto discard_qp; err = -ENOMEM; if (!pskb_pull(skb, skb_network_offset(skb) + ihl)) - goto err; + goto discard_qp; err = pskb_trim_rcsum(skb, end - offset); if (err) - goto err; + goto discard_qp; /* Note : skb->rbnode and skb->dev share the same location. */ dev = skb->dev; @@ -425,6 +425,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) * fragment. */ + err = -EINVAL; /* Find out where to put this fragment. */ prev_tail = qp->q.fragments_tail; if (!prev_tail) @@ -433,7 +434,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) /* This is the common case: skb goes to the end. */ /* Detect and discard overlaps. */ if (offset < prev_tail->ip_defrag_offset + prev_tail->len) - goto discard_qp; + goto overlap; if (offset == prev_tail->ip_defrag_offset + prev_tail->len) ip4_frag_append_to_last_run(&qp->q, skb); else @@ -456,7 +457,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) end <= skb1_run_end) goto err; /* No new data, potential duplicate */ else - goto discard_qp; /* Found an overlap */ + goto overlap; /* Found an overlap */ } while (*rbn); /* Here we have parent properly set, and rbn pointing to * one of its NULL left/right children. Insert skb. @@ -493,16 +494,18 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) skb->_skb_refdst = 0UL; err = ip_frag_reasm(qp, skb, prev_tail, dev); skb->_skb_refdst = orefdst; + if (err) + inet_frag_kill(&qp->q); return err; } skb_dst_drop(skb); return -EINPROGRESS; +overlap: + __IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS); discard_qp: inet_frag_kill(&qp->q); - err = -EINVAL; - __IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS); err: kfree_skb(skb); return err; -- 2.20.1.321.g9e740568ce-goog