diff mbox series

[BlueZ,v4,2/3] audio/avrcp: Only allow absolute volume call/event on category-2 peers

Message ID 20241005214321.84250-3-marijn.suijten@somainline.org (mailing list archive)
State New, archived
Headers show
Series audio/avrcp: Determine Absolute Volume support from feature category 2 | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch warning WARNING:LONG_LINE_COMMENT: line length of 85 exceeds 80 columns #83: FILE: profiles/audio/avrcp.c:1779: + * The controller on the remote end is only allowed to call SetAbsoluteVolume WARNING:LONG_LINE: line length of 118 exceeds 80 columns #87: FILE: profiles/audio/avrcp.c:1783: + (btd_opts.avrcp.volume_category && !(session->target->features & AVRCP_FEATURE_CATEGORY_2))) { WARNING:LONG_LINE_STRING: line length of 84 exceeds 80 columns #88: FILE: profiles/audio/avrcp.c:1784: + error("Remote SetAbsoluteVolume rejected from non-category-2 peer"); WARNING:LONG_LINE_COMMENT: line length of 88 exceeds 80 columns #100: FILE: profiles/audio/avrcp.c:3781: + * The target on the remote end is only allowed to reply to EVENT_VOLUME_CHANGED WARNING:LONG_LINE_COMMENT: line length of 82 exceeds 80 columns #101: FILE: profiles/audio/avrcp.c:3782: + * on our controller if it's at least version 1.4 and a category-2 device. WARNING:LONG_LINE: line length of 122 exceeds 80 columns #104: FILE: profiles/audio/avrcp.c:3785: + (btd_opts.avrcp.volume_category && !(session->controller->features & AVRCP_FEATURE_CATEGORY_2))) { WARNING:LONG_LINE_STRING: line length of 87 exceeds 80 columns #105: FILE: profiles/audio/avrcp.c:3786: + error("Remote EVENT_VOLUME_CHANGED rejected from non-category-2 peer"); WARNING:LONG_LINE: line length of 91 exceeds 80 columns #125: FILE: profiles/audio/avrcp.c:4268: + if (!btd_opts.avrcp.volume_category || target->features & AVRCP_FEATURE_CATEGORY_2) WARNING:LONG_LINE: line length of 126 exceeds 80 columns #143: FILE: profiles/audio/avrcp.c:4692: + (btd_opts.avrcp.volume_category && !(session->target->features & AVRCP_FEATURE_CATEGORY_2))) { WARNING:LONG_LINE_STRING: line length of 88 exceeds 80 columns #144: FILE: profiles/audio/avrcp.c:4693: + error("Can't send EVENT_VOLUME_CHANGED to non-category-2 peer"); WARNING:LONG_LINE: line length of 84 exceeds 80 columns #155: FILE: profiles/audio/avrcp.c:4709: + if (!session->controller || session->controller->version < 0x0104 || WARNING:LONG_LINE: line length of 130 exceeds 80 columns #156: FILE: profiles/audio/avrcp.c:4710: + (btd_opts.avrcp.volume_category && !(session->controller->features & AVRCP_FEATURE_CATEGORY_2))) { WARNING:LONG_LINE_STRING: line length of 85 exceeds 80 columns #157: FILE: profiles/audio/avrcp.c:4711: + error("Can't send SetAbsoluteVolume to non-category-2 peer"); /github/workspace/src/src/13823593.patch total: 0 errors, 13 warnings, 120 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13823593.patch has style problems, please review. NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
tedd_an/GitLint fail WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (85>80): "[BlueZ,v4,2/3] audio/avrcp: Only allow absolute volume call/event on category-2 peers"
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Marijn Suijten Oct. 5, 2024, 9:43 p.m. UTC
Restrict the use of SetAbsoluteVolume and EVENT_VOLUME_CHANGED to peers
with at least AVRCP version 1.4 and AVRCP_FEATURE_CATEGORY_2 on their
respective target or controller profiles.

