| Message ID | 20241002190452.3405592-1-luiz.dentz@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] Bluetooth: SCO: Use disable_delayed_work_sync | expand |
#syz test On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > 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@syzkaller.appspotmail.com > 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@intel.com> > --- > 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 >
#syz test On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > #syz test > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > 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@syzkaller.appspotmail.com > > 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@intel.com> > > --- > > 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
#syz test On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > #syz test > > On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > #syz test > > > > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz > > <luiz.dentz@gmail.com> wrote: > > > > > > #syz test > > > > > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz > > > <luiz.dentz@gmail.com> wrote: > > > > > > > > #syz test > > > > > > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz > > > > <luiz.dentz@gmail.com> wrote: > > > > > > > > > > #syz test > > > > > > > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz > > > > > <luiz.dentz@gmail.com> wrote: > > > > > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > > > > > > > > > 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@syzkaller.appspotmail.com > > > > > > 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@intel.com> > > > > > > --- > > > > > > 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
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: