From patchwork Mon Jun 12 16:52:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Wang X-Patchwork-Id: 774741 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 3wmf9C6Vqcz9s3s for ; Tue, 13 Jun 2017 02:53:19 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="UgbyGCKv"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754209AbdFLQxR (ORCPT ); Mon, 12 Jun 2017 12:53:17 -0400 Received: from mail-pg0-f68.google.com ([74.125.83.68]:34596 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753680AbdFLQxQ (ORCPT ); Mon, 12 Jun 2017 12:53:16 -0400 Received: by mail-pg0-f68.google.com with SMTP id v14so14756433pgn.1 for ; Mon, 12 Jun 2017 09:53:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=wfVN8uYP8pKgD0IUK0txytRjRutMgsTXuRJ7oPeT2d8=; b=UgbyGCKvP+1vEFnZnL0nAIv8YVE+g+qrB6JLFBNGQZo5GVqONXLCUorlFCYKrNv4ql rPjJk78cZA1DyvlE6AdYT5F/WM7UrtBXP+SUM1rgak4LxqsvJdwhkkJkTif32hdz65eR pwox4NbxPanOvWGCCrqS6l7Ml8L/n50alrPKCUZOSbX8Av+Suv5ljvi4zvalXbDpNTXj vhEfZmLUDxvYtO57zIP+1Kp6xbq00opcd/wM1vx5LvwFNAoTFLs+JGqb6iXqBbZMKkR/ L9BxoL67hPpj6adZAsRnNTZAY5jZ4LWg8bWvtQ1ElI0fXQNvNe0QPVUaG9q8NZEtdRWQ P14g== 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; bh=wfVN8uYP8pKgD0IUK0txytRjRutMgsTXuRJ7oPeT2d8=; b=M0pgBevAbXRu0ZIV3GoDA3t95mgXHPyjgx7XXoxLrJu3bYUuc+w8PzVUzRISY6vDsF O3JQu1fPbWzBhahfzZcsd0h33SvBZ2eD6f2k6pgJ27xkSGlavrBMi+Iek9JBmYvCo8Aa xQ4XqymRpe0ihjLMBzjoc4JWaOViGsy1ZN7WFfl6uTsTHggXP8rhQj6BU2QdrzJ8tfXv s3aUuUWwUxySadFM560/jE+Ru+7wZAFjZxwwNN3wx6vW61LEUxSFn9IT7HXGUK0ITCTs l7AiABXR7TsmOVvA7p0cg6j/WJJL7yjgZvbqVfOfu4+P30Hw1oYQ4sIhf1STg7Y6JCYZ Qgjw== X-Gm-Message-State: AODbwcCamVB0FRG251EtqTY/sINPneqSVtyxtSYJFiHkckJg1QzQL9e6 yaoQ43Lf8ub6K9Ym87g= X-Received: by 10.84.225.145 with SMTP id u17mr58817219plj.241.1497286385001; Mon, 12 Jun 2017 09:53:05 -0700 (PDT) Received: from tw-172-25-30-113.office.twttr.net ([8.25.197.25]) by smtp.gmail.com with ESMTPSA id q135sm17599401pgq.41.2017.06.12.09.53.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 12 Jun 2017 09:53:04 -0700 (PDT) From: Cong Wang To: netdev@vger.kernel.org Cc: andreyknvl@google.com, Cong Wang , Eric Dumazet , Xin Long Subject: [Patch net] igmp: acquire pmc lock for ip_mc_clear_src() Date: Mon, 12 Jun 2017 09:52:26 -0700 Message-Id: <1497286346-26888-1-git-send-email-xiyou.wangcong@gmail.com> X-Mailer: git-send-email 2.5.5 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Andrey reported a use-after-free in add_grec(): for (psf = *psf_list; psf; psf = psf_next) { ... psf_next = psf->sf_next; where the struct ip_sf_list's were already freed by: kfree+0xe8/0x2b0 mm/slub.c:3882 ip_mc_clear_src+0x69/0x1c0 net/ipv4/igmp.c:2078 ip_mc_dec_group+0x19a/0x470 net/ipv4/igmp.c:1618 ip_mc_drop_socket+0x145/0x230 net/ipv4/igmp.c:2609 inet_release+0x4e/0x1c0 net/ipv4/af_inet.c:411 sock_release+0x8d/0x1e0 net/socket.c:597 sock_close+0x16/0x20 net/socket.c:1072 This happens because we don't hold pmc->lock in ip_mc_clear_src() and a parallel mr_ifc_timer timer could jump in and access them. The RCU lock is there but it is merely for pmc itself, this spinlock could actually ensure we don't access them in parallel. Thanks to Eric and Long for discussion on this bug. Reported-by: Andrey Konovalov Cc: Eric Dumazet Cc: Xin Long Signed-off-by: Cong Wang Reviewed-by: Xin Long --- net/ipv4/igmp.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c index 44fd86d..8f6b5bb 100644 --- a/net/ipv4/igmp.c +++ b/net/ipv4/igmp.c @@ -2071,21 +2071,26 @@ static int ip_mc_add_src(struct in_device *in_dev, __be32 *pmca, int sfmode, static void ip_mc_clear_src(struct ip_mc_list *pmc) { - struct ip_sf_list *psf, *nextpsf; + struct ip_sf_list *psf, *nextpsf, *tomb, *sources; - for (psf = pmc->tomb; psf; psf = nextpsf) { + spin_lock_bh(&pmc->lock); + tomb = pmc->tomb; + pmc->tomb = NULL; + sources = pmc->sources; + pmc->sources = NULL; + pmc->sfmode = MCAST_EXCLUDE; + pmc->sfcount[MCAST_INCLUDE] = 0; + pmc->sfcount[MCAST_EXCLUDE] = 1; + spin_unlock_bh(&pmc->lock); + + for (psf = tomb; psf; psf = nextpsf) { nextpsf = psf->sf_next; kfree(psf); } - pmc->tomb = NULL; - for (psf = pmc->sources; psf; psf = nextpsf) { + for (psf = sources; psf; psf = nextpsf) { nextpsf = psf->sf_next; kfree(psf); } - pmc->sources = NULL; - pmc->sfmode = MCAST_EXCLUDE; - pmc->sfcount[MCAST_INCLUDE] = 0; - pmc->sfcount[MCAST_EXCLUDE] = 1; } /* Join a multicast group