Hi,

On Wed, Jan 2, 2019 at 4:11 PM Matthias Kaehlcke mka@chromium.org wrote:

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()):

[<ffffff80080d81ec>] __might_sleep+0x4c/0x80 [<ffffff800876c7b0>] lock_sock_nested+0x24/0x58 [<ffffff8000d7c27c>] bt_accept_enqueue+0x48/0xd4 [bluetooth] [<ffffff8000e67d8c>] 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 mka@chromium.org

---

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(-)

This looks nice to me, thanks!

Cc: stable@vger.kernel.org Reviewed-by: Douglas Anderson dianders@chromium.org