For backwards-compatibility, add a (default-enabled) `VolumeCategory =
true` configuration option under `[AVRCP]` to allow optionally disabling
this new check.
---
 profiles/audio/avrcp.c | 39 ++++++++++++++++++++++++++++++++++-----
 src/btd.h              |  1 +
 src/main.c             |  5 +++++
 src/main.conf          |  5 +++++
 4 files changed, 45 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 553673b19..005c3e306 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1775,6 +1775,16 @@  static uint8_t avrcp_handle_set_absolute_volume(struct avrcp *session,
 	if (len != 1)
 		goto err;
 
+	/**
+	 * The controller on the remote end is only allowed to call SetAbsoluteVolume
+	 * on our target if it's at least version 1.4 and a category-2 device.
+	 */
+	if (!session->target || session->target->version < 0x0104 ||
+			(btd_opts.avrcp.volume_category && !(session->target->features & AVRCP_FEATURE_CATEGORY_2))) {
+		error("Remote SetAbsoluteVolume rejected from non-category-2 peer");
+		goto err;
+	}
+
 	volume = pdu->params[0] & 0x7F;
 
 	media_transport_update_device_volume(session->dev, volume);
@@ -3767,6 +3777,16 @@  static void avrcp_volume_changed(struct avrcp *session,
 	struct avrcp_player *player = target_get_player(session);
 	int8_t volume;
 
+	/**
+	 * The target on the remote end is only allowed to reply to EVENT_VOLUME_CHANGED
+	 * on our controller if it's at least version 1.4 and a category-2 device.
+	 */
+	if (!session->controller || session->controller->version < 0x0104 ||
+			(btd_opts.avrcp.volume_category && !(session->controller->features & AVRCP_FEATURE_CATEGORY_2))) {
+		error("Remote EVENT_VOLUME_CHANGED rejected from non-category-2 peer");
+		return;
+	}
+
 	volume = pdu->params[1] & 0x7F;
 
 	/* Always attempt to update the transport volume */
@@ -4041,7 +4061,7 @@  static gboolean avrcp_get_capabilities_resp(struct avctp *conn, uint8_t code,
 		case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
 		case AVRCP_EVENT_UIDS_CHANGED:
 		case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
-			/* These events above requires a player */
+			/* These events above require a player */
 			if (!session->controller ||
 						!session->controller->player)
 				break;
@@ -4245,10 +4265,13 @@  static void target_init(struct avrcp *session)
 	if (target->version < 0x0104)
 		return;
 
+	if (!btd_opts.avrcp.volume_category || target->features & AVRCP_FEATURE_CATEGORY_2)
+		session->supported_events |=
+				(1 << AVRCP_EVENT_VOLUME_CHANGED);
+
 	session->supported_events |=
 				(1 << AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) |
-				(1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED) |
-				(1 << AVRCP_EVENT_VOLUME_CHANGED);
+				(1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED);
 
 	/* Only check capabilities if controller is not supported */
 	if (session->controller == NULL)
@@ -4665,8 +4688,11 @@  int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
 		return -ENOTCONN;
 
 	if (notify) {
-		if (!session->target)
+		if (!session->target || session->target->version < 0x0104 ||
+				(btd_opts.avrcp.volume_category && !(session->target->features & AVRCP_FEATURE_CATEGORY_2))) {
+			error("Can't send EVENT_VOLUME_CHANGED to non-category-2 peer");
 			return -ENOTSUP;
+		}
 		return avrcp_event(session, AVRCP_EVENT_VOLUME_CHANGED,
 								&volume);
 	}
@@ -4680,8 +4706,11 @@  int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
 						AVRCP_EVENT_VOLUME_CHANGED))
 			return -ENOTSUP;
 	} else {
-		if (!session->controller || session->controller->version < 0x0104)
+		if (!session->controller || session->controller->version < 0x0104 ||
+				(btd_opts.avrcp.volume_category && !(session->controller->features & AVRCP_FEATURE_CATEGORY_2))) {
+			error("Can't send SetAbsoluteVolume to non-category-2 peer");
 			return -ENOTSUP;
+		}
 	}
 
 	memset(buf, 0, sizeof(buf));
diff --git a/src/btd.h b/src/btd.h
index 147b61f12..07205aa69 100644
--- a/src/btd.h
+++ b/src/btd.h
@@ -106,6 +106,7 @@  struct btd_avdtp_opts {
 
 struct btd_avrcp_opts {
 	bool		volume_without_target;
+	bool		volume_category;
 };
 
 struct btd_advmon_opts {
diff --git a/src/main.c b/src/main.c
index 5bd3a035d..89ee6897c 100644
--- a/src/main.c
+++ b/src/main.c
@@ -167,6 +167,7 @@  static const char *avdtp_options[] = {
 
 static const char *avrcp_options[] = {
 	"VolumeWithoutTarget",
+	"VolumeCategory",
 	NULL
 };
 
@@ -1151,6 +1152,9 @@  static void parse_avrcp(GKeyFile *config)
 	parse_config_bool(config, "AVRCP",
 		"VolumeWithoutTarget",
 		&btd_opts.avrcp.volume_without_target);
+	parse_config_bool(config, "AVRCP",
+		"VolumeCategory",
+		&btd_opts.avrcp.volume_category);
 }
 
 static void parse_advmon(GKeyFile *config)
@@ -1220,6 +1224,7 @@  static void init_defaults(void)
 	btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
 
 	btd_opts.avrcp.volume_without_target = false;
+	btd_opts.avrcp.volume_category = true;
 
 	btd_opts.advmon.rssi_sampling_period = 0xFF;
 	btd_opts.csis.encrypt = true;
diff --git a/src/main.conf b/src/main.conf
index 5d206b9ec..fff13ed2f 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -311,6 +311,11 @@ 
 # version is ignored.
 #VolumeWithoutTarget = false
 
+# Validate that remote AVRCP profiles advertise the category-2 bit before
+# allowing SetAbsoluteVolume calls or registering for EVENT_VOLUME_CHANGED
+# notifications.
+#VolumeCategory = true
+
 [Policy]
 #
 # The ReconnectUUIDs defines the set of remote services that should try