[GIT PULL] bluetooth 2025-09-20

[GIT PULL] bluetooth 2025-09-20

[Luiz Augusto von Dentz]

The following changes since commit b65678cacc030efd53c38c089fb9b741a2ee34c8:

  ethernet: rvu-af: Remove slash from the driver name (2025-09-19 17:00:53 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git tags/for-net-2025-09-20

for you to fetch changes up to b683725c679a2a30852fa40fa1196d5f7bb4998c:

  Bluetooth: MGMT: Fix possible UAFs (2025-09-20 11:01:42 -0400)

----------------------------------------------------------------
bluetooth pull request for net:

 - Fix build after header cleanup
 - hci_sync: Fix hci_resume_advertising_sync
 - hci_event: Fix UAF in hci_conn_tx_dequeue
 - hci_event: Fix UAF in hci_acl_create_conn_sync
 - MGMT: Fix possible UAFs

----------------------------------------------------------------
Calvin Owens (1):
      Bluetooth: Fix build after header cleanup

Luiz Augusto von Dentz (4):
      Bluetooth: hci_sync: Fix hci_resume_advertising_sync
      Bluetooth: hci_event: Fix UAF in hci_conn_tx_dequeue
      Bluetooth: hci_event: Fix UAF in hci_acl_create_conn_sync
      Bluetooth: MGMT: Fix possible UAFs

 drivers/bluetooth/Kconfig        |   6 +
 drivers/bluetooth/hci_uart.h     |   8 +-
 include/net/bluetooth/hci_core.h |  21 ++++
 net/bluetooth/hci_event.c        |  30 ++++-
 net/bluetooth/hci_sync.c         |   7 ++
 net/bluetooth/mgmt.c             | 244 +++++++++++++++++++++++++++------------
 net/bluetooth/mgmt_util.c        |  24 ++++
 net/bluetooth/mgmt_util.h        |   2 +
 8 files changed, 259 insertions(+), 83 deletions(-)

Re: [GIT PULL] bluetooth 2025-09-20

[Jakub Kicinski]

On Sat, 20 Sep 2025 11:04:53 -0400 Luiz Augusto von Dentz wrote:
>       Bluetooth: MGMT: Fix possible UAFs

Are you amenable to rewriting this one? The conditional locking really
doesn't look great. It's just a few more lines for the caller to take 
the lock, below completely untested but to illustrate..

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1e7886ccee40..23cb19b9915d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1358,8 +1358,10 @@ static int set_powered_sync(struct hci_dev *hdev, void *data)
 	struct mgmt_pending_cmd *cmd = data;
 	struct mgmt_mode cp;
 
+	mutex_lock(&hdev->mgmt_pending_lock);
+
 	/* Make sure cmd still outstanding. */
-	if (!mgmt_pending_valid(hdev, cmd, false))
+	if (!__mgmt_pending_listed(hdev, cmd))
 		return -ECANCELED;
 
 	memcpy(&cp, cmd->param, sizeof(cp));
diff --git a/net/bluetooth/mgmt_util.c b/net/bluetooth/mgmt_util.c
index 258c22d38809..11b1d1667d08 100644
--- a/net/bluetooth/mgmt_util.c
+++ b/net/bluetooth/mgmt_util.c
@@ -320,28 +320,38 @@ void mgmt_pending_remove(struct mgmt_pending_cmd *cmd)
 	mgmt_pending_free(cmd);
 }
 
