Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Marijn Suijten <marijns95@gmail.com>
To:     linux-bluetooth@vger.kernel.org
Cc:     luiz.von.dentz@intel.com, Marijn Suijten <marijns95@gmail.com>
Subject: [BlueZ PATCH] audio: avrcp: Ignore peer RT supported_events when being the RT.
Date:   Sat, 18 Jan 2020 20:48:41 +0100
Message-Id: <20200118194841.439036-1-marijns95@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

From: Marijn Suijten <marijns95@gmail.com>

Remove the check of a received event against supported_events in
avrcp_handle_register_notification, which is only called when BlueZ is
the RT even though supported_events is only valid when BlueZ is the TG.

supported_events is assigned in target_init with events that the
corresponding RT on the other side of the Bluetooth connection supports,
to ensure the local TG will never report anything unexpected in
avrcp_handle_get_capabilities. This value is specific to what the target
should support to be compatible with the peer RT, but a locally running
RT has nothing to do with the external device being the RT.

This addresses the case where Absolute Volume notification registrations
are rejected when audio is played from an Android device to a computer
running BlueZ. The Android BT stack report an RT of version 1.3 [1]
which does not include Absolute Volume support. The RT on the Android
device is not involved in such a playback session, instead the computer
is the RT and the Android device the TG.

This has been tested by disabling registration of the RT service in
Android, to make the device a "pure" media player that cannot receive
audio: target_init does not get called and supported_events stays 0
which would have caused any notification registration to be rejected in
the above case.

[1]: https://android.googlesource.com/platform/system/bt/+/android-10.0.0_r25/bta/av/bta_av_main.cc#712
---
Hi,

I have a separate patch lying around that - instead of removing
supported_events - splits it up in two variables; one for the target and
another for the controller. Let me know if this extra safety is desired.

According to the AVRCP 1.3 specification GetCapabilities is mandatory,
which I have included in that patch. However the documentation also
mentions that this function is only supposed to be called by the CT
meaning that the call in target_init (introduced in 27efc30f0) is not
valid. What is your view on this?
Unfortunately even the small pair of in-ears I have lying around report
AVRCP TG functionality while they are not nearly capable of being a
target/media-source, so I have not been able to confirm how a pure RT
device would respond in such case.

- Marijn Suijten

 profiles/audio/avrcp.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 7fb8af608..820494d2a 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1529,10 +1529,6 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 	if (len != 5)
 		goto err;
 
-	/* Check if event is supported otherwise reject */
-	if (!(session->supported_events & (1 << pdu->params[0])))
-		goto err;
-
 	switch (pdu->params[0]) {
 	case AVRCP_EVENT_STATUS_CHANGED:
 		len = 2;
-- 
2.25.0