From patchwork Tue Jul 10 11:18:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Pisati X-Patchwork-Id: 941936 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41Q08F08kFz9s1B; Tue, 10 Jul 2018 21:19:09 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1fcqfc-0003FT-C1; Tue, 10 Jul 2018 11:19:00 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.86_2) (envelope-from ) id 1fcqfb-0003F7-6m for kernel-team@lists.ubuntu.com; Tue, 10 Jul 2018 11:18:59 +0000 Received: from 1.general.ppisati.uk.vpn ([10.172.193.134] helo=canonical.com) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1fcqfa-0007HI-UL for kernel-team@lists.ubuntu.com; Tue, 10 Jul 2018 11:18:58 +0000 From: Paolo Pisati To: kernel-team@lists.ubuntu.com Subject: [PATCH 1/2] packet: hold bind lock when rebinding to fanout hook Date: Tue, 10 Jul 2018 13:18:57 +0200 Message-Id: <1531221538-10134-2-git-send-email-paolo.pisati@canonical.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1531221538-10134-1-git-send-email-paolo.pisati@canonical.com> References: <1531221538-10134-1-git-send-email-paolo.pisati@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Willem de Bruijn Packet socket bind operations must hold the po->bind_lock. This keeps po->running consistent with whether the socket is actually on a ptype list to receive packets. fanout_add unbinds a socket and its packet_rcv/tpacket_rcv call, then binds the fanout object to receive through packet_rcv_fanout. Make it hold the po->bind_lock when testing po->running and rebinding. Else, it can race with other rebind operations, such as that in packet_set_ring from packet_rcv to tpacket_rcv. Concurrent updates can result in a socket being added to a fanout group twice, causing use-after-free KASAN bug reports, among others. Reported independently by both trinity and syzkaller. Verified that the syzkaller reproducer passes after this patch. Fixes: dc99f600698d ("packet: Add fanout support.") Reported-by: nixioaming Signed-off-by: Willem de Bruijn Signed-off-by: David S. Miller (cherry picked from commit 008ba2a13f2d04c947adc536d19debb8fe66f110) Signed-off-by: Paolo Pisati --- net/packet/af_packet.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 59a1558..2a07eb2 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1317,10 +1317,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags) mutex_lock(&fanout_mutex); - err = -EINVAL; - if (!po->running) - goto out; - err = -EALREADY; if (po->fanout) goto out; @@ -1358,7 +1354,10 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags) list_add(&match->list, &fanout_list); } err = -EINVAL; - if (match->type == type && + + spin_lock(&po->bind_lock); + if (po->running && + match->type == type && match->prot_hook.type == po->prot_hook.type && match->prot_hook.dev == po->prot_hook.dev) { err = -ENOSPC; @@ -1370,6 +1369,13 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags) err = 0; } } + spin_unlock(&po->bind_lock); + + if (err && !atomic_read(&match->sk_ref)) { + list_del(&match->list); + kfree(match); + } + out: mutex_unlock(&fanout_mutex); return err; From patchwork Tue Jul 10 11:18:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Pisati X-Patchwork-Id: 941938 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41Q08F0Pqyz9s1b; Tue, 10 Jul 2018 21:19:09 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1fcqfc-0003Fc-Fe; Tue, 10 Jul 2018 11:19:00 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.86_2) (envelope-from ) id 1fcqfb-0003FE-J3 for kernel-team@lists.ubuntu.com; Tue, 10 Jul 2018 11:18:59 +0000 Received: from 1.general.ppisati.uk.vpn ([10.172.193.134] helo=canonical.com) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1fcqfb-0007HR-AW for kernel-team@lists.ubuntu.com; Tue, 10 Jul 2018 11:18:59 +0000 From: Paolo Pisati To: kernel-team@lists.ubuntu.com Subject: [PATCH 2/2] packet: in packet_do_bind, test fanout with bind_lock held Date: Tue, 10 Jul 2018 13:18:58 +0200 Message-Id: <1531221538-10134-3-git-send-email-paolo.pisati@canonical.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1531221538-10134-1-git-send-email-paolo.pisati@canonical.com> References: <1531221538-10134-1-git-send-email-paolo.pisati@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Willem de Bruijn Once a socket has po->fanout set, it remains a member of the group until it is destroyed. The prot_hook must be constant and identical across sockets in the group. If fanout_add races with packet_do_bind between the test of po->fanout and taking the lock, the bind call may make type or dev inconsistent with that of the fanout group. Hold po->bind_lock when testing po->fanout to avoid this race. I had to introduce artificial delay (local_bh_enable) to actually observe the race. Fixes: dc99f600698d ("packet: Add fanout support.") Signed-off-by: Willem de Bruijn Reviewed-by: Eric Dumazet Signed-off-by: David S. Miller (cherry picked from commit 4971613c1639d8e5f102c4e797c3bf8f83a5a69e) Signed-off-by: Paolo Pisati --- net/packet/af_packet.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 2a07eb2..c0230c7 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2514,16 +2514,18 @@ static int packet_do_bind(struct sock *sk, struct net_device *dev, __be16 protoc { struct packet_sock *po = pkt_sk(sk); + lock_sock(sk); + + spin_lock(&po->bind_lock); + if (po->fanout) { + spin_unlock(&po->bind_lock); + release_sock(sk); if (dev) dev_put(dev); - return -EINVAL; } - lock_sock(sk); - - spin_lock(&po->bind_lock); unregister_prot_hook(sk, true); po->num = protocol;