Bluetooth: L2CAP: Fix use-after-free in l2cap_chan_timeout

l2cap_chan_timeout() reads chan->conn without holding any lock and without taking a reference on the connection. If l2cap_conn_del() frees the connection concurrently, mutex_lock(&conn->lock) operates on freed memory. Additionally, the existing NULL check early return leaks the channel reference held by the timer.

Fix the root cause by disabling channel timers when the channel is being destroyed. Use disable_delayed_work() in l2cap_chan_del() to prevent timers from being rearmed, and disable_delayed_work_sync() in l2cap_conn_del() to ensure running timer work has completed before the connection is freed.

The sync cannot be done directly inside l2cap_chan_del() because it would deadlock in three independent ways: (1) l2cap_chan_timeout() can call l2cap_chan_close() -> l2cap_chan_del(), so syncing chan_timer would wait for itself; (2) callers hold conn->lock while l2cap_chan_timeout() acquires it; (3) callers hold chan->lock while the ERTM timer functions acquire it. Therefore, the sync is performed in l2cap_conn_del() after releasing conn->lock.

Also fix the reference leak on the !conn early return path in l2cap_chan_timeout().

Fixes 3df91ea20e74 ("Bluetooth: Revert to mutexes from RCU list")

Signed-off-by Hyunwoo Kim <[email protected]>

---
Changes in v2:
- Moved the fix from l2cap_chan_timeout() to l2cap_chan_del()/l2cap_conn_del() to address the root cause instead of adding locking in the callback
- Used disable_delayed_work()/disable_delayed_work_sync() to prevent timer rearming and ensure completion, as suggested by Luiz
- Applied the fix to all four channel timers (chan_timer, retrans_timer, monitor_timer, ack_timer)
- Removed l2cap_conn_get()/l2cap_conn_put() and the lock reordering that caused a circular locking dependency in v1
- v1: https://lore.kernel.org/all/abwSxg_C6n8Avid3@v4bel/

---
 net/bluetooth/l2cap_core.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

net/bluetooth/l2cap_core.c +3 -1

index 5deb6c4f1e41..c08478e4512b 100644
@@ -411,8 +411,10 @@ static void l2cap_chan_timeout(struct work_struct *work)
 
        BT_DBG("chan %p state %s", chan, state_to_string(chan->state));
 
-       if (!conn)
+       if (!conn) {
+               l2cap_chan_put(chan);
                return;
+       }
 
        mutex_lock(&conn->lock);
        /* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling

net/bluetooth/l2cap_core.c +1 -0

                skb_queue_purge(&chan->srej_q);
 

@@ -1742,6 +1753,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 {
        struct l2cap_conn *conn = hcon->l2cap_data;
        struct l2cap_chan *chan, *l;
+       LIST_HEAD(del_list);
 
        if (!conn)
                return;

net/bluetooth/l2cap_core.c +4 -0

+               list_add_tail(&chan->list, &del_list);
        }
 
        if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)

@@ -1791,6 +1803,20 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 
        hcon->l2cap_data = NULL;
        mutex_unlock(&conn->lock);
+
+       /* Disable and sync-wait for any running channel timer work without
+        * holding conn->lock to avoid AB-BA deadlock with

net/bluetooth/l2cap_core.c +0 -1

        l2cap_conn_put(conn);
 }
 
--

2.43.0

Hi Hyunwoo,

Quoted 54 lines · Hyunwoo Kim

l2cap_chan_timeout() reads chan->conn without holding any lock and without taking a reference on the connection. If l2cap_conn_del() frees the connection concurrently, mutex_lock(&conn->lock) operates on freed memory. Additionally, the existing NULL check early return leaks the channel reference held by the timer.

Fix the root cause by disabling channel timers when the channel is being destroyed. Use disable_delayed_work() in l2cap_chan_del() to prevent timers from being rearmed, and disable_delayed_work_sync() in l2cap_conn_del() to ensure running timer work has completed before the connection is freed.

The sync cannot be done directly inside l2cap_chan_del() because it would deadlock in three independent ways: (1) l2cap_chan_timeout() can call l2cap_chan_close() -> l2cap_chan_del(), so syncing chan_timer would wait for itself; (2) callers hold conn->lock while l2cap_chan_timeout() acquires it; (3) callers hold chan->lock while the ERTM timer functions acquire it. Therefore, the sync is performed in l2cap_conn_del() after releasing conn->lock.

Also fix the reference leak on the !conn early return path in l2cap_chan_timeout().

Fixes 3df91ea20e74 ("Bluetooth: Revert to mutexes from RCU list")

Signed-off-by Hyunwoo Kim <[email protected]>

---
Changes in v2:
- Moved the fix from l2cap_chan_timeout() to l2cap_chan_del()/l2cap_conn_del() to address the root cause instead of adding locking in the callback
- Used disable_delayed_work()/disable_delayed_work_sync() to prevent timer rearming and ensure completion, as suggested by Luiz
- Applied the fix to all four channel timers (chan_timer, retrans_timer, monitor_timer, ack_timer)
- Removed l2cap_conn_get()/l2cap_conn_put() and the lock reordering that caused a circular locking dependency in v1
- v1: https://lore.kernel.org/all/abwSxg_C6n8Avid3@v4bel/

---
 net/bluetooth/l2cap_core.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

net/bluetooth/l2cap_core.c +3 -1

index 5deb6c4f1e41..c08478e4512b 100644
@@ -411,8 +411,10 @@ static void l2cap_chan_timeout(struct work_struct *work)
 
        BT_DBG("chan %p state %s", chan, state_to_string(chan->state));
 
-       if (!conn)
+       if (!conn) {
+               l2cap_chan_put(chan);
                return;
+       }
 
        mutex_lock(&conn->lock);
        /* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling

net/bluetooth/l2cap_core.c void l2cap_chan_del(struct l2cap_chan *chan, int err) · line 658

Quoted 8 lines

net/bluetooth/l2cap_core.c +5 -0

@@ -649,6 +651,11 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
        struct l2cap_conn *conn = chan->conn;
 
        __clear_chan_timer(chan);
+       /* Prevent the timer from being rearmed since the channel is about
+        * to be destroyed. Running timer work will be synced by
+        * l2cap_conn_del() after releasing conn->lock.
+        */
+       disable_delayed_work(&chan->chan_timer);

Hmm, so we can't use the _sync version because it could be pending on conn->lock while this could be called from l2cap_conn_del? I wonder if we should clarify the comment or if we actually need to mark the l2cap_chan as deleted/disconnected or something, perhaps via state, also we may want to return something like -EINPROGRESS if a timer could not be disabled which would indicate to the likes of l2cap_conn_del that it need a cleanup.

net/bluetooth/l2cap_core.c void l2cap_chan_del(struct l2cap_chan *chan, int err) · line 701

Quoted 10 lines

net/bluetooth/l2cap_core.c +4 -0

        BT_DBG("chan %p, conn %p, err %d, state %s", chan, conn, err,
               state_to_string(chan->state));

@@ -688,6 +695,10 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
                __clear_retrans_timer(chan);
                __clear_monitor_timer(chan);
                __clear_ack_timer(chan);
+               /* Prevent the ERTM timers from being rearmed. */
+               disable_delayed_work(&chan->retrans_timer);
+               disable_delayed_work(&chan->monitor_timer);
+               disable_delayed_work(&chan->ack_timer);

l2cap_clear_timer drops the reference if the timer was cancelled, so I think we will need to check the return of disable as well, or create a function like l2cap_disable_timer, that said Id return something to indicate that l2cap_chan_del didn't cleanup everything.

Quoted 11 lines

net/bluetooth/l2cap_core.c +1 -0

                skb_queue_purge(&chan->srej_q);
 

@@ -1742,6 +1753,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 {
        struct l2cap_conn *conn = hcon->l2cap_data;
        struct l2cap_chan *chan, *l;
+       LIST_HEAD(del_list);
 
        if (!conn)
                return;

net/bluetooth/l2cap_core.c static void l2cap_conn_del(struct hci_conn *hcon, int err) · line 1783

Quoted 4 lines

net/bluetooth/l2cap_core.c +0 -1

@@ -1780,7 +1792,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
                chan->ops->close(chan);
 
                l2cap_chan_unlock(chan);
-               l2cap_chan_put(chan);

Let's populate the del_list only with channels that actually had a pending timer.

Quoted 11 lines

net/bluetooth/l2cap_core.c +4 -0

+               list_add_tail(&chan->list, &del_list);
        }
 
        if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)

