diff --git a/doc/connectivity/bluetooth/api/connection_mgmt.rst b/doc/connectivity/bluetooth/api/connection_mgmt.rst index 3a3d8d02a82..1b7ea2788c3 100644 --- a/doc/connectivity/bluetooth/api/connection_mgmt.rst +++ b/doc/connectivity/bluetooth/api/connection_mgmt.rst @@ -15,6 +15,14 @@ longer period of time, since this ensures that the object remains valid :c:func:`bt_conn_unref` API is to be used when releasing a reference to a connection. +A common mistake is to forget unreleasing a reference to a connection +object created by functions :c:func:`bt_conn_le_create` and +:c:func:`bt_conn_le_create_synced`. To protect against this, use the +:kconfig:option:`CONFIG_BT_CONN_CHECK_NULL_BEFORE_CREATE` Kconfig option, +which forces these functions return an error if the connection pointer +passed to them is not NULL. This helps to spot such issues and avoid +sporadic bugs caused by not releasing the connection object. + An application may track connections by registering a :c:struct:`bt_conn_cb` struct using the :c:func:`bt_conn_cb_register` or :c:macro:`BT_CONN_CB_DEFINE` APIs. This struct lets the application diff --git a/doc/releases/release-notes-4.0.rst b/doc/releases/release-notes-4.0.rst index 48f57ad7137..d27e7657cb2 100644 --- a/doc/releases/release-notes-4.0.rst +++ b/doc/releases/release-notes-4.0.rst @@ -123,6 +123,13 @@ Bluetooth * The host now disconnects from the peer upon ATT timeout. + * Added a warning to :c:func:`bt_conn_le_create` and :c:func:`bt_conn_le_create_synced` if + the connection pointer passed as an argument is not NULL. + + * Added Kconfig option :kconfig:option:`CONFIG_BT_CONN_CHECK_NULL_BEFORE_CREATE` to enforce + :c:func:`bt_conn_le_create` and :c:func:`bt_conn_le_create_synced` return an error if the + connection pointer passed as an argument is not NULL. + * HCI Drivers Boards & SoC Support diff --git a/include/zephyr/bluetooth/conn.h b/include/zephyr/bluetooth/conn.h index 4d2da8d7a58..b4cf1c5c6ba 100644 --- a/include/zephyr/bluetooth/conn.h +++ b/include/zephyr/bluetooth/conn.h @@ -1336,7 +1336,9 @@ struct bt_conn_le_create_param { * Allows initiate new LE link to remote peer using its address. * * The caller gets a new reference to the connection object which must be - * released with bt_conn_unref() once done using the object. + * released with bt_conn_unref() once done using the object. If + * @kconfig{CONFIG_BT_CONN_CHECK_NULL_BEFORE_CREATE} is enabled, this function + * will return -EINVAL if dereferenced @p conn is not NULL. * * This uses the General Connection Establishment procedure. * @@ -1375,7 +1377,9 @@ struct bt_conn_le_create_synced_param { * with Responses (PAwR) train. * * The caller gets a new reference to the connection object which must be - * released with bt_conn_unref() once done using the object. + * released with bt_conn_unref() once done using the object. If + * @kconfig{CONFIG_BT_CONN_CHECK_NULL_BEFORE_CREATE} is enabled, this function + * will return -EINVAL if dereferenced @p conn is not NULL. * * This uses the Periodic Advertising Connection Procedure. * diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index 7f39fa5d9bb..db247d76b93 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/subsys/bluetooth/host/Kconfig @@ -343,6 +343,17 @@ config BT_CONN_PARAM_ANY min and max connection intervals in order to verify whether the desired parameters have been established in the connection. +config BT_CONN_CHECK_NULL_BEFORE_CREATE + bool "Check if *conn is NULL when creating a connection" + help + Enable this option to ensure that bt_conn_le_create and + bt_conn_le_create_synced return an error if *conn is not initialized + to NULL. This option is recommended to use to catch programming + errors where the application reuses the connection pointer of an + active connection object without dereferencing it. Without + dereferencing, the connection object stays alive which can lead to an + unpredictable behavior. + config BT_USER_PHY_UPDATE bool "User control of PHY Update Procedure" depends on BT_PHY_UPDATE diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index f4c4fa7f6fa..f77710b3e08 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -3720,6 +3720,23 @@ int bt_conn_le_create(const bt_addr_le_t *peer, const struct bt_conn_le_create_p struct bt_conn *conn; int err; + CHECKIF(ret_conn == NULL) { + return -EINVAL; + } + + CHECKIF(*ret_conn != NULL) { + /* This rule helps application developers prevent leaks of connection references. If + * a bt_conn variable is not null, it presumably holds a reference and must not be + * overwritten. To avoid this warning, initialize the variables to null, and set + * them to null when moving the reference. + */ + LOG_WRN("*conn should be unreferenced and initialized to NULL"); + + if (IS_ENABLED(CONFIG_BT_CONN_CHECK_NULL_BEFORE_CREATE)) { + return -EINVAL; + } + } + err = conn_le_create_common_checks(peer, conn_param); if (err) { return err; @@ -3779,6 +3796,23 @@ int bt_conn_le_create_synced(const struct bt_le_ext_adv *adv, struct bt_conn *conn; int err; + CHECKIF(ret_conn == NULL) { + return -EINVAL; + } + + CHECKIF(*ret_conn != NULL) { + /* This rule helps application developers prevent leaks of connection references. If + * a bt_conn variable is not null, it presumably holds a reference and must not be + * overwritten. To avoid this warning, initialize the variables to null, and set + * them to null when moving the reference. + */ + LOG_WRN("*conn should be unreferenced and initialized to NULL"); + + if (IS_ENABLED(CONFIG_BT_CONN_CHECK_NULL_BEFORE_CREATE)) { + return -EINVAL; + } + } + err = conn_le_create_common_checks(synced_param->peer, conn_param); if (err) { return err;