Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



#syz test

On Fri, Oct 4, 2024 at 12:06 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> #syz test
>
> On Thu, Oct 3, 2024 at 3:21 PM Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
> >
> > #syz test
> >
> > On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz
> > <luiz.dentz@xxxxxxxxx> wrote:
> > >
> > > #syz test
> > >
> > > On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
> > > <luiz.dentz@xxxxxxxxx> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@xxxxxxxxx> wrote:
> > > > >
> > > > > #syz test
> > > > >
> > > > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@xxxxxxxxx> wrote:
> > > > > >
> > > > > > #syz test
> > > > > >
> > > > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@xxxxxxxxx> wrote:
> > > > > > >
> > > > > > > #syz test
> > > > > > >
> > > > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > > > > <luiz.dentz@xxxxxxxxx> wrote:
> > > > > > > >
> > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> > > > > > > >
> > > > > > > > This makes use of disable_delayed_work_sync instead
> > > > > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > > > > disables new submit which is disarable since the object holding the work
> > > > > > > > is about to be freed.
> > > > > > > >
> > > > > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > > > > there is nothing to be done in sco_sock_timeout.
> > > > > > > >
> > > > > > > > Reported-by: syzbot+4c0d0c4cde787116d465@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> > > > > > > > ---
> > > > > > > >  net/bluetooth/sco.c | 13 +------------
> > > > > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > > > > --- a/net/bluetooth/sco.c
> > > > > > > > +++ b/net/bluetooth/sco.c
> > > > > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > > > > >         }
> > > > > > > >
> > > > > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > > > > >
> > > > > > > >         hcon->sco_data = NULL;
> > > > > > > >         kfree(conn);
> > > > > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > > > > >
> > > > > > > >         case BT_CONNECTED:
> > > > > > > >         case BT_CONFIG:
> > > > > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > > > > -               } else
> > > > > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > > > > -               break;
> > > > > > > > -
> > > > > > > >         case BT_CONNECT2:
> > > > > > > >         case BT_CONNECT:
> > > > > > > >         case BT_DISCONN:
> > > > > > > > --
> > > > > > > > 2.46.1
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Luiz Augusto von Dentz
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz
From 0534aba3c6e0f8bebb003516e433dcbddd3e2014 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
Date: Tue, 1 Oct 2024 16:15:51 -0400
Subject: [PATCH v4 2/2] Bluetooth: ISO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancels the ongoing work but also
disables new submissions which is disarable since the object holding the
work is about to be freed.

In addition to it remove call to iso_sock_set_timer on iso_sock_disconn
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in iso_sock_timeout.

Fixes: ccf74f2390d6 ("Bluetooth: Add BTPROTO_ISO socket type")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
---
 net/bluetooth/iso.c | 58 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index d5e00d0dd1a0..030d402cc9bd 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -163,6 +163,24 @@ static struct iso_conn *iso_conn_add(struct hci_conn *hcon)
 	return conn;
 }
 
+static void iso_conn_drop(struct iso_conn *conn)
+{
+	if (!conn || !conn->hcon)
+		return;
+
+	BT_DBG("conn %p hcon %p", conn, conn->hcon);
+
+	hci_conn_drop(conn->hcon);
+
+	iso_conn_lock(conn);
+	conn->hcon->iso_data = NULL;
+	conn->hcon = NULL;
+	iso_conn_unlock(conn);
+
+	/* Ensure no more work items will run since hci_conn has been dropped */
+	disable_delayed_work_sync(&conn->timeout_work);
+}
+
 /* Delete channel. Must be called on the locked socket. */
 static void iso_chan_del(struct sock *sk, int err)
 {
@@ -179,8 +197,7 @@ static void iso_chan_del(struct sock *sk, int err)
 		iso_pi(sk)->conn = NULL;
 		iso_conn_unlock(conn);
 
-		if (conn->hcon)
-			hci_conn_drop(conn->hcon);
+		iso_conn_drop(conn);
 	}
 
 	sk->sk_state = BT_CLOSED;
@@ -197,6 +214,21 @@ static void iso_chan_del(struct sock *sk, int err)
 	sock_set_flag(sk, SOCK_ZAPPED);
 }
 
+static void iso_conn_destruct(struct iso_conn *conn)
+{
+	if (!conn)
+		return;
+
+	BT_DBG("conn %p", conn);
+
+	if (conn->sk)
+		iso_pi(conn->sk)->conn = NULL;
+
+	iso_conn_drop(conn);
+
+	kfree(conn);
+}
+
 static void iso_conn_del(struct hci_conn *hcon, int err)
 {
 	struct iso_conn *conn = hcon->iso_data;
@@ -214,19 +246,16 @@ static void iso_conn_del(struct hci_conn *hcon, int err)
 		sock_hold(sk);
 	iso_conn_unlock(conn);
 
-	if (sk) {
-		lock_sock(sk);
-		iso_sock_clear_timer(sk);
-		iso_chan_del(sk, err);
-		release_sock(sk);
-		sock_put(sk);
+	if (!sk) {
+		iso_conn_destruct(conn);
+		return;
 	}
 
-	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
-
-	hcon->iso_data = NULL;
-	kfree(conn);
+	lock_sock(sk);
+	iso_sock_clear_timer(sk);
+	iso_chan_del(sk, err);
+	release_sock(sk);
+	sock_put(sk);
 }
 
 static int __iso_chan_add(struct iso_conn *conn, struct sock *sk,
@@ -646,6 +675,8 @@ static void iso_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
+	iso_conn_destruct(iso_pi(sk)->conn);
+
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 }
@@ -714,7 +745,6 @@ static void iso_sock_disconn(struct sock *sk)
 	}
 
 	sk->sk_state = BT_DISCONN;
-	iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
 	iso_conn_lock(iso_pi(sk)->conn);
 	hci_conn_drop(iso_pi(sk)->conn->hcon);
 	iso_pi(sk)->conn->hcon = NULL;
-- 
2.46.1


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux