Skip to content

Commit e530b15

Browse files
committed
bluetooth: buf: Fix callback protection for ISR context
Fix assertion failure when buf_rx_freed_notify() is called from ISR by replacing k_sched_lock with atomic_ptr operations for callback pointer access. The scheduler lock is retained during callback execution in thread context to maintain backward compatibility, but is skipped in ISR context where it's not available. Updated documentation to clarify the behavior difference between thread and ISR contexts. Signed-off-by: Pavel Vasilyev <[email protected]>
1 parent d02cdc7 commit e530b15

File tree

3 files changed

+34
-17
lines changed

3 files changed

+34
-17
lines changed

include/zephyr/bluetooth/buf.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,15 +196,22 @@ struct net_buf *bt_buf_get_rx(enum bt_buf_type type, k_timeout_t timeout);
196196
* @ref bt_buf_get_rx function. However, this callback is called from the context of the buffer
197197
* freeing operation and must not attempt to allocate a new buffer from the same pool.
198198
*
199-
* @warning When this callback is called, the scheduler is locked and the callee must not perform
200-
* any action that makes the current thread unready. This callback must only be used for very
201-
* short non-blocking operation (e.g. submitting a work item).
199+
* This callback must only be used for very short non-blocking operations (e.g. submitting a work
200+
* item). When called from thread context, the scheduler is locked during execution and the
201+
* callee must not perform any action that makes the current thread unready. When called from ISR
202+
* context, the callback runs without scheduler lock.
203+
*
204+
* @funcprops \isr_ok
202205
*
203206
* @param type_mask A bit mask of buffer types that have been freed.
204207
*/
205208
typedef void (*bt_buf_rx_freed_cb_t)(enum bt_buf_type type_mask);
206209

207210
/** Set the callback to notify about freed buffer in the incoming data pool.
211+
*
212+
* It's safe to call this inside the callback itself.
213+
*
214+
* @funcprops \isr_ok
208215
*
209216
* @param cb Callback to notify about freed buffer in the incoming data pool. If NULL, the callback
210217
* is disabled.

subsys/bluetooth/host/buf.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <zephyr/logging/log.h>
1919
#include <zephyr/net_buf.h>
2020
#include <zephyr/sys/__assert.h>
21+
#include <zephyr/sys/atomic.h>
2122
#include <zephyr/sys/util_macro.h>
2223
#include <zephyr/sys_clock.h>
2324

@@ -36,17 +37,26 @@ LOG_MODULE_REGISTER(bt_buf, CONFIG_BT_LOG_LEVEL);
3637
*/
3738
#define SYNC_EVT_SIZE (BT_BUF_RESERVE + BT_HCI_EVT_HDR_SIZE + 255)
3839

39-
static bt_buf_rx_freed_cb_t buf_rx_freed_cb;
40+
static atomic_ptr_t buf_rx_freed_cb;
4041

4142
static void buf_rx_freed_notify(enum bt_buf_type mask)
4243
{
43-
k_sched_lock();
44+
bt_buf_rx_freed_cb_t cb;
45+
bool in_isr = k_is_in_isr();
4446

45-
if (buf_rx_freed_cb) {
46-
buf_rx_freed_cb(mask);
47+
if (!in_isr) {
48+
k_sched_lock();
4749
}
4850

49-
k_sched_unlock();
51+
cb = (bt_buf_rx_freed_cb_t)atomic_ptr_get(&buf_rx_freed_cb);
52+
53+
if (cb != NULL) {
54+
cb(mask);
55+
}
56+
57+
if (!in_isr) {
58+
k_sched_unlock();
59+
}
5060
}
5161

5262
#if defined(CONFIG_BT_ISO_RX)
@@ -134,15 +144,11 @@ struct net_buf *bt_buf_get_rx(enum bt_buf_type type, k_timeout_t timeout)
134144

135145
void bt_buf_rx_freed_cb_set(bt_buf_rx_freed_cb_t cb)
136146
{
137-
k_sched_lock();
138-
139-
buf_rx_freed_cb = cb;
147+
atomic_ptr_set(&buf_rx_freed_cb, (void *)cb);
140148

141149
#if defined(CONFIG_BT_ISO_RX)
142150
bt_iso_buf_rx_freed_cb_set(cb != NULL ? iso_rx_freed_cb : NULL);
143151
#endif
144-
145-
k_sched_unlock();
146152
}
147153

148154
struct net_buf *bt_buf_get_evt(uint8_t evt, bool discardable,

subsys/bluetooth/host/iso.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,18 @@ LOG_MODULE_REGISTER(bt_iso, CONFIG_BT_ISO_LOG_LEVEL);
5252
#define iso_chan(_iso) ((_iso)->iso.chan);
5353

5454
#if defined(CONFIG_BT_ISO_RX)
55-
static bt_iso_buf_rx_freed_cb_t buf_rx_freed_cb;
55+
static atomic_ptr_t buf_rx_freed_cb;
5656

5757
static void iso_rx_buf_destroy(struct net_buf *buf)
5858
{
59+
bt_iso_buf_rx_freed_cb_t cb;
60+
61+
cb = (bt_iso_buf_rx_freed_cb_t)atomic_ptr_get(&buf_rx_freed_cb);
62+
5963
net_buf_destroy(buf);
6064

61-
if (buf_rx_freed_cb) {
62-
buf_rx_freed_cb();
65+
if (cb != NULL) {
66+
cb();
6367
}
6468
}
6569

@@ -643,7 +647,7 @@ struct net_buf *bt_iso_get_rx(k_timeout_t timeout)
643647

644648
void bt_iso_buf_rx_freed_cb_set(bt_iso_buf_rx_freed_cb_t cb)
645649
{
646-
buf_rx_freed_cb = cb;
650+
atomic_ptr_set(&buf_rx_freed_cb, (void *)cb);
647651
}
648652

649653
void bt_iso_recv(struct bt_conn *iso, struct net_buf *buf, uint8_t flags)

0 commit comments

Comments
 (0)