All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pauli Virtanen <pav@iki.fi>
To: linux-bluetooth@vger.kernel.org
Cc: Pauli Virtanen <pav@iki.fi>
Subject: [PATCH RFC 0/5] hci_conn and ISO concurrency fixes
Date: Fri, 23 Jun 2023 20:18:37 +0300	[thread overview]
Message-ID: <cover.1687525956.git.pav@iki.fi> (raw)

This series addresses additional KASAN errors seen in the same test case
as in the previous one.  These are mostly due to hci_conn being deleted
at a bad time, or manipulated without necessary locks.

With this series, the test setup doesn't seem to be producing KASAN
crashes any more.

The test setup is still

while true; do bluetoothctl power on; sleep 12; bluetoothctl power off; sleep 1.5; bluetoothctl power off; sleep 2.5; done;
while true; do sudo systemctl restart bluetooth; sleep 110; done
while true; do systemctl --user restart pipewire wireplumber pipewire-pulse; sleep 91; done
while true; do paplay sample.flac & sleep 2; kill %1; sleep 0.7; done

and equivalent operations manually, on VM + connect to TWS earbuds,
and let it run until it hits a crash.

There's an RFC question here: it would seem useful to be able to keep
references to hci_conn around without RCU or other locks, and be able
to safely continue later if the conn is still around. I.e.

    hci_conn_get(conn);
    hci_dev_unlock(hdev);
    ...
    hci_dev_lock(hdev);
    if (!hci_conn_is_alive(hdev, conn)) {
	    hci_conn_put(conn);
	    goto bail_out;
    }
    hci_conn_put(conn);
    ...

The first commit here adds this function. It should also be RCU-correct
too, but I'll need to think that through a second time.

It seems in several parts in hci_sync.c it is assumed the conn is not
deleted while the code is blocking waiting for controller events.  At
first sight it's not so clear that it's really guaranteed there can't be
UAF here, so I'm wondering if there would be a need to start polluting
hci_sync.c with locks and aliveness checks after waits.

Or if it's guaranteed by something not apparent and nothing needs to be
done, or if some other thing should be better (such as serializing
operations that delete hci_conn through hci_sync).

Pauli Virtanen (5):
  Bluetooth: hci_conn: add hci_conn_is_alive
  Bluetooth: hci_sync: iterate conn_hash safely in
    hci_disconnect_all_sync
  Bluetooth: hci_conn: hold ref while hci_connect_le_sync is pending
  Bluetooth: ISO: fix use-after-free in __iso_sock_close
  Bluetooth: ISO: fix locking in iso_conn_ready

 include/net/bluetooth/hci_core.h |  18 ++++
 net/bluetooth/hci_conn.c         |  24 ++++--
 net/bluetooth/hci_sync.c         | 140 ++++++++++++++++++++++++++++---
 net/bluetooth/iso.c              |  69 +++++++++++----
 4 files changed, 214 insertions(+), 37 deletions(-)

-- 
2.41.0


             reply	other threads:[~2023-06-23 17:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-23 17:18 Pauli Virtanen [this message]
2023-06-23 17:18 ` [PATCH RFC 1/5] Bluetooth: hci_conn: add hci_conn_is_alive Pauli Virtanen
2023-06-23 18:00   ` hci_conn and ISO concurrency fixes bluez.test.bot
2023-06-23 19:39   ` [PATCH RFC 1/5] Bluetooth: hci_conn: add hci_conn_is_alive Luiz Augusto von Dentz
2023-06-23 22:21     ` Pauli Virtanen
2023-06-27 23:05       ` Luiz Augusto von Dentz
2023-06-28 16:24         ` Pauli Virtanen
2023-06-28 19:29           ` Luiz Augusto von Dentz
2023-06-28 20:08             ` Pauli Virtanen
2023-06-28 21:17               ` Luiz Augusto von Dentz
2023-06-23 17:18 ` [PATCH RFC 2/5] Bluetooth: hci_sync: iterate conn_hash safely in hci_disconnect_all_sync Pauli Virtanen
2023-06-23 17:18 ` [PATCH RFC 3/5] Bluetooth: hci_conn: hold ref while hci_connect_le_sync is pending Pauli Virtanen
2023-06-23 17:18 ` [PATCH RFC 4/5] Bluetooth: ISO: fix use-after-free in __iso_sock_close Pauli Virtanen
2023-06-23 17:18 ` [PATCH RFC 5/5] Bluetooth: ISO: fix locking in iso_conn_ready Pauli Virtanen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1687525956.git.pav@iki.fi \
    --to=pav@iki.fi \
    --cc=linux-bluetooth@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.