@@ -1791,6 +1803,20 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 
        hcon->l2cap_data = NULL;
        mutex_unlock(&conn->lock);
+
+       /* Disable and sync-wait for any running channel timer work without
+        * holding conn->lock to avoid AB-BA deadlock with

net/bluetooth/l2cap_core.c static void l2cap_conn_del(struct hci_conn *hcon, int err) · line 1818

Quoted 10 lines

net/bluetooth/l2cap_core.c +10 -0

+        * l2cap_chan_timeout() which acquires conn->lock.
+        */
+       list_for_each_entry_safe(chan, l, &del_list, list) {
+               disable_delayed_work_sync(&chan->chan_timer);
+               disable_delayed_work_sync(&chan->retrans_timer);
+               disable_delayed_work_sync(&chan->monitor_timer);
+               disable_delayed_work_sync(&chan->ack_timer);
+               list_del(&chan->list);
+               l2cap_chan_put(chan);
+       }

It might be better to place the disable sync-wait on l2cap_chan_destroy then since l2cap_chan_del is not garanteed to do it, which means l2cap_chan_put is not safe to be called with conn->lock, but I guess it was never really safe since if we destroy the chan while a timer is pending/running it would cause an UAF

Quoted 6 lines

net/bluetooth/l2cap_core.c +0 -1

        l2cap_conn_put(conn);
 }
 
--

2.43.0

Signature · 1 line

Luiz Augusto von Dentz

Quoted 141 lines · Luiz Augusto von Dentz

Hi Hyunwoo,

Quoted 54 lines · Hyunwoo Kim

l2cap_chan_timeout() reads chan->conn without holding any lock and without taking a reference on the connection. If l2cap_conn_del() frees the connection concurrently, mutex_lock(&conn->lock) operates on freed memory. Additionally, the existing NULL check early return leaks the channel reference held by the timer.

Fix the root cause by disabling channel timers when the channel is being destroyed. Use disable_delayed_work() in l2cap_chan_del() to prevent timers from being rearmed, and disable_delayed_work_sync() in l2cap_conn_del() to ensure running timer work has completed before the connection is freed.

The sync cannot be done directly inside l2cap_chan_del() because it would deadlock in three independent ways: (1) l2cap_chan_timeout() can call l2cap_chan_close() -> l2cap_chan_del(), so syncing chan_timer would wait for itself; (2) callers hold conn->lock while l2cap_chan_timeout() acquires it; (3) callers hold chan->lock while the ERTM timer functions acquire it. Therefore, the sync is performed in l2cap_conn_del() after releasing conn->lock.

Also fix the reference leak on the !conn early return path in l2cap_chan_timeout().

Fixes 3df91ea20e74 ("Bluetooth: Revert to mutexes from RCU list")

Signed-off-by Hyunwoo Kim <[email protected]>

---
Changes in v2:
- Moved the fix from l2cap_chan_timeout() to l2cap_chan_del()/l2cap_conn_del() to address the root cause instead of adding locking in the callback
- Used disable_delayed_work()/disable_delayed_work_sync() to prevent timer rearming and ensure completion, as suggested by Luiz
- Applied the fix to all four channel timers (chan_timer, retrans_timer, monitor_timer, ack_timer)
- Removed l2cap_conn_get()/l2cap_conn_put() and the lock reordering that caused a circular locking dependency in v1
- v1: https://lore.kernel.org/all/abwSxg_C6n8Avid3@v4bel/

---
 net/bluetooth/l2cap_core.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

net/bluetooth/l2cap_core.c +3 -1

index 5deb6c4f1e41..c08478e4512b 100644
@@ -411,8 +411,10 @@ static void l2cap_chan_timeout(struct work_struct *work)
 
        BT_DBG("chan %p state %s", chan, state_to_string(chan->state));
 