-bool mgmt_pending_valid(struct hci_dev *hdev, struct mgmt_pending_cmd *cmd,
-			bool remove_unlock)
+bool __mgmt_pending_listed(struct hci_dev *hdev, struct mgmt_pending_cmd *cmd)
 {
 	struct mgmt_pending_cmd *tmp;
 
+	lockdep_assert_held(&hdev->mgmt_pending_lock);
 	if (!cmd)
 		return false;
 
-	mutex_lock(&hdev->mgmt_pending_lock);
-
 	list_for_each_entry(tmp, &hdev->mgmt_pending, list) {
-		if (cmd == tmp) {
-			if (remove_unlock) {
-				list_del(&cmd->list);
-				mutex_unlock(&hdev->mgmt_pending_lock);
-			}
+		if (cmd == tmp)
 			return true;
-		}
 	}
+	return false;
+}
+
+bool mgmt_pending_valid(struct hci_dev *hdev, struct mgmt_pending_cmd *cmd)
+{
+	struct mgmt_pending_cmd *tmp;
+	bool listed;
+
+	if (!cmd)
+		return false;
+
+	mutex_lock(&hdev->mgmt_pending_lock);
+
+	listed = __mgmt_pending_listed(hdev, cmd);
+	if (listed)
+		list_del(&cmd->list);
 
 	mutex_unlock(&hdev->mgmt_pending_lock);
-	return false;
+
+	return listed;
 }
 
 void mgmt_mesh_foreach(struct hci_dev *hdev,

Re: [GIT PULL] bluetooth 2025-09-20

[Luiz Augusto von Dentz]

Hi Jakub,

On Sat, Sep 20, 2025 at 4:38 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 20 Sep 2025 11:04:53 -0400 Luiz Augusto von Dentz wrote:
> >       Bluetooth: MGMT: Fix possible UAFs
>
> Are you amenable to rewriting this one? The conditional locking really
> doesn't look great. It's just a few more lines for the caller to take
> the lock, below completely untested but to illustrate..

I guess the idea is to have it open coded to avoid mistakes like
unbalanced locking, etc, right?

> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 1e7886ccee40..23cb19b9915d 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1358,8 +1358,10 @@ static int set_powered_sync(struct hci_dev *hdev, void *data)
>         struct mgmt_pending_cmd *cmd = data;
>         struct mgmt_mode cp;
>
> +       mutex_lock(&hdev->mgmt_pending_lock);
> +
>         /* Make sure cmd still outstanding. */
> -       if (!mgmt_pending_valid(hdev, cmd, false))
> +       if (!__mgmt_pending_listed(hdev, cmd))
>                 return -ECANCELED;

Sure, this does require calling unlocking also when it fails though,
but I guess that is to be expected with this kind of construct and we
could have a variant that does the locking inline in case the cmd
fields don't need to be accessed.

>
>         memcpy(&cp, cmd->param, sizeof(cp));
> diff --git a/net/bluetooth/mgmt_util.c b/net/bluetooth/mgmt_util.c
> index 258c22d38809..11b1d1667d08 100644
> --- a/net/bluetooth/mgmt_util.c
> +++ b/net/bluetooth/mgmt_util.c
> @@ -320,28 +320,38 @@ void mgmt_pending_remove(struct mgmt_pending_cmd *cmd)
>         mgmt_pending_free(cmd);
>  }
>
> -bool mgmt_pending_valid(struct hci_dev *hdev, struct mgmt_pending_cmd *cmd,
> -                       bool remove_unlock)
> +bool __mgmt_pending_listed(struct hci_dev *hdev, struct mgmt_pending_cmd *cmd)
>  {
>         struct mgmt_pending_cmd *tmp;
>
> +       lockdep_assert_held(&hdev->mgmt_pending_lock);
>         if (!cmd)
>                 return false;
>
> -       mutex_lock(&hdev->mgmt_pending_lock);
> -
>         list_for_each_entry(tmp, &hdev->mgmt_pending, list) {
> -               if (cmd == tmp) {
> -                       if (remove_unlock) {
> -                               list_del(&cmd->list);
> -                               mutex_unlock(&hdev->mgmt_pending_lock);
> -                       }
> +               if (cmd == tmp)
>                         return true;
> -               }
>         }
> +       return false;
> +}
> +
> +bool mgmt_pending_valid(struct hci_dev *hdev, struct mgmt_pending_cmd *cmd)
> +{
> +       struct mgmt_pending_cmd *tmp;
> +       bool listed;
> +
> +       if (!cmd)
> +               return false;
> +
> +       mutex_lock(&hdev->mgmt_pending_lock);
> +
> +       listed = __mgmt_pending_listed(hdev, cmd);
> +       if (listed)
> +               list_del(&cmd->list);
>
>         mutex_unlock(&hdev->mgmt_pending_lock);
> -       return false;
> +
> +       return listed;
>  }
>
>  void mgmt_mesh_foreach(struct hci_dev *hdev,



-- 
Luiz Augusto von Dentz

Re: [GIT PULL] bluetooth 2025-09-20

[Jakub Kicinski]

On Mon, 22 Sep 2025 09:59:44 -0400 Luiz Augusto von Dentz wrote:
> > On Sat, 20 Sep 2025 11:04:53 -0400 Luiz Augusto von Dentz wrote:  
> > >       Bluetooth: MGMT: Fix possible UAFs  
> >
> > Are you amenable to rewriting this one? The conditional locking really
> > doesn't look great. It's just a few more lines for the caller to take
> > the lock, below completely untested but to illustrate..  
> 
> I guess the idea is to have it open coded to avoid mistakes like
> unbalanced locking, etc, right?

Yup! Makes it easier to read the code basically.

Thanks for the v2!