Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp17935663ybl; Thu, 2 Jan 2020 15:07:59 -0800 (PST) X-Google-Smtp-Source: APXvYqwydcekSZKBl6lTk2iMrds5NsfdGcofe19UdvTHl2SiqYGwrot9YotsSmN9qlUBpxvjbfNm X-Received: by 2002:a9d:7f11:: with SMTP id j17mr98345631otq.281.1578006479245; Thu, 02 Jan 2020 15:07:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1578006479; cv=none; d=google.com; s=arc-20160816; b=eSbh5FkvEEeZDE0qU1HQFOOzE3wBL7QSRGQ9wBtaKPqOUjPzwq7eIul+KxdlVB14BX 9ZArQ4Lp55nJeBu+wZnImaFbl9OxVUz6gvzlgp2slRi1aoTBX7ne/79PAfaad7TKSpn9 LHrTW/ugZ2PzXXy/MrCZZe2lC0ZXHhsIanA3jE0udia7yb9UD+C+mPmkqr/IgPzOKpgt BLAIvxaCd9Ao5rGo4u+Ddev7cSCDlSpN3eyPW/TXzdmVFU4+b4jr9x5IawCOSb1zcGNE dTo4+np51tQgAV5FivIiZ3H1m+n15E3haaBnl6xuLK5rudWhlDra9cPxVQpSaf9IoIQj MLfg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=r4HJNH9mONDgmzS4jxt241/YN+KXqY82E/FgYbTyw5U=; b=fhrMDvBhQKDBWcCXb3BAfJMFi0iQx6GJmcLeSXNdqDPt8wiBKgPijkSXkwm/TpokMU UFzbLPBDbAdjYjSkEtQ0jilWOpOBGCg8VWsA6dFfHjrkpiWu2sJ111PgAnI94RyrrFKU enKNZ4Ljq3puRKYGsIisxmjuE7JH19XYM0xjJ/uirsKcctKwLqIe2XhX8tXH/WxOPNd1 ZkWiwVyZo3cC/zkzI/ADLh8CSOiMZ9coqMFtDg5750fXAioScXy+EDZYgWwIZH0Qbsbz swJooPdVz2mNXJS/XH5b0yRXIsV0SFjpjmGqbXOLjx/VO1bRPLiMVb11RIGbVVg7XMjW /9JA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=KP4YzfLC; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id v26si30094069otj.0.2020.01.02.15.07.47; Thu, 02 Jan 2020 15:07:59 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=KP4YzfLC; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729103AbgABWWH (ORCPT + 99 others); Thu, 2 Jan 2020 17:22:07 -0500 Received: from mail.kernel.org ([198.145.29.99]:42068 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729086AbgABWWE (ORCPT ); Thu, 2 Jan 2020 17:22:04 -0500 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B695B222C3; Thu, 2 Jan 2020 22:22:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1578003723; bh=VOUT318r59RAxhuHzAdS0CGGtsryt1VTz1lPfrvr2xY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KP4YzfLC0EsiAI1PoeXEk/Az1uTEOZrVfaVBht04qQLQ3yVLGjIcoCVg/ORTudAjk iDStDJiBepGjQMfBgwtgJ2eSp7Q6A7Zl6KFWTSS5WWuRm2lqGGPA/AAAJfLki/KRvJ AwrlicvyzFsDUU5A+Zq/gssq2OuDCF/hjcDxcrAI= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Eric Dumazet , syzbot , "David S. Miller" Subject: [PATCH 4.19 088/114] net: icmp: fix data-race in cmp_global_allow() Date: Thu, 2 Jan 2020 23:07:40 +0100 Message-Id: <20200102220038.083133999@linuxfoundation.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200102220029.183913184@linuxfoundation.org> References: <20200102220029.183913184@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Eric Dumazet commit bbab7ef235031f6733b5429ae7877bfa22339712 upstream. This code reads two global variables without protection of a lock. We need READ_ONCE()/WRITE_ONCE() pairs to avoid load/store-tearing and better document the intent. KCSAN reported : BUG: KCSAN: data-race in icmp_global_allow / icmp_global_allow read to 0xffffffff861a8014 of 4 bytes by task 11201 on cpu 0: icmp_global_allow+0x36/0x1b0 net/ipv4/icmp.c:254 icmpv6_global_allow net/ipv6/icmp.c:184 [inline] icmpv6_global_allow net/ipv6/icmp.c:179 [inline] icmp6_send+0x493/0x1140 net/ipv6/icmp.c:514 icmpv6_send+0x71/0xb0 net/ipv6/ip6_icmp.c:43 ip6_link_failure+0x43/0x180 net/ipv6/route.c:2640 dst_link_failure include/net/dst.h:419 [inline] vti_xmit net/ipv4/ip_vti.c:243 [inline] vti_tunnel_xmit+0x27f/0xa50 net/ipv4/ip_vti.c:279 __netdev_start_xmit include/linux/netdevice.h:4420 [inline] netdev_start_xmit include/linux/netdevice.h:4434 [inline] xmit_one net/core/dev.c:3280 [inline] dev_hard_start_xmit+0xef/0x430 net/core/dev.c:3296 __dev_queue_xmit+0x14c9/0x1b60 net/core/dev.c:3873 dev_queue_xmit+0x21/0x30 net/core/dev.c:3906 neigh_direct_output+0x1f/0x30 net/core/neighbour.c:1530 neigh_output include/net/neighbour.h:511 [inline] ip6_finish_output2+0x7a6/0xec0 net/ipv6/ip6_output.c:116 __ip6_finish_output net/ipv6/ip6_output.c:142 [inline] __ip6_finish_output+0x2d7/0x330 net/ipv6/ip6_output.c:127 ip6_finish_output+0x41/0x160 net/ipv6/ip6_output.c:152 NF_HOOK_COND include/linux/netfilter.h:294 [inline] ip6_output+0xf2/0x280 net/ipv6/ip6_output.c:175 dst_output include/net/dst.h:436 [inline] ip6_local_out+0x74/0x90 net/ipv6/output_core.c:179 write to 0xffffffff861a8014 of 4 bytes by task 11183 on cpu 1: icmp_global_allow+0x174/0x1b0 net/ipv4/icmp.c:272 icmpv6_global_allow net/ipv6/icmp.c:184 [inline] icmpv6_global_allow net/ipv6/icmp.c:179 [inline] icmp6_send+0x493/0x1140 net/ipv6/icmp.c:514 icmpv6_send+0x71/0xb0 net/ipv6/ip6_icmp.c:43 ip6_link_failure+0x43/0x180 net/ipv6/route.c:2640 dst_link_failure include/net/dst.h:419 [inline] vti_xmit net/ipv4/ip_vti.c:243 [inline] vti_tunnel_xmit+0x27f/0xa50 net/ipv4/ip_vti.c:279 __netdev_start_xmit include/linux/netdevice.h:4420 [inline] netdev_start_xmit include/linux/netdevice.h:4434 [inline] xmit_one net/core/dev.c:3280 [inline] dev_hard_start_xmit+0xef/0x430 net/core/dev.c:3296 __dev_queue_xmit+0x14c9/0x1b60 net/core/dev.c:3873 dev_queue_xmit+0x21/0x30 net/core/dev.c:3906 neigh_direct_output+0x1f/0x30 net/core/neighbour.c:1530 neigh_output include/net/neighbour.h:511 [inline] ip6_finish_output2+0x7a6/0xec0 net/ipv6/ip6_output.c:116 __ip6_finish_output net/ipv6/ip6_output.c:142 [inline] __ip6_finish_output+0x2d7/0x330 net/ipv6/ip6_output.c:127 ip6_finish_output+0x41/0x160 net/ipv6/ip6_output.c:152 NF_HOOK_COND include/linux/netfilter.h:294 [inline] ip6_output+0xf2/0x280 net/ipv6/ip6_output.c:175 Reported by Kernel Concurrency Sanitizer on: CPU: 1 PID: 11183 Comm: syz-executor.2 Not tainted 5.4.0-rc3+ #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Fixes: 4cdf507d5452 ("icmp: add a global rate limitation") Signed-off-by: Eric Dumazet Reported-by: syzbot Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/ipv4/icmp.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -254,10 +254,11 @@ bool icmp_global_allow(void) bool rc = false; /* Check if token bucket is empty and cannot be refilled - * without taking the spinlock. + * without taking the spinlock. The READ_ONCE() are paired + * with the following WRITE_ONCE() in this same function. */ - if (!icmp_global.credit) { - delta = min_t(u32, now - icmp_global.stamp, HZ); + if (!READ_ONCE(icmp_global.credit)) { + delta = min_t(u32, now - READ_ONCE(icmp_global.stamp), HZ); if (delta < HZ / 50) return false; } @@ -267,14 +268,14 @@ bool icmp_global_allow(void) if (delta >= HZ / 50) { incr = sysctl_icmp_msgs_per_sec * delta / HZ ; if (incr) - icmp_global.stamp = now; + WRITE_ONCE(icmp_global.stamp, now); } credit = min_t(u32, icmp_global.credit + incr, sysctl_icmp_msgs_burst); if (credit) { credit--; rc = true; } - icmp_global.credit = credit; + WRITE_ONCE(icmp_global.credit, credit); spin_unlock(&icmp_global.lock); return rc; }