Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Pauli Virtanen <pav@iki.fi>
To:     linux-bluetooth@vger.kernel.org
Cc:     Pauli Virtanen <pav@iki.fi>
Subject: [PATCH BlueZ v2 1/2] shared/util: use 64-bit bitmap in util_get/clear_uid
Date:   Sun,  5 Sep 2021 18:43:55 +0300
Message-Id: <20210905154356.8296-1-pav@iki.fi>
In-Reply-To: <20210829155012.164880-1-pav@iki.fi>
References: <20210829155012.164880-1-pav@iki.fi>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

The util_get/clear_uid functions use int type for bitmap, and are used
e.g. for SEID allocation. However, valid SEIDs are in range 1 to 0x3E
(AVDTP spec v1.3, 8.20.1), and 8*sizeof(int) is often smaller than 0x3E.

The function is also used in src/advertising.c, but an explicit maximum
value is always provided, so growing the bitmap size is safe there.

Use 64-bit bitmap instead, to be able to cover the valid range.
---
Changes in v2:
* Use ffsll

 android/avdtp.c        |  2 +-
 profiles/audio/avdtp.c |  2 +-
 src/advertising.c      |  2 +-
 src/shared/util.c      | 16 +++++++---------
 src/shared/util.h      |  4 ++--
 unit/test-avdtp.c      |  2 +-
 6 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/android/avdtp.c b/android/avdtp.c
index 8c2930ec1..a261a8e5f 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -34,7 +34,7 @@
 #include "../profiles/audio/a2dp-codecs.h"
 
 #define MAX_SEID 0x3E
-static unsigned int seids;
+static uint64_t seids;
 
 #ifndef MAX
 # define MAX(x, y) ((x) > (y) ? (x) : (y))
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 946231b71..25520ceec 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -44,7 +44,7 @@
 #define AVDTP_PSM 25
 
 #define MAX_SEID 0x3E
-static unsigned int seids;
+static uint64_t seids;
 
 #ifndef MAX
 # define MAX(x, y) ((x) > (y) ? (x) : (y))
diff --git a/src/advertising.c b/src/advertising.c
index bd79454d5..41b818650 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -48,7 +48,7 @@ struct btd_adv_manager {
 	uint8_t max_scan_rsp_len;
 	uint8_t max_ads;
 	uint32_t supported_flags;
-	unsigned int instance_bitmap;
+	uint64_t instance_bitmap;
 	bool extended_add_cmds;
 	int8_t min_tx_power;
 	int8_t max_tx_power;
diff --git a/src/shared/util.c b/src/shared/util.c
index 244756456..2887a3efa 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -124,30 +124,28 @@ unsigned char util_get_dt(const char *parent, const char *name)
 
 /* Helpers for bitfield operations */
 
-/* Find unique id in range from 1 to max but no bigger then
- * sizeof(int) * 8. ffs() is used since it is POSIX standard
- */
-uint8_t util_get_uid(unsigned int *bitmap, uint8_t max)
+/* Find unique id in range from 1 to max but no bigger than 64. */
+uint8_t util_get_uid(uint64_t *bitmap, uint8_t max)
 {
 	uint8_t id;
 
-	id = ffs(~*bitmap);
+	id = ffsll(~*bitmap);
 
 	if (!id || id > max)
 		return 0;
 
-	*bitmap |= 1u << (id - 1);
+	*bitmap |= ((uint64_t)1) << (id - 1);
 
 	return id;
 }
 
 /* Clear id bit in bitmap */
-void util_clear_uid(unsigned int *bitmap, uint8_t id)
+void util_clear_uid(uint64_t *bitmap, uint8_t id)
 {
-	if (!id)
+	if (!id || id > 64)
 		return;
 
-	*bitmap &= ~(1u << (id - 1));
+	*bitmap &= ~(((uint64_t)1) << (id - 1));
 }
 
 static const struct {
diff --git a/src/shared/util.h b/src/shared/util.h
index 9920b7f76..60908371d 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -102,8 +102,8 @@ void util_hexdump(const char dir, const unsigned char *buf, size_t len,
 
 unsigned char util_get_dt(const char *parent, const char *name);
 
-uint8_t util_get_uid(unsigned int *bitmap, uint8_t max);
-void util_clear_uid(unsigned int *bitmap, uint8_t id);
+uint8_t util_get_uid(uint64_t *bitmap, uint8_t max);
+void util_clear_uid(uint64_t *bitmap, uint8_t id);
 
 const char *bt_uuid16_to_str(uint16_t uuid);
 const char *bt_uuid32_to_str(uint32_t uuid);
diff --git a/unit/test-avdtp.c b/unit/test-avdtp.c
index f5340d6f3..4e8a68c6b 100644
--- a/unit/test-avdtp.c
+++ b/unit/test-avdtp.c
@@ -550,7 +550,7 @@ static void test_server_seid(gconstpointer data)
 	struct avdtp_local_sep *sep;
 	unsigned int i;
 
-	for (i = 0; i < sizeof(int) * 8; i++) {
+	for (i = 0; i < MAX_SEID; i++) {
 		sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
 						AVDTP_MEDIA_TYPE_AUDIO,
 						0x00, TRUE, &sep_ind, NULL,
-- 
2.31.1