From patchwork Thu Jan 3 00:11:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthias Kaehlcke X-Patchwork-Id: 1020119 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@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=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=chromium.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43VSyT3lbXz9s55 for ; Thu, 3 Jan 2019 11:11:45 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728228AbfACALk (ORCPT ); Wed, 2 Jan 2019 19:11:40 -0500 Received: from mail-pg1-f195.google.com ([209.85.215.195]:44442 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727729AbfACALj (ORCPT ); Wed, 2 Jan 2019 19:11:39 -0500 Received: by mail-pg1-f195.google.com with SMTP id t13so15216896pgr.11 for ; Wed, 02 Jan 2019 16:11:39 -0800 (PST) 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=pgzDVuFkNdcUaGFgKDWKsKa3d95bqeFYv0F5c9kcBGA=; b=JoAOWiXymVD0BUo5JlCsQpFfU7ZktGGsK1f+aJFNreQw9g/FmSDKT2YrG/UsMVRLE8 Xu7q+w2EhzZ6GmqvVwfV9xhsxtlAwx0BhC1u83asKVetHwJhuHRlXpqlgEMkT23mNDrC 3W+w4ndk5KkFJEYhPbgaOr1II9UnwWOMXzNBcp8oAq29wsYMT0PwAw5cSswUpM10rA4U NdKg3xLycsBMbpAkaQHyfvVFgQaEq7SfaMm8lgsaiSNnPPpYVdSWeuz+9dpz/f3EVAo5 hY662drQ8qyIB88NZBeA7i3mGMgG91zb6pPVEAR83VRRObmDtKrF7URxQYpz3fjWxVGh AUbA== X-Gm-Message-State: AA+aEWYmstjkj90Q1R7Ph2F8zRr5ayw3vcx9TmA8Q9DDix8tJl+8idOX OfJvZrokiGaIfsQFkq4Yi+Gt4Q== X-Google-Smtp-Source: AFSGD/UVuDMkbqvT8A7TR6lY4KaRAXEBZlgsLd+JAPQ4YB6fYUn5ElwAjInVSAwp9b+ZmtD1h2ZH1w== X-Received: by 2002:a62:7c47:: with SMTP id x68mr46847050pfc.209.1546474298363; Wed, 02 Jan 2019 16:11:38 -0800 (PST) Received: from mka.mtv.corp.google.com ([2620:15c:202:1:75a:3f6e:21d:9374]) by smtp.gmail.com with ESMTPSA id t12sm83822010pfi.45.2019.01.02.16.11.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Jan 2019 16:11:37 -0800 (PST) From: Matthias Kaehlcke To: Marcel Holtmann , Johan Hedberg , "David S . Miller" , Dean Jenkins Cc: linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Konstantin Khlebnikov , Balakrishna Godavarthi , Douglas Anderson , Dmitry Grinberg , Matthias Kaehlcke Subject: [PATCH v2] Bluetooth: Fix locking in bt_accept_enqueue() for BH context Date: Wed, 2 Jan 2019 16:11:20 -0800 Message-Id: <20190103001120.177784-1-mka@chromium.org> X-Mailer: git-send-email 2.20.1.415.g653613c723-goog MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org With commit e16337622016 ("Bluetooth: Handle bt_accept_enqueue() socket atomically") lock_sock[_nested]() is used to acquire the socket lock before manipulating the socket. lock_sock[_nested]() may block, which is problematic since bt_accept_enqueue() can be called in bottom half context (e.g. from rfcomm_connect_ind()): [] __might_sleep+0x4c/0x80 [] lock_sock_nested+0x24/0x58 [] bt_accept_enqueue+0x48/0xd4 [bluetooth] [] rfcomm_connect_ind+0x190/0x218 [rfcomm] Add a parameter to bt_accept_enqueue() to indicate whether the function is called from BH context, and acquire the socket lock with bh_lock_sock_nested() if that's the case. Also adapt all callers of bt_accept_enqueue() to pass the new parameter: - l2cap_sock_new_connection_cb() - uses lock_sock() to lock the parent socket => process context - rfcomm_connect_ind() - acquires the parent socket lock with bh_lock_sock() => BH context - __sco_chan_add() - called from sco_chan_add(), which is called from sco_connect(). parent is NULL, hence bt_accept_enqueue() isn't called in this code path and we can ignore it - also called from sco_conn_ready(). uses bh_lock_sock() to acquire the parent lock => BH context Fixes: e16337622016 ("Bluetooth: Handle bt_accept_enqueue() socket atomically") Signed-off-by: Matthias Kaehlcke Reviewed-by: Douglas Anderson --- Changes in v2: - use parameter in bt_accept_enqueue() to decide which lock to acquire and adapt all callers - updated commit message --- include/net/bluetooth/bluetooth.h | 2 +- net/bluetooth/af_bluetooth.c | 16 +++++++++++++--- net/bluetooth/l2cap_sock.c | 2 +- net/bluetooth/rfcomm/sock.c | 2 +- net/bluetooth/sco.c | 2 +- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index ec9d6bc658559..fabee6db0abb7 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -276,7 +276,7 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg); int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo); int bt_sock_wait_ready(struct sock *sk, unsigned long flags); -void bt_accept_enqueue(struct sock *parent, struct sock *sk); +void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh); void bt_accept_unlink(struct sock *sk); struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock); diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c index deacc52d7ff18..8d12198eaa949 100644 --- a/net/bluetooth/af_bluetooth.c +++ b/net/bluetooth/af_bluetooth.c @@ -154,15 +154,25 @@ void bt_sock_unlink(struct bt_sock_list *l, struct sock *sk) } EXPORT_SYMBOL(bt_sock_unlink); -void bt_accept_enqueue(struct sock *parent, struct sock *sk) +void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh) { BT_DBG("parent %p, sk %p", parent, sk); sock_hold(sk); - lock_sock_nested(sk, SINGLE_DEPTH_NESTING); + + if (bh) + bh_lock_sock_nested(sk); + else + lock_sock_nested(sk, SINGLE_DEPTH_NESTING); + list_add_tail(&bt_sk(sk)->accept_q, &bt_sk(parent)->accept_q); bt_sk(sk)->parent = parent; - release_sock(sk); + + if (bh) + bh_unlock_sock(sk); + else + release_sock(sk); + parent->sk_ack_backlog++; } EXPORT_SYMBOL(bt_accept_enqueue); diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 686bdc6b35b03..a3a2cd55e23a9 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1252,7 +1252,7 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) l2cap_sock_init(sk, parent); - bt_accept_enqueue(parent, sk); + bt_accept_enqueue(parent, sk, false); release_sock(parent); diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c index aa0db1d1bd9b4..b1f49fcc04780 100644 --- a/net/bluetooth/rfcomm/sock.c +++ b/net/bluetooth/rfcomm/sock.c @@ -988,7 +988,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc * rfcomm_pi(sk)->channel = channel; sk->sk_state = BT_CONFIG; - bt_accept_enqueue(parent, sk); + bt_accept_enqueue(parent, sk, true); /* Accept connection and return socket DLC */ *d = rfcomm_pi(sk)->dlc; diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index 529b38996d8bc..9a580999ca57e 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -193,7 +193,7 @@ static void __sco_chan_add(struct sco_conn *conn, struct sock *sk, conn->sk = sk; if (parent) - bt_accept_enqueue(parent, sk); + bt_accept_enqueue(parent, sk, true); } static int sco_chan_add(struct sco_conn *conn, struct sock *sk,