14 defects fixed, and 1 error check added.
Let me know whether there's any problems with the implementation, I'm
thinking in particular of the avdtp changes which are pretty invasive.
Cheers
Bastien Nocera (9):
rctest: Fix possible overrun
mgmt-tester: Fix buffer overrun
l2test: Add missing error checking
rfkill: Avoid using a signed int for an unsigned variable
shared/mainloop: Fix integer overflow
sdp: Fix ineffective error guard
obexd: Fix buffer overrun
bap: Fix more memory leaks on error
avdtp: Fix manipulating struct as an array
gobex/gobex.c | 2 +-
lib/sdp.c | 8 ++++----
profiles/audio/avdtp.c | 33 ++++++++++++++++++++++-----------
profiles/audio/bap.c | 5 ++++-
src/rfkill.c | 2 +-
src/shared/mainloop-notify.c | 3 ++-
tools/l2test.c | 5 +++++
tools/mgmt-tester.c | 2 +-
tools/rctest.c | 3 ++-
9 files changed, 42 insertions(+), 21 deletions(-)
--
2.45.1
Error: OVERRUN (CWE-119): [#def56] [important]
bluez-5.76/tools/mgmt-tester.c:12667:2: identity_transfer: Passing "512UL" as argument 3 to function "vhci_read_devcd", which returns that argument.
bluez-5.76/tools/mgmt-tester.c:12667:2: assignment: Assigning: "read" = "vhci_read_devcd(vhci, buf, 512UL)". The value of "read" is now 512.
bluez-5.76/tools/mgmt-tester.c:12674:2: overrun-local: Overrunning array "buf" of 513 bytes at byte offset 513 using index "read + 1" (which evaluates to 513).
12672| }
12673| /* Make sure buf is nul-terminated */
12674|-> buf[read + 1] = '\0';
12675|
12676| /* Verify if all devcoredump header fields are present */
Fixes: 49d06560692f ("mgmt-tester: Fix non-nul-terminated string")
---
tools/mgmt-tester.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index 8076ec105ebb..1d5c82ae0745 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -12671,7 +12671,7 @@ static void verify_devcd(void *user_data)
return;
}
/* Make sure buf is nul-terminated */
- buf[read + 1] = '\0';
+ buf[read] = '\0';
/* Verify if all devcoredump header fields are present */
line = strtok_r(buf, delim, &saveptr);
--
2.45.1
signalfd_siginfo uses a u32 for the signal number, but siginfo_t uses a
signed integer for it, so an (unlikely) big value for the signal number
could result in a negative value being passed to the callbacks. Catch
that and bail early.
Error: INTEGER_OVERFLOW (CWE-190): [#def44] [important]
bluez-5.76/src/shared/mainloop-notify.c:132:2: tainted_data_argument: The value "si" is considered tainted.
bluez-5.76/src/shared/mainloop-notify.c:137:3: tainted_data_argument: "si.ssi_signo" is considered tainted.
bluez-5.76/src/shared/mainloop-notify.c:137:3: underflow: The cast of "si.ssi_signo" to a signed type could result in a negative number.
135|
136| if (data && data->func)
137|-> data->func(si.ssi_signo, data->user_data);
138|
139| return true;
---
src/shared/mainloop-notify.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/shared/mainloop-notify.c b/src/shared/mainloop-notify.c
index 33be3cf8d78e..11989512e013 100644
--- a/src/shared/mainloop-notify.c
+++ b/src/shared/mainloop-notify.c
@@ -15,6 +15,7 @@
#define _GNU_SOURCE
#include <stdio.h>
#include <errno.h>
+#include <limits.h>
#include <unistd.h>
#include <stdlib.h>
#include <stddef.h>
@@ -130,7 +131,7 @@ static bool signal_read(struct io *io, void *user_data)
fd = io_get_fd(io);
result = read(fd, &si, sizeof(si));
- if (result != sizeof(si))
+ if (result != sizeof(si) || si.ssi_signo > INT_MAX)
return false;
if (data && data->func)
--
2.45.1
send() might fail and return a negative len, catch that to avoid
advancing the send buffer in the wrong direction and causing all sorts
of problems.
977|-> len = send(sk, buf + sent, buflen, 0);
978|
979| sent += len;
---
tools/l2test.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/l2test.c b/tools/l2test.c
index 011a68c3781e..7b6c36e165da 100644
--- a/tools/l2test.c
+++ b/tools/l2test.c
@@ -975,6 +975,11 @@ static void do_send(int sk)
buflen = (size > omtu) ? omtu : size;
len = send(sk, buf + sent, buflen, 0);
+ if (len < 0) {
+ syslog(LOG_ERR, "Send failed: %s (%d)",
+ strerror(errno), errno);
+ exit(1);
+ }
sent += len;
size -= len;
--
2.45.1
Error: RESOURCE_LEAK (CWE-772): [#def32] [important]
bluez-5.76/profiles/audio/bap.c:1166:4: alloc_arg: "asprintf" allocates memory that is stored into "path". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.76/profiles/audio/bap.c:1178:5: leaked_storage: Variable "path" going out of scope leaks the storage it points to.
1176| free(l3_caps);
1177| ret = false;
1178|-> goto group_fail;
1179| }
1180|
Error: RESOURCE_LEAK (CWE-772): [#def33] [important]
bluez-5.76/profiles/audio/bap.c:1166:4: alloc_arg: "asprintf" allocates memory that is stored into "path". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.76/profiles/audio/bap.c:1199:5: leaked_storage: Variable "path" going out of scope leaks the storage it points to.
1197|
1198| if (matched_lpac == NULL || merged_caps == NULL)
1199|-> continue;
1200|
1201| create_stream_for_bis(bap_data, matched_lpac, qos,
---
profiles/audio/bap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 3fcf21df58aa..53e7b3e34378 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -1174,6 +1174,7 @@ static bool parse_base(struct bap_data *bap_data, struct bt_iso_base *base,
if (!util_iov_pull_u8(&iov,
(void *)&l3_caps->iov_len)) {
free(l3_caps);
+ free(path);
ret = false;
goto group_fail;
}
@@ -1195,8 +1196,10 @@ static bool parse_base(struct bap_data *bap_data, struct bt_iso_base *base,
l2_caps, l3_caps, &matched_lpac,
&merged_caps);
- if (matched_lpac == NULL || merged_caps == NULL)
+ if (matched_lpac == NULL || merged_caps == NULL) {
+ free(path);
continue;
+ }
create_stream_for_bis(bap_data, matched_lpac, qos,
merged_caps, meta, path);
--
2.45.1
Error: OVERRUN (CWE-119): [#def57] [important]
bluez-5.76/tools/rctest.c:556:3: return_constant: Function call "read(fd, buf, data_size)" may return -1. [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.76/tools/rctest.c:556:3: assignment: Assigning: "len" = "read(fd, buf, data_size)". The value of "len" is now -1.
bluez-5.76/tools/rctest.c:557:3: overrun-buffer-arg: Calling "send" with "buf" and "len" is suspicious because of the very large index, 18446744073709551615. The index may be due to a negative parameter being interpreted as unsigned.
555| }
556| len = read(fd, buf, data_size);
557|-> send(sk, buf, len, 0);
558| close(fd);
559| return;
---
tools/rctest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/rctest.c b/tools/rctest.c
index d31180880ef4..ff91eb2f159d 100644
--- a/tools/rctest.c
+++ b/tools/rctest.c
@@ -554,7 +554,8 @@ static void do_send(int sk)
exit(1);
}
len = read(fd, buf, data_size);
- send(sk, buf, len, 0);
+ if (len > 0)
+ send(sk, buf, len, 0);
close(fd);
return;
} else {
--
2.45.1
Don't access path at byte 2 when it might only contain a single byte.
Error: OVERRUN (CWE-119): [#def27] [important]
bluez-5.76/obexd/client/session.c:1135:2: alias: Assigning: "first" = """". "first" now points to byte 0 of """" (which consists of 1 bytes).
bluez-5.76/obexd/client/session.c:1142:2: overrun-buffer-val: Overrunning buffer pointed to by "first" of 1 bytes by passing it to a function which accesses it at byte offset 2.
1140| req->index++;
1141|
1142|-> p->req_id = g_obex_setpath(p->session->obex, first, setpath_cb, p, err);
1143| if (*err != NULL)
1144| return (*err)->code;
---
gobex/gobex.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gobex/gobex.c b/gobex/gobex.c
index fdeb11c65130..40d6b8129b00 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -1611,7 +1611,7 @@ guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func,
memset(&data, 0, sizeof(data));
- if (path != NULL && strncmp("..", path, 2) == 0) {
+ if (path != NULL && strlen(path) >= 2 && strncmp("..", path, 2) == 0) {
data.flags = 0x03;
folder = (path[2] == '/') ? &path[3] : NULL;
} else {
--
2.45.1
Error: INTEGER_OVERFLOW (CWE-190): [#def37] [important]
bluez-5.76/src/rfkill.c:101:3: tainted_data_argument: The value "event" is considered tainted.
bluez-5.76/src/rfkill.c:105:3: tainted_data_argument: "event.idx" is considered tainted.
bluez-5.76/src/rfkill.c:105:3: underflow: The cast of "event.idx" to a signed type could result in a negative number.
103| break;
104|
105|-> id = get_adapter_id_for_rfkill(event.idx);
106|
107| if (index == id) {
Error: INTEGER_OVERFLOW (CWE-190): [#def38] [important]
bluez-5.76/src/rfkill.c:133:2: tainted_data_argument: The value "event" is considered tainted.
bluez-5.76/src/rfkill.c:143:2: tainted_data_argument: "event.idx" is considered tainted.
bluez-5.76/src/rfkill.c:157:2: underflow: The cast of "event.idx" to a signed type could result in a negative number.
155| return TRUE;
156|
157|-> id = get_adapter_id_for_rfkill(event.idx);
158| if (id < 0)
159| return TRUE;
---
src/rfkill.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rfkill.c b/src/rfkill.c
index a0a50d9e45d9..8a0e48f01c4f 100644
--- a/src/rfkill.c
+++ b/src/rfkill.c
@@ -55,7 +55,7 @@ struct rfkill_event {
};
#define RFKILL_EVENT_SIZE_V1 8
-static int get_adapter_id_for_rfkill(int rfkill_id)
+static int get_adapter_id_for_rfkill(uint32_t rfkill_id)
{
char sysname[PATH_MAX];
int namefd;
--
2.45.1
Don't manipulate the "req" structs as if they were flat arrays, static
analysis and humans are both equally confused by this kind of usage.
Error: ARRAY_VS_SINGLETON (CWE-119): [#def26] [important]
bluez-5.76/profiles/audio/avdtp.c:1675:2: address_of: Taking address with "&start->first_seid" yields a singleton pointer.
bluez-5.76/profiles/audio/avdtp.c:1675:2: assign: Assigning: "seid" = "&start->first_seid".
bluez-5.76/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): [#def27] [important]
bluez-5.76/profiles/audio/avdtp.c:1690:2: address_of: Taking address with "&suspend->first_seid" yields a singleton pointer.
bluez-5.76/profiles/audio/avdtp.c:1690:2: assign: Assigning: "seid" = "&suspend->first_seid".
bluez-5.76/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): [#def28] [important]
bluez-5.76/profiles/audio/avdtp.c:1799:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer.
bluez-5.76/profiles/audio/avdtp.c:1799:2: assign: Assigning: "seid" = "&req->first_seid".
bluez-5.76/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): [#def29] [important]
bluez-5.76/profiles/audio/avdtp.c:1912:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer.
bluez-5.76/profiles/audio/avdtp.c:1912:2: assign: Assigning: "seid" = "&req->first_seid".
bluez-5.76/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|
---
profiles/audio/avdtp.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 3667e08400dd..38c1870e619d 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -429,6 +429,20 @@ static void avdtp_sep_set_state(struct avdtp *session,
struct avdtp_local_sep *sep,
avdtp_state_t state);
+#define REQ_GET_NTH_SEID(x) \
+ static struct seid * \
+ x##_req_get_nth_seid(struct x##_req *req, int count, int i) \
+ { \
+ if (count == 0 || i >= count) \
+ return NULL; \
+ if (i == 1) \
+ return &req->first_seid; \
+ return &req->other_seids[i]; \
+ }
+
+REQ_GET_NTH_SEID(start)
+REQ_GET_NTH_SEID(suspend)
+
static const char *avdtp_statestr(avdtp_state_t state)
{
switch (state) {
@@ -1672,11 +1686,11 @@ static void check_seid_collision(struct pending_req *req, uint8_t id)
static void check_start_collision(struct pending_req *req, uint8_t id)
{
struct start_req *start = req->data;
- struct seid *seid = &start->first_seid;
int count = 1 + req->data_size - sizeof(struct start_req);
int i;
- for (i = 0; i < count; i++, seid++) {
+ for (i = 0; i < count; i++) {
+ struct seid *seid = start_req_get_nth_seid(start, count, i);
if (seid->seid == id) {
req->collided = TRUE;
return;
@@ -1687,11 +1701,11 @@ static void check_start_collision(struct pending_req *req, uint8_t id)
static void check_suspend_collision(struct pending_req *req, uint8_t id)
{
struct suspend_req *suspend = req->data;
- struct seid *seid = &suspend->first_seid;
int count = 1 + req->data_size - sizeof(struct suspend_req);
int i;
- for (i = 0; i < count; i++, seid++) {
+ for (i = 0; i < count; i++) {
+ struct seid *seid = suspend_req_get_nth_seid(suspend, count, i);
if (seid->seid == id) {
req->collided = TRUE;
return;
@@ -1785,7 +1799,6 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
struct avdtp_local_sep *sep;
struct avdtp_stream *stream;
struct stream_rej rej;
- struct seid *seid;
uint8_t err, failed_seid;
int seid_count, i;
@@ -1796,9 +1809,9 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
seid_count = 1 + size - sizeof(struct start_req);
- seid = &req->first_seid;
+ for (i = 0; i < seid_count; i++) {
+ struct seid *seid = start_req_get_nth_seid(req, seid_count, i);
- for (i = 0; i < seid_count; i++, seid++) {
failed_seid = seid->seid;
sep = find_local_sep_by_seid(session, seid->seid);
@@ -1898,7 +1911,6 @@ static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
struct avdtp_local_sep *sep;
struct avdtp_stream *stream;
struct stream_rej rej;
- struct seid *seid;
uint8_t err, failed_seid;
int seid_count, i;
@@ -1909,9 +1921,8 @@ static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
seid_count = 1 + size - sizeof(struct suspend_req);
- seid = &req->first_seid;
-
- for (i = 0; i < seid_count; i++, seid++) {
+ for (i = 0; i < seid_count; i++) {
+ struct seid *seid = suspend_req_get_nth_seid(req, seid_count, i);
failed_seid = seid->seid;
sep = find_local_sep_by_seid(session, seid->seid);
--
2.45.1
The return value from gen_attridseq_pdu() can be -ENOMEM or the always
positive return value from sdp_gen_pdu(), but we only guard against a
single negative return value "-1" (-EPERM).
Check for all negative values to avoid manipulating a negative length as
a valid one.
Error: INTEGER_OVERFLOW (CWE-190): [#def10] [important]
bluez-5.76/lib/sdp.c:4082:2: tainted_data_return: Called function "gen_attridseq_pdu(pdata, attrid_list, ((reqtype == SDP_ATTR_REQ_INDIVIDUAL) ? 9 : 10))", and a possible return value is known to be less than zero.
bluez-5.76/lib/sdp.c:4082:2: assign: Assigning: "seqlen" = "gen_attridseq_pdu(pdata, attrid_list, ((reqtype == SDP_ATTR_REQ_INDIVIDUAL) ? 9 : 10))".
bluez-5.76/lib/sdp.c:4091:2: overflow: The expression "t->reqsize" is considered to have possibly overflowed.
bluez-5.76/lib/sdp.c:4097:2: overflow: The expression "t->reqsize + cstate_len" is deemed overflowed because at least one of its arguments has overflowed.
bluez-5.76/lib/sdp.c:4097:2: overflow_sink: "t->reqsize + cstate_len", which might have underflowed, is passed to "sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len)".
4095| reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
4096|
4097|-> if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
4098| SDPERR("Error sending data:%m");
4099| t->err = errno;
Error: INTEGER_OVERFLOW (CWE-190): [#def11] [important]
bluez-5.76/lib/sdp.c:4466:2: tainted_data_return: Called function "gen_attridseq_pdu(pdata, attrids, ((reqtype == SDP_ATTR_REQ_INDIVIDUAL) ? 9 : 10))", and a possible return value is known to be less than zero.
bluez-5.76/lib/sdp.c:4466:2: assign: Assigning: "seqlen" = "gen_attridseq_pdu(pdata, attrids, ((reqtype == SDP_ATTR_REQ_INDIVIDUAL) ? 9 : 10))".
bluez-5.76/lib/sdp.c:4475:2: overflow: The expression "reqsize" is considered to have possibly overflowed.
bluez-5.76/lib/sdp.c:4480:2: assign: Assigning: "_reqsize" = "reqsize".
bluez-5.76/lib/sdp.c:4486:3: overflow: The expression "_reqsize + copy_cstate(_pdata, 2048U - _reqsize, cstate)" is deemed overflowed because at least one of its arguments has overflowed.
bluez-5.76/lib/sdp.c:4486:3: assign: Assigning: "reqsize" = "_reqsize + copy_cstate(_pdata, 2048U - _reqsize, cstate)".
bluez-5.76/lib/sdp.c:4492:3: overflow_sink: "reqsize", which might have underflowed, is passed to "sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize)".
4490| reqhdr->plen = htons(reqsize - sizeof(sdp_pdu_hdr_t));
4491| rsphdr = (sdp_pdu_hdr_t *) rspbuf;
4492|-> status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);
4493| if (rspsize < sizeof(sdp_pdu_hdr_t)) {
4494| SDPERR("Unexpected end of packet");
---
lib/sdp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index d43bbbd2de05..2e66505b21b8 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -3604,7 +3604,7 @@ sdp_record_t *sdp_service_attr_req(sdp_session_t *session, uint32_t handle,
/* get attr seq PDU form */
seqlen = gen_attridseq_pdu(pdata, attrids,
reqtype == SDP_ATTR_REQ_INDIVIDUAL? SDP_UINT16 : SDP_UINT32);
- if (seqlen == -1) {
+ if (seqlen < 0) {
errno = EINVAL;
goto end;
}
@@ -3959,7 +3959,7 @@ int sdp_service_attr_async(sdp_session_t *session, uint32_t handle, sdp_attrreq_
/* get attr seq PDU form */
seqlen = gen_attridseq_pdu(pdata, attrid_list,
reqtype == SDP_ATTR_REQ_INDIVIDUAL? SDP_UINT16 : SDP_UINT32);
- if (seqlen == -1) {
+ if (seqlen < 0) {
t->err = EINVAL;
goto end;
}
@@ -4081,7 +4081,7 @@ int sdp_service_search_attr_async(sdp_session_t *session, const sdp_list_t *sear
/* get attr seq PDU form */
seqlen = gen_attridseq_pdu(pdata, attrid_list,
reqtype == SDP_ATTR_REQ_INDIVIDUAL ? SDP_UINT16 : SDP_UINT32);
- if (seqlen == -1) {
+ if (seqlen < 0) {
t->err = EINVAL;
goto end;
}
@@ -4465,7 +4465,7 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
/* get attr seq PDU form */
seqlen = gen_attridseq_pdu(pdata, attrids,
reqtype == SDP_ATTR_REQ_INDIVIDUAL ? SDP_UINT16 : SDP_UINT32);
- if (seqlen == -1) {
+ if (seqlen < 0) {
errno = EINVAL;
status = -1;
goto end;
--
2.45.1
Hi,
to, 2024-05-30 kello 16:58 +0200, Bastien Nocera kirjoitti:
> Don't manipulate the "req" structs as if they were flat arrays, static
> analysis and humans are both equally confused by this kind of usage.
struct start_req {
union {
struct seid required[1];
struct seid seids[0];
};
} __attribute__ ((packed));
and access only via req->seids?
>
> Error: ARRAY_VS_SINGLETON (CWE-119): [#def26] [important]
> bluez-5.76/profiles/audio/avdtp.c:1675:2: address_of: Taking address with "&start->first_seid" yields a singleton pointer.
> bluez-5.76/profiles/audio/avdtp.c:1675:2: assign: Assigning: "seid" = "&start->first_seid".
> bluez-5.76/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): [#def27] [important]
> bluez-5.76/profiles/audio/avdtp.c:1690:2: address_of: Taking address with "&suspend->first_seid" yields a singleton pointer.
> bluez-5.76/profiles/audio/avdtp.c:1690:2: assign: Assigning: "seid" = "&suspend->first_seid".
> bluez-5.76/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): [#def28] [important]
> bluez-5.76/profiles/audio/avdtp.c:1799:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer.
> bluez-5.76/profiles/audio/avdtp.c:1799:2: assign: Assigning: "seid" = "&req->first_seid".
> bluez-5.76/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): [#def29] [important]
> bluez-5.76/profiles/audio/avdtp.c:1912:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer.
> bluez-5.76/profiles/audio/avdtp.c:1912:2: assign: Assigning: "seid" = "&req->first_seid".
> bluez-5.76/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|
> ---
> profiles/audio/avdtp.c | 33 ++++++++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index 3667e08400dd..38c1870e619d 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -429,6 +429,20 @@ static void avdtp_sep_set_state(struct avdtp *session,
> struct avdtp_local_sep *sep,
> avdtp_state_t state);
>
> +#define REQ_GET_NTH_SEID(x) \
> + static struct seid * \
> + x##_req_get_nth_seid(struct x##_req *req, int count, int i) \
> + { \
> + if (count == 0 || i >= count) \
> + return NULL; \
> + if (i == 1) \
> + return &req->first_seid; \
> + return &req->other_seids[i]; \
(i == 0) and [i - 1]?
> + }
> +
> +REQ_GET_NTH_SEID(start)
> +REQ_GET_NTH_SEID(suspend)
> +
> static const char *avdtp_statestr(avdtp_state_t state)
> {
> switch (state) {
> @@ -1672,11 +1686,11 @@ static void check_seid_collision(struct pending_req *req, uint8_t id)
> static void check_start_collision(struct pending_req *req, uint8_t id)
> {
> struct start_req *start = req->data;
> - struct seid *seid = &start->first_seid;
> int count = 1 + req->data_size - sizeof(struct start_req);
> int i;
>
> - for (i = 0; i < count; i++, seid++) {
> + for (i = 0; i < count; i++) {
> + struct seid *seid = start_req_get_nth_seid(start, count, i);
> if (seid->seid == id) {
> req->collided = TRUE;
> return;
> @@ -1687,11 +1701,11 @@ static void check_start_collision(struct pending_req *req, uint8_t id)
> static void check_suspend_collision(struct pending_req *req, uint8_t id)
> {
> struct suspend_req *suspend = req->data;
> - struct seid *seid = &suspend->first_seid;
> int count = 1 + req->data_size - sizeof(struct suspend_req);
> int i;
>
> - for (i = 0; i < count; i++, seid++) {
> + for (i = 0; i < count; i++) {
> + struct seid *seid = suspend_req_get_nth_seid(suspend, count, i);
> if (seid->seid == id) {
> req->collided = TRUE;
> return;
> @@ -1785,7 +1799,6 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
> struct avdtp_local_sep *sep;
> struct avdtp_stream *stream;
> struct stream_rej rej;
> - struct seid *seid;
> uint8_t err, failed_seid;
> int seid_count, i;
>
> @@ -1796,9 +1809,9 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
>
> seid_count = 1 + size - sizeof(struct start_req);
>
> - seid = &req->first_seid;
> + for (i = 0; i < seid_count; i++) {
> + struct seid *seid = start_req_get_nth_seid(req, seid_count, i);
>
> - for (i = 0; i < seid_count; i++, seid++) {
> failed_seid = seid->seid;
>
> sep = find_local_sep_by_seid(session, seid->seid);
> @@ -1898,7 +1911,6 @@ static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
> struct avdtp_local_sep *sep;
> struct avdtp_stream *stream;
> struct stream_rej rej;
> - struct seid *seid;
> uint8_t err, failed_seid;
> int seid_count, i;
>
> @@ -1909,9 +1921,8 @@ static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
>
> seid_count = 1 + size - sizeof(struct suspend_req);
>
> - seid = &req->first_seid;
> -
> - for (i = 0; i < seid_count; i++, seid++) {
> + for (i = 0; i < seid_count; i++) {
> + struct seid *seid = suspend_req_get_nth_seid(req, seid_count, i);
> failed_seid = seid->seid;
>
> sep = find_local_sep_by_seid(session, seid->seid);
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=857401
---Test result---
Test Summary:
CheckPatch FAIL 4.74 seconds
GitLint FAIL 3.24 seconds
BuildEll PASS 25.18 seconds
BluezMake PASS 1728.81 seconds
MakeCheck PASS 13.48 seconds
MakeDistcheck PASS 183.04 seconds
CheckValgrind PASS 257.35 seconds
CheckSmatch WARNING 363.75 seconds
bluezmakeextell PASS 122.78 seconds
IncrementalBuild PASS 14512.82 seconds
ScanBuild WARNING 1040.80 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,2/9] mgmt-tester: Fix buffer overrun
WARNING:UNKNOWN_COMMIT_ID: Unknown commit id '49d06560692f', maybe rebased or not pulled?
#60:
Fixes: 49d06560692f ("mgmt-tester: Fix non-nul-terminated string")
/github/workspace/src/src/13680511.patch total: 0 errors, 1 warnings, 8 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/13680511.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.
[BlueZ,6/9] sdp: Fix ineffective error guard
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#63:
4095| reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
/github/workspace/src/src/13680516.patch total: 0 errors, 1 warnings, 32 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/13680516.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.
[BlueZ,7/9] obexd: Fix buffer overrun
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#57:
1142|-> p->req_id = g_obex_setpath(p->session->obex, first, setpath_cb, p, err);
/github/workspace/src/src/13680515.patch total: 0 errors, 1 warnings, 8 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/13680515.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.
[BlueZ,9/9] avdtp: Fix manipulating struct as an array
WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#185: FILE: profiles/audio/avdtp.c:1925:
+ struct seid *seid = suspend_req_get_nth_seid(req, seid_count, i);
/github/workspace/src/src/13680514.patch total: 0 errors, 1 warnings, 82 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/13680514.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.
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,1/9] rctest: Fix possible overrun
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
4: B1 Line exceeds max length (200>80): "bluez-5.76/tools/rctest.c:556:3: return_constant: Function call "read(fd, buf, data_size)" may return -1. [Note: The source code implementation of the function has been overridden by a builtin model.]"
5: B1 Line exceeds max length (121>80): "bluez-5.76/tools/rctest.c:556:3: assignment: Assigning: "len" = "read(fd, buf, data_size)". The value of "len" is now -1."
6: B1 Line exceeds max length (233>80): "bluez-5.76/tools/rctest.c:557:3: overrun-buffer-arg: Calling "send" with "buf" and "len" is suspicious because of the very large index, 18446744073709551615. The index may be due to a negative parameter being interpreted as unsigned."
7: B3 Line contains hard tab characters (\t): "555| }"
8: B3 Line contains hard tab characters (\t): "556| len = read(fd, buf, data_size);"
9: B3 Line contains hard tab characters (\t): "557|-> send(sk, buf, len, 0);"
10: B3 Line contains hard tab characters (\t): "558| close(fd);"
11: B3 Line contains hard tab characters (\t): "559| return;"
[BlueZ,2/9] mgmt-tester: Fix buffer overrun
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
4: B1 Line exceeds max length (148>80): "bluez-5.76/tools/mgmt-tester.c:12667:2: identity_transfer: Passing "512UL" as argument 3 to function "vhci_read_devcd", which returns that argument."
5: B1 Line exceeds max length (140>80): "bluez-5.76/tools/mgmt-tester.c:12667:2: assignment: Assigning: "read" = "vhci_read_devcd(vhci, buf, 512UL)". The value of "read" is now 512."
6: B1 Line exceeds max length (159>80): "bluez-5.76/tools/mgmt-tester.c:12674:2: overrun-local: Overrunning array "buf" of 513 bytes at byte offset 513 using index "read + 1" (which evaluates to 513)."
7: B3 Line contains hard tab characters (\t): "12672| }"
8: B3 Line contains hard tab characters (\t): "12673| /* Make sure buf is nul-terminated */"
9: B3 Line contains hard tab characters (\t): "12674|-> buf[read + 1] = '\0';"
11: B3 Line contains hard tab characters (\t): "12676| /* Verify if all devcoredump header fields are present */"
[BlueZ,3/9] l2test: Add missing error checking
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
7: B3 Line contains hard tab characters (\t): "977|-> len = send(sk, buf + sent, buflen, 0);"
9: B3 Line contains hard tab characters (\t): "979| sent += len;"
[BlueZ,4/9] rfkill: Avoid using a signed int for an unsigned variable
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
4: B1 Line exceeds max length (94>80): "bluez-5.76/src/rfkill.c:101:3: tainted_data_argument: The value "event" is considered tainted."
5: B1 Line exceeds max length (88>80): "bluez-5.76/src/rfkill.c:105:3: tainted_data_argument: "event.idx" is considered tainted."
6: B1 Line exceeds max length (117>80): "bluez-5.76/src/rfkill.c:105:3: underflow: The cast of "event.idx" to a signed type could result in a negative number."
7: B3 Line contains hard tab characters (\t): "103| break;"
9: B3 Line contains hard tab characters (\t): "105|-> id = get_adapter_id_for_rfkill(event.idx);"
11: B3 Line contains hard tab characters (\t): "107| if (index == id) {"
14: B1 Line exceeds max length (94>80): "bluez-5.76/src/rfkill.c:133:2: tainted_data_argument: The value "event" is considered tainted."
15: B1 Line exceeds max length (88>80): "bluez-5.76/src/rfkill.c:143:2: tainted_data_argument: "event.idx" is considered tainted."
16: B1 Line exceeds max length (117>80): "bluez-5.76/src/rfkill.c:157:2: underflow: The cast of "event.idx" to a signed type could result in a negative number."
17: B3 Line contains hard tab characters (\t): "155| return TRUE;"
19: B3 Line contains hard tab characters (\t): "157|-> id = get_adapter_id_for_rfkill(event.idx);"
20: B3 Line contains hard tab characters (\t): "158| if (id < 0)"
21: B3 Line contains hard tab characters (\t): "159| return TRUE;"
[BlueZ,5/9] shared/mainloop: Fix integer overflow
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
9: B1 Line exceeds max length (107>80): "bluez-5.76/src/shared/mainloop-notify.c:132:2: tainted_data_argument: The value "si" is considered tainted."
10: B1 Line exceeds max length (107>80): "bluez-5.76/src/shared/mainloop-notify.c:137:3: tainted_data_argument: "si.ssi_signo" is considered tainted."
11: B1 Line exceeds max length (136>80): "bluez-5.76/src/shared/mainloop-notify.c:137:3: underflow: The cast of "si.ssi_signo" to a signed type could result in a negative number."
13: B3 Line contains hard tab characters (\t): "136| if (data && data->func)"
14: B3 Line contains hard tab characters (\t): "137|-> data->func(si.ssi_signo, data->user_data);"
16: B3 Line contains hard tab characters (\t): "139| return true;"
[BlueZ,6/9] sdp: Fix ineffective error guard
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
11: B1 Line exceeds max length (214>80): "bluez-5.76/lib/sdp.c:4082:2: tainted_data_return: Called function "gen_attridseq_pdu(pdata, attrid_list, ((reqtype == SDP_ATTR_REQ_INDIVIDUAL) ? 9 : 10))", and a possible return value is known to be less than zero."
12: B1 Line exceeds max length (148>80): "bluez-5.76/lib/sdp.c:4082:2: assign: Assigning: "seqlen" = "gen_attridseq_pdu(pdata, attrid_list, ((reqtype == SDP_ATTR_REQ_INDIVIDUAL) ? 9 : 10))"."
13: B1 Line exceeds max length (109>80): "bluez-5.76/lib/sdp.c:4091:2: overflow: The expression "t->reqsize" is considered to have possibly overflowed."
14: B1 Line exceeds max length (154>80): "bluez-5.76/lib/sdp.c:4097:2: overflow: The expression "t->reqsize + cstate_len" is deemed overflowed because at least one of its arguments has overflowed."
15: B1 Line exceeds max length (174>80): "bluez-5.76/lib/sdp.c:4097:2: overflow_sink: "t->reqsize + cstate_len", which might have underflowed, is passed to "sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len)"."
16: B3 Line contains hard tab characters (\t): "4095| reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));"
18: B3 Line contains hard tab characters (\t): "4097|-> if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {"
19: B3 Line contains hard tab characters (\t): "4098| SDPERR("Error sending data:%m");"
20: B3 Line contains hard tab characters (\t): "4099| t->err = errno;"
23: B1 Line exceeds max length (210>80): "bluez-5.76/lib/sdp.c:4466:2: tainted_data_return: Called function "gen_attridseq_pdu(pdata, attrids, ((reqtype == SDP_ATTR_REQ_INDIVIDUAL) ? 9 : 10))", and a possible return value is known to be less than zero."
24: B1 Line exceeds max length (144>80): "bluez-5.76/lib/sdp.c:4466:2: assign: Assigning: "seqlen" = "gen_attridseq_pdu(pdata, attrids, ((reqtype == SDP_ATTR_REQ_INDIVIDUAL) ? 9 : 10))"."
25: B1 Line exceeds max length (106>80): "bluez-5.76/lib/sdp.c:4475:2: overflow: The expression "reqsize" is considered to have possibly overflowed."
27: B1 Line exceeds max length (187>80): "bluez-5.76/lib/sdp.c:4486:3: overflow: The expression "_reqsize + copy_cstate(_pdata, 2048U - _reqsize, cstate)" is deemed overflowed because at least one of its arguments has overflowed."
28: B1 Line exceeds max length (119>80): "bluez-5.76/lib/sdp.c:4486:3: assign: Assigning: "reqsize" = "_reqsize + copy_cstate(_pdata, 2048U - _reqsize, cstate)"."
29: B1 Line exceeds max length (164>80): "bluez-5.76/lib/sdp.c:4492:3: overflow_sink: "reqsize", which might have underflowed, is passed to "sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize)"."
30: B3 Line contains hard tab characters (\t): "4490| reqhdr->plen = htons(reqsize - sizeof(sdp_pdu_hdr_t));"
31: B3 Line contains hard tab characters (\t): "4491| rsphdr = (sdp_pdu_hdr_t *) rspbuf;"
32: B1 Line exceeds max length (83>80): "4492|-> status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);"
32: B3 Line contains hard tab characters (\t): "4492|-> status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);"
33: B3 Line contains hard tab characters (\t): "4493| if (rspsize < sizeof(sdp_pdu_hdr_t)) {"
34: B3 Line contains hard tab characters (\t): "4494| SDPERR("Unexpected end of packet");"
[BlueZ,7/9] obexd: Fix buffer overrun
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
6: B1 Line exceeds max length (141>80): "bluez-5.76/obexd/client/session.c:1135:2: alias: Assigning: "first" = """". "first" now points to byte 0 of """" (which consists of 1 bytes)."
7: B1 Line exceeds max length (177>80): "bluez-5.76/obexd/client/session.c:1142:2: overrun-buffer-val: Overrunning buffer pointed to by "first" of 1 bytes by passing it to a function which accesses it at byte offset 2."
8: B3 Line contains hard tab characters (\t): "1140| req->index++;"
10: B1 Line exceeds max length (81>80): "1142|-> p->req_id = g_obex_setpath(p->session->obex, first, setpath_cb, p, err);"
10: B3 Line contains hard tab characters (\t): "1142|-> p->req_id = g_obex_setpath(p->session->obex, first, setpath_cb, p, err);"
11: B3 Line contains hard tab characters (\t): "1143| if (*err != NULL)"
12: B3 Line contains hard tab characters (\t): "1144| return (*err)->code;"
[BlueZ,8/9] bap: Fix more memory leaks on error
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
4: B1 Line exceeds max length (201>80): "bluez-5.76/profiles/audio/bap.c:1166:4: alloc_arg: "asprintf" allocates memory that is stored into "path". [Note: The source code implementation of the function has been overridden by a builtin model.]"
5: B1 Line exceeds max length (122>80): "bluez-5.76/profiles/audio/bap.c:1178:5: leaked_storage: Variable "path" going out of scope leaks the storage it points to."
6: B3 Line contains hard tab characters (\t): "1176| free(l3_caps);"
7: B3 Line contains hard tab characters (\t): "1177| ret = false;"
8: B3 Line contains hard tab characters (\t): "1178|-> goto group_fail;"
9: B3 Line contains hard tab characters (\t): "1179| }"
13: B1 Line exceeds max length (201>80): "bluez-5.76/profiles/audio/bap.c:1166:4: alloc_arg: "asprintf" allocates memory that is stored into "path". [Note: The source code implementation of the function has been overridden by a builtin model.]"
14: B1 Line exceeds max length (122>80): "bluez-5.76/profiles/audio/bap.c:1199:5: leaked_storage: Variable "path" going out of scope leaks the storage it points to."
16: B3 Line contains hard tab characters (\t): "1198| if (matched_lpac == NULL || merged_caps == NULL)"
17: B3 Line contains hard tab characters (\t): "1199|-> continue;"
19: B3 Line contains hard tab characters (\t): "1201| create_stream_for_bis(bap_data, matched_lpac, qos,"
[BlueZ,9/9] avdtp: Fix manipulating struct as an array
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
7: B1 Line exceeds max length (122>80): "bluez-5.76/profiles/audio/avdtp.c:1675:2: address_of: Taking address with "&start->first_seid" yields a singleton pointer."
8: B1 Line exceeds max length (91>80): "bluez-5.76/profiles/audio/avdtp.c:1675:2: assign: Assigning: "seid" = "&start->first_seid"."
9: B1 Line exceeds max length (142>80): "bluez-5.76/profiles/audio/avdtp.c:1679:25: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations."
17: B1 Line exceeds max length (124>80): "bluez-5.76/profiles/audio/avdtp.c:1690:2: address_of: Taking address with "&suspend->first_seid" yields a singleton pointer."
18: B1 Line exceeds max length (93>80): "bluez-5.76/profiles/audio/avdtp.c:1690:2: assign: Assigning: "seid" = "&suspend->first_seid"."
19: B1 Line exceeds max length (142>80): "bluez-5.76/profiles/audio/avdtp.c:1694:25: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations."
20: B3 Line contains hard tab characters (\t): "1692| int i;"
22: B3 Line contains hard tab characters (\t): "1694|-> for (i = 0; i < count; i++, seid++) {"
23: B3 Line contains hard tab characters (\t): "1695| if (seid->seid == id) {"
24: B3 Line contains hard tab characters (\t): "1696| req->collided = TRUE;"
27: B1 Line exceeds max length (120>80): "bluez-5.76/profiles/audio/avdtp.c:1799:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer."
28: B1 Line exceeds max length (89>80): "bluez-5.76/profiles/audio/avdtp.c:1799:2: assign: Assigning: "seid" = "&req->first_seid"."
29: B1 Line exceeds max length (142>80): "bluez-5.76/profiles/audio/avdtp.c:1801:30: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations."
30: B3 Line contains hard tab characters (\t): "1799| seid = &req->first_seid;"
32: B3 Line contains hard tab characters (\t): "1801|-> for (i = 0; i < seid_count; i++, seid++) {"
33: B3 Line contains hard tab characters (\t): "1802| failed_seid = seid->seid;"
37: B1 Line exceeds max length (120>80): "bluez-5.76/profiles/audio/avdtp.c:1912:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer."
38: B1 Line exceeds max length (89>80): "bluez-5.76/profiles/audio/avdtp.c:1912:2: assign: Assigning: "seid" = "&req->first_seid"."
39: B1 Line exceeds max length (142>80): "bluez-5.76/profiles/audio/avdtp.c:1914:30: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations."
40: B3 Line contains hard tab characters (\t): "1912| seid = &req->first_seid;"
42: B3 Line contains hard tab characters (\t): "1914|-> for (i = 0; i < seid_count; i++, seid++) {"
43: B3 Line contains hard tab characters (\t): "1915| failed_seid = seid->seid;"
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
tools/rctest.c:625:33: warning: non-ANSI function declaration of function 'automated_send_recv'
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
profiles/audio/avdtp.c:909:25: warning: Use of memory after it is freed
session->prio_queue = g_slist_remove(session->prio_queue, req);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/avdtp.c:916:24: warning: Use of memory after it is freed
session->req_queue = g_slist_remove(session->req_queue, req);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
---
Regards,
Linux Bluetooth
On Thu, 2024-05-30 at 19:57 +0300, Pauli Virtanen wrote:
> Hi,
>
> to, 2024-05-30 kello 16:58 +0200, Bastien Nocera kirjoitti:
> > Don't manipulate the "req" structs as if they were flat arrays,
> > static
> > analysis and humans are both equally confused by this kind of
> > usage.
>
> struct start_req {
> union {
> struct seid required[1];
> struct seid seids[0];
> };
> } __attribute__ ((packed));
>
> and access only via req->seids?
That's a good idea, I'll give it a try.
> <snip>
> > +#define
> > REQ_GET_NTH_SEID(x) \
> > + static struct seid
> > * \
> > + x##_req_get_nth_seid(struct x##_req *req, int count, int
> > i) \
> > + {
> > \
> > + if (count == 0 || i >=
> > count) \
> > + return
> > NULL; \
> > + if (i ==
> > 1) \
> > + return &req-
> > >first_seid; \
> > + return &req-
> > >other_seids[i]; \
>
> (i == 0) and [i - 1]?
*facepalm*
Yes, this will need a v2, thanks for spotting that.
Hi Bastien,
On Fri, May 31, 2024 at 11:51 AM Bastien Nocera <[email protected]> wrote:
>
> On Thu, 2024-05-30 at 19:57 +0300, Pauli Virtanen wrote:
> > Hi,
> >
> > to, 2024-05-30 kello 16:58 +0200, Bastien Nocera kirjoitti:
> > > Don't manipulate the "req" structs as if they were flat arrays,
> > > static
> > > analysis and humans are both equally confused by this kind of
> > > usage.
> >
> > struct start_req {
> > union {
> > struct seid required[1];
> > struct seid seids[0];
> > };
> > } __attribute__ ((packed));
> >
> > and access only via req->seids?
>
> That's a good idea, I'll give it a try.
>
> > <snip>
> > > +#define
> > > REQ_GET_NTH_SEID(x) \
> > > + static struct seid
> > > * \
> > > + x##_req_get_nth_seid(struct x##_req *req, int count, int
> > > i) \
> > > + {
> > > \
> > > + if (count == 0 || i >=
> > > count) \
> > > + return
> > > NULL; \
> > > + if (i ==
> > > 1) \
> > > + return &req-
> > > >first_seid; \
> > > + return &req-
> > > >other_seids[i]; \
> >
> > (i == 0) and [i - 1]?
>
> *facepalm*
>
> Yes, this will need a v2, thanks for spotting that.
Ive applied the set except for this one, please resend once you are
done with the suggested changes.
--
Luiz Augusto von Dentz
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Thu, 30 May 2024 16:57:54 +0200 you wrote:
> 14 defects fixed, and 1 error check added.
>
> Let me know whether there's any problems with the implementation, I'm
> thinking in particular of the avdtp changes which are pretty invasive.
>
> Cheers
>
> [...]
Here is the summary with links:
- [BlueZ,1/9] rctest: Fix possible overrun
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=24cf04939502
- [BlueZ,2/9] mgmt-tester: Fix buffer overrun
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=aa54087f13d5
- [BlueZ,3/9] l2test: Add missing error checking
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ccec5e8ef171
- [BlueZ,4/9] rfkill: Avoid using a signed int for an unsigned variable
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c9fcea121f9a
- [BlueZ,5/9] shared/mainloop: Fix integer overflow
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6cf9117bfd3f
- [BlueZ,6/9] sdp: Fix ineffective error guard
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=bd954700e631
- [BlueZ,7/9] obexd: Fix buffer overrun
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1764cea5c7fd
- [BlueZ,8/9] bap: Fix more memory leaks on error
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=cc8e6ef63509
- [BlueZ,9/9] avdtp: Fix manipulating struct as an array
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html