[PATCH BlueZ] device: Reply br-connection-key-missing on connection bonding error

[PATCH BlueZ] device: Reply br-connection-key-missing on connection bonding error

[Frédéric Danis]

Currently when connection is removed while the client is waiting for
Connect() the failed message is br-connection-canceled, even if this
is due to bonding error.

This commit reply br-connection-key-missing when connection request
fails due to bonding error, allowing the client to differentiate
connection failure reasons.
---
 src/device.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/device.c b/src/device.c
index c364d72c3..816282eab 100644
--- a/src/device.c
+++ b/src/device.c
@@ -246,6 +246,7 @@ struct btd_device {
 	struct browse_req *browse;		/* service discover request */
 	struct bonding_req *bonding;
 	struct authentication_req *authr;	/* authentication request */
+	uint8_t		bonding_status;
 	GSList		*disconnects;		/* disconnects message */
 	DBusMessage	*connect;		/* connect message */
 	DBusMessage	*disconnect;		/* disconnect message */
@@ -3636,6 +3637,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
 	DBusMessage *reply;
 	bool remove_device = false;
 	bool paired_status_updated = false;
+	uint8_t bonding_status = device->bonding_status;
 
 	if (!state->connected)
 		return;
@@ -3643,6 +3645,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
 	state->connected = false;
 	state->initiator = false;
 	device->general_connect = FALSE;
+	device->bonding_status = 0;
 
 	device_set_svc_refreshed(device, false);
 
@@ -3658,6 +3661,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
 	if (device->connect) {
 		DBG("connection removed while Connect() is waiting reply");
 		reply = btd_error_failed(device->connect,
+				bonding_status ? ERR_BREDR_CONN_KEY_MISSING :
 						ERR_BREDR_CONN_CANCELED);
 		g_dbus_send_message(dbus_conn, reply);
 		dbus_message_unref(device->connect);
@@ -6763,6 +6767,8 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
 
 	DBG("bonding %p status 0x%02x", bonding, status);
 
+	device->bonding_status = status;
+
 	if (auth && auth->agent)
 		agent_cancel(auth->agent);
 
-- 
2.43.0

Re: [PATCH BlueZ] device: Reply br-connection-key-missing on connection bonding error

[Paul Menzel]

Dear Frédéric,


Am 14.05.25 um 13:20 schrieb Frédéric Danis:
> Currently when connection is removed while the client is waiting for
> Connect() the failed message is br-connection-canceled, even if this
> is due to bonding error.
> 
> This commit reply br-connection-key-missing when connection request
> fails due to bonding error, allowing the client to differentiate
> connection failure reasons.

This sentence is hard to parse for me. Maybe:

Implement to reply with br-connection-key-missing, when the connection 
request fails due to a bonding error, ….

Also, it’d be great if you added the steps, how to reproduce and test this.

> ---
>   src/device.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/src/device.c b/src/device.c
> index c364d72c3..816282eab 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -246,6 +246,7 @@ struct btd_device {
>   	struct browse_req *browse;		/* service discover request */
>   	struct bonding_req *bonding;
>   	struct authentication_req *authr;	/* authentication request */
> +	uint8_t		bonding_status;
>   	GSList		*disconnects;		/* disconnects message */

disconnects → disconnect

>   	DBusMessage	*connect;		/* connect message */
>   	DBusMessage	*disconnect;		/* disconnect message */
> @@ -3636,6 +3637,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
>   	DBusMessage *reply;
>   	bool remove_device = false;
>   	bool paired_status_updated = false;
> +	uint8_t bonding_status = device->bonding_status;
>   
>   	if (!state->connected)
>   		return;
> @@ -3643,6 +3645,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
>   	state->connected = false;
>   	state->initiator = false;
>   	device->general_connect = FALSE;
> +	device->bonding_status = 0;
>   
>   	device_set_svc_refreshed(device, false);
>   
> @@ -3658,6 +3661,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
>   	if (device->connect) {
>   		DBG("connection removed while Connect() is waiting reply");
>   		reply = btd_error_failed(device->connect,
> +				bonding_status ? ERR_BREDR_CONN_KEY_MISSING :
>   						ERR_BREDR_CONN_CANCELED);
>   		g_dbus_send_message(dbus_conn, reply);
>   		dbus_message_unref(device->connect);
> @@ -6763,6 +6767,8 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
>   
>   	DBG("bonding %p status 0x%02x", bonding, status);
>   
> +	device->bonding_status = status;
> +
>   	if (auth && auth->agent)
>   		agent_cancel(auth->agent);
>   


Kind regards,

Paul

RE: [BlueZ] device: Reply br-connection-key-missing on connection bonding error

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=962794

---Test result---

Test Summary:
CheckPatch                    PENDING   0.32 seconds
GitLint                       PENDING   0.35 seconds
BuildEll                      PASS      20.37 seconds
BluezMake                     PASS      2631.16 seconds
MakeCheck                     PASS      20.22 seconds
MakeDistcheck                 PASS      198.74 seconds
CheckValgrind                 PASS      275.97 seconds
CheckSmatch                   PASS      302.51 seconds
bluezmakeextell               PASS      128.26 seconds
IncrementalBuild              PENDING   0.40 seconds
ScanBuild                     PASS      907.99 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ] device: Reply br-connection-key-missing on connection bonding error

[Luiz Augusto von Dentz]

Hi Frederic,

On Wed, May 14, 2025 at 7:21 AM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> Currently when connection is removed while the client is waiting for
> Connect() the failed message is br-connection-canceled, even if this
> is due to bonding error.
>
> This commit reply br-connection-key-missing when connection request
> fails due to bonding error, allowing the client to differentiate
> connection failure reasons.
> ---
>  src/device.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index c364d72c3..816282eab 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -246,6 +246,7 @@ struct btd_device {
>         struct browse_req *browse;              /* service discover request */
>         struct bonding_req *bonding;
>         struct authentication_req *authr;       /* authentication request */
> +       uint8_t         bonding_status;
>         GSList          *disconnects;           /* disconnects message */
>         DBusMessage     *connect;               /* connect message */
>         DBusMessage     *disconnect;            /* disconnect message */
> @@ -3636,6 +3637,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
>         DBusMessage *reply;
>         bool remove_device = false;
>         bool paired_status_updated = false;
> +       uint8_t bonding_status = device->bonding_status;
>
>         if (!state->connected)
>                 return;
> @@ -3643,6 +3645,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
>         state->connected = false;
>         state->initiator = false;
>         device->general_connect = FALSE;
> +       device->bonding_status = 0;
>
>         device_set_svc_refreshed(device, false);
>
> @@ -3658,6 +3661,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
>         if (device->connect) {
>                 DBG("connection removed while Connect() is waiting reply");
>                 reply = btd_error_failed(device->connect,
> +                               bonding_status ? ERR_BREDR_CONN_KEY_MISSING :
>                                                 ERR_BREDR_CONN_CANCELED);

Hmm, we shouldn't really be assuming that if status != 0 then it must
be a key missing error.

>                 g_dbus_send_message(dbus_conn, reply);
>                 dbus_message_unref(device->connect);
> @@ -6763,6 +6767,8 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
>
>         DBG("bonding %p status 0x%02x", bonding, status);
>
> +       device->bonding_status = status;

I also wonder if we should reply directly here in case the bonding
fails, but perhaps that depends on the error.

>         if (auth && auth->agent)
>                 agent_cancel(auth->agent);
>
> --
> 2.43.0
>
>


-- 
Luiz Augusto von Dentz