-       if (!conn)
+       if (!conn) {
+               l2cap_chan_put(chan);
                return;
+       }
 
        mutex_lock(&conn->lock);
        /* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling

net/bluetooth/l2cap_core.c void l2cap_chan_del(struct l2cap_chan *chan, int err) · line 658

Quoted 8 lines

net/bluetooth/l2cap_core.c +5 -0

@@ -649,6 +651,11 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
        struct l2cap_conn *conn = chan->conn;
 
        __clear_chan_timer(chan);
+       /* Prevent the timer from being rearmed since the channel is about
+        * to be destroyed. Running timer work will be synced by
+        * l2cap_conn_del() after releasing conn->lock.
+        */
+       disable_delayed_work(&chan->chan_timer);

Hmm, so we can't use the _sync version because it could be pending on conn->lock while this could be called from l2cap_conn_del? I wonder if we should clarify the comment or if we actually need to mark the l2cap_chan as deleted/disconnected or something, perhaps via state, also we may want to return something like -EINPROGRESS if a timer could not be disabled which would indicate to the likes of l2cap_conn_del that it need a cleanup.

net/bluetooth/l2cap_core.c void l2cap_chan_del(struct l2cap_chan *chan, int err) · line 701

Quoted 10 lines

net/bluetooth/l2cap_core.c +4 -0

        BT_DBG("chan %p, conn %p, err %d, state %s", chan, conn, err,
               state_to_string(chan->state));

@@ -688,6 +695,10 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
                __clear_retrans_timer(chan);
                __clear_monitor_timer(chan);
                __clear_ack_timer(chan);
+               /* Prevent the ERTM timers from being rearmed. */
+               disable_delayed_work(&chan->retrans_timer);
+               disable_delayed_work(&chan->monitor_timer);
+               disable_delayed_work(&chan->ack_timer);

l2cap_clear_timer drops the reference if the timer was cancelled, so I think we will need to check the return of disable as well, or create a function like l2cap_disable_timer, that said Id return something to indicate that l2cap_chan_del didn't cleanup everything.

Quoted 11 lines

net/bluetooth/l2cap_core.c +1 -0

                skb_queue_purge(&chan->srej_q);
 

@@ -1742,6 +1753,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 {
        struct l2cap_conn *conn = hcon->l2cap_data;
        struct l2cap_chan *chan, *l;
+       LIST_HEAD(del_list);
 
        if (!conn)
                return;

net/bluetooth/l2cap_core.c static void l2cap_conn_del(struct hci_conn *hcon, int err) · line 1783

Quoted 4 lines

net/bluetooth/l2cap_core.c +0 -1

@@ -1780,7 +1792,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
                chan->ops->close(chan);
 
                l2cap_chan_unlock(chan);
-               l2cap_chan_put(chan);

Let's populate the del_list only with channels that actually had a pending timer.

Quoted 11 lines

net/bluetooth/l2cap_core.c +4 -0

+               list_add_tail(&chan->list, &del_list);
        }
 
        if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)

@@ -1791,6 +1803,20 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 
        hcon->l2cap_data = NULL;
        mutex_unlock(&conn->lock);
+
+       /* Disable and sync-wait for any running channel timer work without
+        * holding conn->lock to avoid AB-BA deadlock with

net/bluetooth/l2cap_core.c static void l2cap_conn_del(struct hci_conn *hcon, int err) · line 1818

Quoted 10 lines

net/bluetooth/l2cap_core.c +10 -0

+        * l2cap_chan_timeout() which acquires conn->lock.
+        */
+       list_for_each_entry_safe(chan, l, &del_list, list) {
+               disable_delayed_work_sync(&chan->chan_timer);
+               disable_delayed_work_sync(&chan->retrans_timer);
+               disable_delayed_work_sync(&chan->monitor_timer);
+               disable_delayed_work_sync(&chan->ack_timer);
+               list_del(&chan->list);
+               l2cap_chan_put(chan);
+       }

It might be better to place the disable sync-wait on l2cap_chan_destroy then since l2cap_chan_del is not garanteed to do it, which means l2cap_chan_put is not safe to be called with conn->lock, but I guess it was never really safe since if we destroy the chan while a timer is pending/running it would cause an UAF

