Hi Pauli,
On Sun, Aug 29, 2021 at 8:52 AM Pauli Virtanen <pav@xxxxxx> wrote:
>
> 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.
> ---
> android/avdtp.c | 2 +-
> profiles/audio/avdtp.c | 2 +-
> src/advertising.c | 2 +-
> src/shared/util.c | 27 +++++++++++++++------------
> src/shared/util.h | 4 ++--
> unit/test-avdtp.c | 2 +-
> 6 files changed, 21 insertions(+), 18 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..723dedd75 100644
> --- a/src/shared/util.c
> +++ b/src/shared/util.c
> @@ -124,30 +124,33 @@ 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);
Can't we use ffsll instead of using a for loop testing every bit?
Afaik long long should be at least 64 bits.
> + if (max > 64)
> + max = 64;
>
> - if (!id || id > max)
> - return 0;
> + for (id = 1; id <= max; ++id) {
> + uint64_t mask = ((uint64_t)1) << (id - 1);
>
> - *bitmap |= 1u << (id - 1);
> + if (!(*bitmap & mask)) {
> + *bitmap |= mask;
> + return id;
> + }
> + }
>
> - return id;
> + return 0;
> }
>
> /* 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 == 0 || 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
>
--
Luiz Augusto von Dentz
