From patchwork Thu Oct 19 19:00:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Craig Gallek X-Patchwork-Id: 828311 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yHytW4jqkz9t4b for ; Fri, 20 Oct 2017 06:00:35 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752925AbdJSTAd (ORCPT ); Thu, 19 Oct 2017 15:00:33 -0400 Received: from mail-io0-f196.google.com ([209.85.223.196]:44465 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752866AbdJSTAc (ORCPT ); Thu, 19 Oct 2017 15:00:32 -0400 Received: by mail-io0-f196.google.com with SMTP id m16so10924709iod.1 for ; Thu, 19 Oct 2017 12:00:32 -0700 (PDT) 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=QX2JHOuB+BglwR204EGOoGJIK1HuHk2LYvWpWaniag8=; b=LgOPDqdprIdUERIwLbMqNi7116uLm7qkRLKTij7F9yPOLwiVehneD0/CtRaM7psfTq kxs53m+NjgcyRded6o781dBhpQ356rjXScdIaaTN/vUtP4bWJLtfYBa2J9BQZaYQg+S0 fUk8VDDUtgxIV6hJpn3XiH1awKF3fnl/VzVBhTsVAnyOikrVVj5XIAMxSt9+2Lkj2o/e /YZE0xq4WXcKaKDw6ZWXeE8zayF75xJtD7IFgBskEhu0HuS/69h0P8Q1GngxW9soTD/x LvmJdROYRdVMZrgLJlR0fM5qVpI8vect80VgnGKwYJ6BNsoUZDH4Q7SPR5RWOGcG0Eul qVLQ== X-Gm-Message-State: AMCzsaW9GChg1TVfGTYxybVEzAncxvxdZgptoUXxP09Ka54hL6Si/i7m +i9JcybIAAjIJKMH8EiiWoEp3J4pOkk= X-Google-Smtp-Source: ABhQp+TihnFdwoWIhcoztGXsiUJyKnDxTvipQlbMGCPk+JaHZ4uZcmvAH+TXgIodMhO1B6BttPgIVw== X-Received: by 10.107.143.20 with SMTP id r20mr3137746iod.205.1508439631436; Thu, 19 Oct 2017 12:00:31 -0700 (PDT) Received: from monkey.nyc.corp.google.com ([100.101.213.10]) by smtp.gmail.com with ESMTPSA id d187sm7225955iog.75.2017.10.19.12.00.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 19 Oct 2017 12:00:30 -0700 (PDT) From: Craig Gallek To: "David S . Miller" Cc: netdev@vger.kernel.org Subject: [PATCH net] soreuseport: fix initialization race Date: Thu, 19 Oct 2017 15:00:29 -0400 Message-Id: <20171019190029.163693-1-kraigatgoog@gmail.com> X-Mailer: git-send-email 2.15.0.rc1.287.g2b38de12cc-goog Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Craig Gallek Syzkaller stumbled upon a way to trigger WARNING: CPU: 1 PID: 13881 at net/core/sock_reuseport.c:41 reuseport_alloc+0x306/0x3b0 net/core/sock_reuseport.c:39 There are two initialization paths for the sock_reuseport structure in a socket: Through the udp/tcp bind paths of SO_REUSEPORT sockets or through SO_ATTACH_REUSEPORT_[CE]BPF before bind. The existing implementation assumedthat the socket lock protected both of these paths when it actually only protects the SO_ATTACH_REUSEPORT path. Syzkaller triggered this double allocation by running these paths concurrently. This patch moves the check for double allocation into the reuseport_alloc function which is protected by a global spin lock. Fixes: e32ea7e74727 ("soreuseport: fast reuseport UDP socket selection") Fixes: c125e80b8868 ("soreuseport: fast reuseport TCP socket selection") Signed-off-by: Craig Gallek --- net/core/sock_reuseport.c | 12 +++++++++--- net/ipv4/inet_hashtables.c | 5 +---- net/ipv4/udp.c | 5 +---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c index eed1ebf7f29d..b1e0dbea1e8c 100644 --- a/net/core/sock_reuseport.c +++ b/net/core/sock_reuseport.c @@ -36,9 +36,14 @@ int reuseport_alloc(struct sock *sk) * soft irq of receive path or setsockopt from process context */ spin_lock_bh(&reuseport_lock); - WARN_ONCE(rcu_dereference_protected(sk->sk_reuseport_cb, - lockdep_is_held(&reuseport_lock)), - "multiple allocations for the same socket"); + + /* Allocation attempts can occur concurrently via the setsockopt path + * and the bind/hash path. Nothing to do when we lose the race. + */ + if (rcu_dereference_protected(sk->sk_reuseport_cb, + lockdep_is_held(&reuseport_lock))) + goto out; + reuse = __reuseport_alloc(INIT_SOCKS); if (!reuse) { spin_unlock_bh(&reuseport_lock); @@ -49,6 +54,7 @@ int reuseport_alloc(struct sock *sk) reuse->num_socks = 1; rcu_assign_pointer(sk->sk_reuseport_cb, reuse); +out: spin_unlock_bh(&reuseport_lock); return 0; diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c index 597bb4cfe805..e7d15fb0d94d 100644 --- a/net/ipv4/inet_hashtables.c +++ b/net/ipv4/inet_hashtables.c @@ -456,10 +456,7 @@ static int inet_reuseport_add_sock(struct sock *sk, return reuseport_add_sock(sk, sk2); } - /* Initial allocation may have already happened via setsockopt */ - if (!rcu_access_pointer(sk->sk_reuseport_cb)) - return reuseport_alloc(sk); - return 0; + return reuseport_alloc(sk); } int __inet_hash(struct sock *sk, struct sock *osk) diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index e45177ceb0ee..3cd8103bab2c 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -231,10 +231,7 @@ static int udp_reuseport_add_sock(struct sock *sk, struct udp_hslot *hslot) } } - /* Initial allocation may have already happened via setsockopt */ - if (!rcu_access_pointer(sk->sk_reuseport_cb)) - return reuseport_alloc(sk); - return 0; + return reuseport_alloc(sk); } /**