Hi,
to, 2024-05-16 kello 11:13 +0200, Bastien Nocera kirjoitti:
> I was looking at the code in profiles/audio/avdtp.c surrounding those
> static analyser warnings, and couldn't understand how the seid arrays
> were constructed.
>
> There's similar code in android/ which might also need fixing.
>
> I could only find the code that assigned ".first_seid", nothing about
> how the rest of the structure is allocated and assigned.
These structs are from AVDTP spec, see eg. §8.13 for Start Stream
Command <-> struct start_req.
IIUC, they're actually arrays of struct seid, but the first element is
defined as a separate field. I guess the static checker chokes on that,
and not sure right now if this is even strictly allowed in C.
The structures are allocated in send_request() for the outgoing
messages and the bounds checking is via req->data_size. For incoming
messages they're raw message data from the remote device.
> Cheers
>
> PS: Please CC: on the answer, as I'm not subscribed to the list
>
> Error: ARRAY_VS_SINGLETON (CWE-119): [#def29] [important]
> bluez-5.75/profiles/audio/avdtp.c:1675:2: address_of: Taking address with "&start->first_seid" yields a singleton pointer.
> bluez-5.75/profiles/audio/avdtp.c:1675:2: assign: Assigning: "seid" = "&start->first_seid".
> bluez-5.75/profiles/audio/avdtp.c:1679:25: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations.
> # 1677| int i;
> # 1678|
> # 1679|-> for (i = 0; i < count; i++, seid++) {
> # 1680| if (seid->seid == id) {
> # 1681| req->collided = TRUE;
>
> Error: ARRAY_VS_SINGLETON (CWE-119): [#def30] [important]
> bluez-5.75/profiles/audio/avdtp.c:1690:2: address_of: Taking address with "&suspend->first_seid" yields a singleton pointer.
> bluez-5.75/profiles/audio/avdtp.c:1690:2: assign: Assigning: "seid" = "&suspend->first_seid".
> bluez-5.75/profiles/audio/avdtp.c:1694:25: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations.
> # 1692| int i;
> # 1693|
> # 1694|-> for (i = 0; i < count; i++, seid++) {
> # 1695| if (seid->seid == id) {
> # 1696| req->collided = TRUE;
>
> Error: ARRAY_VS_SINGLETON (CWE-119): [#def31] [important]
> bluez-5.75/profiles/audio/avdtp.c:1799:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer.
> bluez-5.75/profiles/audio/avdtp.c:1799:2: assign: Assigning: "seid" = "&req->first_seid".
> bluez-5.75/profiles/audio/avdtp.c:1801:30: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations.
> # 1799| seid = &req->first_seid;
> # 1800|
> # 1801|-> for (i = 0; i < seid_count; i++, seid++) {
> # 1802| failed_seid = seid->seid;
> # 1803|
>
> Error: ARRAY_VS_SINGLETON (CWE-119): [#def32] [important]
> bluez-5.75/profiles/audio/avdtp.c:1912:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer.
> bluez-5.75/profiles/audio/avdtp.c:1912:2: assign: Assigning: "seid" = "&req->first_seid".
> bluez-5.75/profiles/audio/avdtp.c:1914:30: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations.
> # 1912| seid = &req->first_seid;
> # 1913|
> # 1914|-> for (i = 0; i < seid_count; i++, seid++) {
> # 1915| failed_seid = seid->seid;
> # 1916|
--
Pauli Virtanen