Quoted 3 lines

net/bluetooth/l2cap_core.c +0 -0

        l2cap_conn_put(conn);
 }
 

Quoted 7 lines

Quoted 3 lines

--
2.43.0

--
Luiz Augusto von Dentz

This is regarding the missing Reported-by credit that was discussed on [email protected]. My credit has still not been added to the commit in the netdev tree:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/net/bluetooth?id=9d87cb22195b2c67405f5485d525190747ad5493

If this gets merged into mainline as-is, my credit will be permanently lost, correct?

It seems like the mainline pull request is not far away. I would appreciate a quick confirmation on this.

Best regards,
Hyunwoo Kim

Hi Hyunwoo,

Quoted 157 lines · Hyunwoo Kim

Quoted 141 lines · Luiz Augusto von Dentz

Hi Hyunwoo,

Quoted 54 lines · Hyunwoo Kim

l2cap_chan_timeout() reads chan->conn without holding any lock and without taking a reference on the connection. If l2cap_conn_del() frees the connection concurrently, mutex_lock(&conn->lock) operates on freed memory. Additionally, the existing NULL check early return leaks the channel reference held by the timer.

Fix the root cause by disabling channel timers when the channel is being destroyed. Use disable_delayed_work() in l2cap_chan_del() to prevent timers from being rearmed, and disable_delayed_work_sync() in l2cap_conn_del() to ensure running timer work has completed before the connection is freed.

The sync cannot be done directly inside l2cap_chan_del() because it would deadlock in three independent ways: (1) l2cap_chan_timeout() can call l2cap_chan_close() -> l2cap_chan_del(), so syncing chan_timer would wait for itself; (2) callers hold conn->lock while l2cap_chan_timeout() acquires it; (3) callers hold chan->lock while the ERTM timer functions acquire it. Therefore, the sync is performed in l2cap_conn_del() after releasing conn->lock.

Also fix the reference leak on the !conn early return path in l2cap_chan_timeout().

Fixes 3df91ea20e74 ("Bluetooth: Revert to mutexes from RCU list")

Signed-off-by Hyunwoo Kim <[email protected]>

---
Changes in v2:
- Moved the fix from l2cap_chan_timeout() to l2cap_chan_del()/l2cap_conn_del() to address the root cause instead of adding locking in the callback
- Used disable_delayed_work()/disable_delayed_work_sync() to prevent timer rearming and ensure completion, as suggested by Luiz
- Applied the fix to all four channel timers (chan_timer, retrans_timer, monitor_timer, ack_timer)
- Removed l2cap_conn_get()/l2cap_conn_put() and the lock reordering that caused a circular locking dependency in v1
- v1: https://lore.kernel.org/all/abwSxg_C6n8Avid3@v4bel/

---
 net/bluetooth/l2cap_core.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

net/bluetooth/l2cap_core.c +3 -1

index 5deb6c4f1e41..c08478e4512b 100644
@@ -411,8 +411,10 @@ static void l2cap_chan_timeout(struct work_struct *work)
 
        BT_DBG("chan %p state %s", chan, state_to_string(chan->state));
 
-       if (!conn)
+       if (!conn) {
+               l2cap_chan_put(chan);
                return;
+       }
 
        mutex_lock(&conn->lock);
        /* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling

net/bluetooth/l2cap_core.c void l2cap_chan_del(struct l2cap_chan *chan, int err) · line 658

Quoted 8 lines

net/bluetooth/l2cap_core.c +5 -0

@@ -649,6 +651,11 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
        struct l2cap_conn *conn = chan->conn;
 
        __clear_chan_timer(chan);
+       /* Prevent the timer from being rearmed since the channel is about
+        * to be destroyed. Running timer work will be synced by
+        * l2cap_conn_del() after releasing conn->lock.
+        */
+       disable_delayed_work(&chan->chan_timer);

Hmm, so we can't use the _sync version because it could be pending on conn->lock while this could be called from l2cap_conn_del? I wonder if we should clarify the comment or if we actually need to mark the l2cap_chan as deleted/disconnected or something, perhaps via state, also we may want to return something like -EINPROGRESS if a timer could not be disabled which would indicate to the likes of l2cap_conn_del that it need a cleanup.

