All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Javier S. Pedro" <dev.git@javispedro.com>
To: linux-bluetooth@vger.kernel.org
Cc: "Javier de San Pedro" <dev.git@javispedro.com>,
	"Luiz Augusto von Dentz" <luiz.von.dentz@intel.com>
Subject: [PATCH] gatt-client: when disconnected return default MTU for GattCharacteristic1.MTU
Date: Sat, 27 Nov 2021 23:21:36 +0100	[thread overview]
Message-ID: <20211127222132.14351-1-dev.git@javispedro.com> (raw)

From: "Javier de San Pedro" <dev.git@javispedro.com>

After the MTU dbus property patches in 5.62 we are seeing bluetoothd
terminate frequently with "Disconnected from D-Bus. Exiting." msgs.
Apparently this is because bluetoothd sent an invalid reply to a D-Bus
Property Get (for GattCharacteristic1's MTU).
Multiple issues in bluez Github.com project reported similar behavior;
at least #235 (see Fixes:), #219, and likely #238.

When the Characteristic1 object is still cached/alive, but the
underlying att connection is not (e.g. someone just called Disconnect),
the property getter (characteristic_get_mtu) right now returns false.
However, gdbus seems to ignore the return value and sends the empty reply
message anyway (rather than a dbus error?), and this seems to cause
the dbus connection to be terminated (due to the ill-formed reply?).
bluetoothd then aborts.

This patch makes the property value BT_ATT_DEFAULT_LE_MTU if the
underlying att object does not exist, rather than returning an invalid
message. This is consistent with the existing PropertyChanged signal
behavior (we will emit a PropertyChange only if a larger MTU is
exchanged), and fixes the issue on my machines.
An alternative could be to change gdbus behavior, but I'm not sure if we
are allowed to return an error here anyway without causing problems in
other dbus libraries/wrappers.

Signed-off-by: Javier de San Pedro <dev.git@javispedro.com>
Fixes: aaa0c4996ae9 ("gatt: Add implementation of GattCharacteristic1.MTU")
Fixes: https://github.com/bluez/bluez/issues/235
---
 src/gatt-client.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index de18bea9708e..6ee984db9410 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -883,10 +883,7 @@ static gboolean characteristic_get_mtu(const GDBusPropertyTable *property,
 	uint16_t mtu;
 
 	att = bt_gatt_client_get_att(gatt);
-	if (!att)
-		return FALSE;
-
-	mtu = bt_att_get_mtu(att);
+	mtu = att ? bt_att_get_mtu(att) : BT_ATT_DEFAULT_LE_MTU;
 
 	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &mtu);
 
-- 
2.34.0


             reply	other threads:[~2021-11-27 22:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-27 22:21 Javier S. Pedro [this message]
2021-11-27 23:50 ` gatt-client: when disconnected return default MTU for GattCharacteristic1.MTU bluez.test.bot
2021-11-29 21:17   ` Luiz Augusto von Dentz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211127222132.14351-1-dev.git@javispedro.com \
    --to=dev.git@javispedro.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=luiz.von.dentz@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.