On Thu, 2024-05-16 at 19:00 +0300, Pauli Virtanen wrote: > 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. Thanks Pauli for the explanation. The static analysers didn't like that construct, and I've attempted to fix it in: https://lore.kernel.org/linux-bluetooth/20240530150057.444585-10-hadess@xxxxxxxxxx/T/#u If that doesn't pass muster, we can also do a simple "#define start_seq suspend_seq" and have a single actual struct type for both cases. Cheers > > > > 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| >