net/bluetooth/l2cap_core.c void l2cap_chan_del(struct l2cap_chan *chan, int err) · line 701

Quoted 10 lines

net/bluetooth/l2cap_core.c +4 -0

        BT_DBG("chan %p, conn %p, err %d, state %s", chan, conn, err,
               state_to_string(chan->state));

@@ -688,6 +695,10 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
                __clear_retrans_timer(chan);
                __clear_monitor_timer(chan);
                __clear_ack_timer(chan);
+               /* Prevent the ERTM timers from being rearmed. */
+               disable_delayed_work(&chan->retrans_timer);
+               disable_delayed_work(&chan->monitor_timer);
+               disable_delayed_work(&chan->ack_timer);

l2cap_clear_timer drops the reference if the timer was cancelled, so I think we will need to check the return of disable as well, or create a function like l2cap_disable_timer, that said Id return something to indicate that l2cap_chan_del didn't cleanup everything.

Quoted 11 lines

net/bluetooth/l2cap_core.c +1 -0

                skb_queue_purge(&chan->srej_q);
 

@@ -1742,6 +1753,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 {
        struct l2cap_conn *conn = hcon->l2cap_data;
        struct l2cap_chan *chan, *l;
+       LIST_HEAD(del_list);
 
        if (!conn)
                return;

net/bluetooth/l2cap_core.c static void l2cap_conn_del(struct hci_conn *hcon, int err) · line 1783

Quoted 4 lines

net/bluetooth/l2cap_core.c +0 -1

@@ -1780,7 +1792,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
                chan->ops->close(chan);
 
                l2cap_chan_unlock(chan);
-               l2cap_chan_put(chan);

Let's populate the del_list only with channels that actually had a pending timer.

Quoted 11 lines

net/bluetooth/l2cap_core.c +4 -0

+               list_add_tail(&chan->list, &del_list);
        }
 
        if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)

@@ -1791,6 +1803,20 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 
        hcon->l2cap_data = NULL;
        mutex_unlock(&conn->lock);
+
+       /* Disable and sync-wait for any running channel timer work without
+        * holding conn->lock to avoid AB-BA deadlock with

net/bluetooth/l2cap_core.c static void l2cap_conn_del(struct hci_conn *hcon, int err) · line 1818

Quoted 10 lines

net/bluetooth/l2cap_core.c +10 -0

+        * l2cap_chan_timeout() which acquires conn->lock.
+        */
+       list_for_each_entry_safe(chan, l, &del_list, list) {
+               disable_delayed_work_sync(&chan->chan_timer);
+               disable_delayed_work_sync(&chan->retrans_timer);
+               disable_delayed_work_sync(&chan->monitor_timer);
+               disable_delayed_work_sync(&chan->ack_timer);
+               list_del(&chan->list);
+               l2cap_chan_put(chan);
+       }

It might be better to place the disable sync-wait on l2cap_chan_destroy then since l2cap_chan_del is not garanteed to do it, which means l2cap_chan_put is not safe to be called with conn->lock, but I guess it was never really safe since if we destroy the chan while a timer is pending/running it would cause an UAF

Quoted 3 lines

net/bluetooth/l2cap_core.c +0 -0

        l2cap_conn_put(conn);
 }
 

Quoted 7 lines

Quoted 3 lines

--
2.43.0

--
Luiz Augusto von Dentz

This is regarding the missing Reported-by credit that was discussed on [email protected]. My credit has still not been added to the commit in the netdev tree:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/net/bluetooth?id=9d87cb22195b2c67405f5485d525190747ad5493

Quoted 5 lines

If this gets merged into mainline as-is, my credit will be permanently lost, correct?

It seems like the mainline pull request is not far away. I would appreciate a quick confirmation on this.

Yeah, I rushed that due to a regression fix and unfortunately forgot about your Reported-by:.

Don't see any comments regarding the changes above, the faster we got to these in the less more likely are they to enter the next merge window.

Quoted 4 lines

Best regards,
Hyunwoo Kim

Signature · 1 line

Luiz Augusto von Dentz