| Message ID | 20240805140840.1606239-2-hadess@hadess.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix a number of static analysis issues #6 | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | 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 (160>80): "bluez-5.77/lib/sdp.c:1685:2: tainted_data_argument: The check "sent < size" contains the tainted expression "sent" which causes "size" to be considered tainted." 5: B1 Line exceeds max length (142>80): "bluez-5.77/lib/sdp.c:1686:3: overflow: The expression "size - sent" is deemed overflowed because at least one of its arguments has overflowed." 6: B1 Line exceeds max length (152>80): "bluez-5.77/lib/sdp.c:1686:3: overflow_sink: "size - sent", which might have underflowed, is passed to "send(session->sock, buf + sent, size - sent, 0)"." 8: B3 Line contains hard tab characters (\t): "1685| while (sent < size) {" 9: B3 Line contains hard tab characters (\t): "1686|-> int n = send(session->sock, buf + sent, size - sent, 0);" 10: B3 Line contains hard tab characters (\t): "1687| if (n < 0)" 11: B3 Line contains hard tab characters (\t): "1688| return -1;" |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
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=876731
---Test result---
Test Summary:
CheckPatch PASS 3.58 seconds
GitLint FAIL 3.12 seconds
BuildEll PASS 24.61 seconds
BluezMake PASS 1794.29 seconds
MakeCheck PASS 13.96 seconds
MakeDistcheck PASS 179.69 seconds
CheckValgrind PASS 255.36 seconds
CheckSmatch PASS 357.62 seconds
bluezmakeextell PASS 120.81 seconds
IncrementalBuild PASS 13591.94 seconds
ScanBuild PASS 1074.61 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,1/8] sdp: Ensure size doesn't 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
4: B1 Line exceeds max length (160>80): "bluez-5.77/lib/sdp.c:1685:2: tainted_data_argument: The check "sent < size" contains the tainted expression "sent" which causes "size" to be considered tainted."
5: B1 Line exceeds max length (142>80): "bluez-5.77/lib/sdp.c:1686:3: overflow: The expression "size - sent" is deemed overflowed because at least one of its arguments has overflowed."
6: B1 Line exceeds max length (152>80): "bluez-5.77/lib/sdp.c:1686:3: overflow_sink: "size - sent", which might have underflowed, is passed to "send(session->sock, buf + sent, size - sent, 0)"."
8: B3 Line contains hard tab characters (\t): "1685| while (sent < size) {"
9: B3 Line contains hard tab characters (\t): "1686|-> int n = send(session->sock, buf + sent, size - sent, 0);"
10: B3 Line contains hard tab characters (\t): "1687| if (n < 0)"
11: B3 Line contains hard tab characters (\t): "1688| return -1;"
[BlueZ,2/8] tools/isotest: Ensure ret doesn't 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
4: B1 Line exceeds max length (165>80): "bluez-5.77/tools/isotest.c:778:2: tainted_data_argument: The check "ret < count" contains the tainted expression "ret" which causes "count" to be considered tainted."
5: B1 Line exceeds max length (147>80): "bluez-5.77/tools/isotest.c:779:3: overflow: The expression "count - ret" is deemed overflowed because at least one of its arguments has overflowed."
6: B1 Line exceeds max length (237>80): "bluez-5.77/tools/isotest.c:779:3: overflow_sink: "count - ret", which might have underflowed, is passed to "read(fd, buf + ret, count - ret)". [Note: The source code implementation of the function has been overridden by a builtin model.]"
8: B3 Line contains hard tab characters (\t): "778| while (ret < count) {"
9: B3 Line contains hard tab characters (\t): "779|-> len = read(fd, buf + ret, count - ret);"
10: B3 Line contains hard tab characters (\t): "780| if (len < 0)"
11: B3 Line contains hard tab characters (\t): "781| return -errno;"
[BlueZ,3/8] health: mcap: Ensure sent doesn't 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
4: B1 Line exceeds max length (172>80): "bluez-5.77/profiles/health/mcap.c:390:2: tainted_data_argument: The check "sent < size" contains the tainted expression "sent" which causes "size" to be considered tainted."
5: B1 Line exceeds max length (154>80): "bluez-5.77/profiles/health/mcap.c:391:3: overflow: The expression "size - sent" is deemed overflowed because at least one of its arguments has overflowed."
6: B1 Line exceeds max length (155>80): "bluez-5.77/profiles/health/mcap.c:391:3: overflow_sink: "size - sent", which might have underflowed, is passed to "write(sock, buf_b + sent, size - sent)"."
8: B3 Line contains hard tab characters (\t): "390| while (sent < size) {"
9: B3 Line contains hard tab characters (\t): "391|-> int n = write(sock, buf_b + sent, size - sent);"
10: B3 Line contains hard tab characters (\t): "392| if (n < 0)"
11: B3 Line contains hard tab characters (\t): "393| return -1;"
[BlueZ,4/8] shared/tester: Add early failure check
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 (107>80): "bluez-5.77/src/shared/tester.c:946:2: return_constant: Function call "io_send(io, iov, 1)" may return -107."
8: B1 Line exceeds max length (123>80): "bluez-5.77/src/shared/tester.c:946:2: assignment: Assigning: "len" = "io_send(io, iov, 1)". The value of "len" is now -107."
9: B1 Line exceeds max length (258>80): "bluez-5.77/src/shared/tester.c:948:2: overrun-buffer-arg: Calling "tester_monitor" with "iov->iov_base" and "len" is suspicious because of the very large index, 18446744073709551509. The index may be due to a negative parameter being interpreted as unsigned."
10: B3 Line contains hard tab characters (\t): "946| len = io_send(io, iov, 1);"
12: B3 Line contains hard tab characters (\t): "948|-> tester_monitor('<', 0x0004, 0x0000, iov->iov_base, len);"
14: B3 Line contains hard tab characters (\t): "950| g_assert_cmpint(len, ==, iov->iov_len);"
[BlueZ,5/8] mesh: Fix possible 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
4: B1 Line exceeds max length (120>80): "bluez-5.77/mesh/net.c:3164:4: cast_overflow: Truncation due to cast operation on "msg->len - seg_off" from 32 to 8 bits."
5: B1 Line exceeds max length (95>80): "bluez-5.77/mesh/net.c:3164:4: overflow_assign: "seg_len" is assigned from "msg->len - seg_off"."
6: B1 Line exceeds max length (306>80): "bluez-5.77/mesh/net.c:3178:2: overflow_sink: "seg_len", which might have overflowed, is passed to "mesh_crypto_packet_build(false, msg->ttl, seq_num, msg->src, msg->remote, 0, msg->segmented, msg->key_aid, msg->szmic, false, msg->seqZero, segO, segN, msg->buf + seg_off, seg_len, packet + 1, &packet_len)"."
8: B3 Line contains hard tab characters (\t): "3177| /* TODO: Are we RXing on an LPN's behalf? Then set RLY bit */"
9: B3 Line contains hard tab characters (\t): "3178|-> if (!mesh_crypto_packet_build(false, msg->ttl, seq_num, msg->src,"
10: B3 Line contains hard tab characters (\t): "3179| msg->remote, 0, msg->segmented,"
11: B3 Line contains hard tab characters (\t): "3180| msg->key_aid, msg->szmic, false,"
[BlueZ,6/8] shared/gatt-db: Fix possible 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
13: B1 Line exceeds max length (152>80): "bluez-5.77/src/shared/gatt-db.c:612:2: assignment: Assigning: "len" = "uuid_to_le(uuid, value)". The value of "len" is now between 0 and 31 (inclusive)."
14: B1 Line exceeds max length (206>80): "bluez-5.77/src/shared/gatt-db.c:614:2: overrun-buffer-arg: Overrunning array "value" of 16 bytes by passing it to a function which accesses it at byte offset 30 using argument "len" (which evaluates to 31)."
15: B3 Line contains hard tab characters (\t): "612| len = uuid_to_le(uuid, value);"
17: B3 Line contains hard tab characters (\t): "614|-> service->attributes[0] = new_attribute(service, handle, type, value,"
18: B3 Line contains hard tab characters (\t): "615| len);"
19: B3 Line contains hard tab characters (\t): "616| if (!service->attributes[0]) {"
[BlueZ,7/8] shared/btsnoop: Avoid underflowing toread 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 (136>80): "bluez-5.77/src/shared/btsnoop.c:556:3: underflow: The decrement operator on the unsigned variable "toread" might result in an underflow."
5: B1 Line exceeds max length (236>80): "bluez-5.77/src/shared/btsnoop.c:572:2: overflow_sink: "toread", which might have underflowed, is passed to "read(btsnoop->fd, data, toread)". [Note: The source code implementation of the function has been overridden by a builtin model.]"
6: B3 Line contains hard tab characters (\t): "570| }"
8: B3 Line contains hard tab characters (\t): "572|-> len = read(btsnoop->fd, data, toread);"
9: B3 Line contains hard tab characters (\t): "573| if (len < 0) {"
10: B3 Line contains hard tab characters (\t): "574| btsnoop->aborted = true;"
[BlueZ,8/8] monitor: Check for possible integer underflow
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 (205>80): "bluez-5.77/monitor/control.c:1094:2: tainted_data_return: Called function "recv(data->fd, data->buf + data->offset, 1490UL - data->offset, MSG_DONTWAIT)", and a possible return value may be less than zero."
5: B1 Line exceeds max length (144>80): "bluez-5.77/monitor/control.c:1094:2: assign: Assigning: "len" = "recv(data->fd, data->buf + data->offset, 1490UL - data->offset, MSG_DONTWAIT)"."
6: B1 Line exceeds max length (119>80): "bluez-5.77/monitor/control.c:1099:2: overflow: The expression "data->offset" is considered to have possibly overflowed."
7: B1 Line exceeds max length (165>80): "bluez-5.77/monitor/control.c:1115:3: overflow: The expression "data->offset -= pktlen + 6" is deemed overflowed because at least one of its arguments has overflowed."
8: B1 Line exceeds max length (265>80): "bluez-5.77/monitor/control.c:1118:4: overflow_sink: "data->offset", which might have underflowed, is passed to "memmove(data->buf, data->buf + 6 + pktlen, data->offset)". [Note: The source code implementation of the function has been overridden by a builtin model.]"
10: B3 Line contains hard tab characters (\t): "1117| if (data->offset > 0)"
11: B3 Line contains hard tab characters (\t): "1118|-> memmove(data->buf, data->buf + MGMT_HDR_SIZE + pktlen,"
12: B3 Line contains hard tab characters (\t): "1119| data->offset);"
13: B3 Line contains hard tab characters (\t): "1120| }"
---
Regards,
Linux Bluetooth
Hi Bastien, On Mon, Aug 5, 2024 at 10:09 AM Bastien Nocera <hadess@hadess.net> wrote: > > Error: INTEGER_OVERFLOW (CWE-190): [#def1] [important] > bluez-5.77/lib/sdp.c:1685:2: tainted_data_argument: The check "sent < size" contains the tainted expression "sent" which causes "size" to be considered tainted. > bluez-5.77/lib/sdp.c:1686:3: overflow: The expression "size - sent" is deemed overflowed because at least one of its arguments has overflowed. > bluez-5.77/lib/sdp.c:1686:3: overflow_sink: "size - sent", which might have underflowed, is passed to "send(session->sock, buf + sent, size - sent, 0)". > 1684| > 1685| while (sent < size) { > 1686|-> int n = send(session->sock, buf + sent, size - sent, 0); > 1687| if (n < 0) > 1688| return -1; > --- > lib/sdp.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/lib/sdp.c b/lib/sdp.c > index 411a95b8a7d3..8a15ad803db1 100644 > --- a/lib/sdp.c > +++ b/lib/sdp.c > @@ -1678,13 +1678,13 @@ sdp_data_t *sdp_data_get(const sdp_record_t *rec, uint16_t attrId) > return NULL; > } > > -static int sdp_send_req(sdp_session_t *session, uint8_t *buf, uint32_t size) > +static int sdp_send_req(sdp_session_t *session, uint8_t *buf, size_t size) > { > - uint32_t sent = 0; > + size_t sent = 0; > > while (sent < size) { > int n = send(session->sock, buf + sent, size - sent, 0); > - if (n < 0) > + if (n < 0 || sent > SIZE_MAX - n) > return -1; Not really following you here, it seems the problem is that sent + n could overflow causing sent to loop around but if that happens don't we get a problem with send syscall itself? Using size_t makes sense but I guess we want n to also be ssize_t then, and perhap use if (n < 0 || n > size - sent) logic if we cannot trust syscalls returns the correct number of bytes. > sent += n; > } > -- > 2.45.2 > >
On Tue, 2024-08-06 at 14:38 -0400, Luiz Augusto von Dentz wrote: > Hi Bastien, > > On Mon, Aug 5, 2024 at 10:09 AM Bastien Nocera <hadess@hadess.net> > wrote: > > > > Error: INTEGER_OVERFLOW (CWE-190): [#def1] [important] > > bluez-5.77/lib/sdp.c:1685:2: tainted_data_argument: The check "sent > > < size" contains the tainted expression "sent" which causes "size" > > to be considered tainted. > > bluez-5.77/lib/sdp.c:1686:3: overflow: The expression "size - sent" > > is deemed overflowed because at least one of its arguments has > > overflowed. > > bluez-5.77/lib/sdp.c:1686:3: overflow_sink: "size - sent", which > > might have underflowed, is passed to "send(session->sock, buf + > > sent, size - sent, 0)". > > 1684| > > 1685| while (sent < size) { > > 1686|-> int n = send(session->sock, buf + sent, > > size - sent, 0); > > 1687| if (n < 0) > > 1688| return -1; > > --- > > lib/sdp.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/lib/sdp.c b/lib/sdp.c > > index 411a95b8a7d3..8a15ad803db1 100644 > > --- a/lib/sdp.c > > +++ b/lib/sdp.c > > @@ -1678,13 +1678,13 @@ sdp_data_t *sdp_data_get(const sdp_record_t > > *rec, uint16_t attrId) > > return NULL; > > } > > > > -static int sdp_send_req(sdp_session_t *session, uint8_t *buf, > > uint32_t size) > > +static int sdp_send_req(sdp_session_t *session, uint8_t *buf, > > size_t size) > > { > > - uint32_t sent = 0; > > + size_t sent = 0; > > > > while (sent < size) { > > int n = send(session->sock, buf + sent, size - > > sent, 0); > > - if (n < 0) > > + if (n < 0 || sent > SIZE_MAX - n) > > return -1; > > Not really following you here, it seems the problem is that sent + n > could overflow causing sent to loop around but if that happens don't > we get a problem with send syscall itself? Using size_t makes sense > but I guess we want n to also be ssize_t then, and perhap use if (n < > 0 || n > size - sent) logic if we cannot trust syscalls returns the > correct number of bytes. I don't think that the static analysis is this clever, it just wants to make sure that bounds checking is done to avoid integer overflows. I'm fine with quieting Coverity on my end, after fixing the data types if that's alright with you. Also goes for patch 8 of this series. > > > sent += n; > > } > > -- > > 2.45.2 > > > > > >
diff --git a/lib/sdp.c b/lib/sdp.c index 411a95b8a7d3..8a15ad803db1 100644 --- a/lib/sdp.c +++ b/lib/sdp.c @@ -1678,13 +1678,13 @@ sdp_data_t *sdp_data_get(const sdp_record_t *rec, uint16_t attrId) return NULL; } -static int sdp_send_req(sdp_session_t *session, uint8_t *buf, uint32_t size) +static int sdp_send_req(sdp_session_t *session, uint8_t *buf, size_t size) { - uint32_t sent = 0; + size_t sent = 0; while (sent < size) { int n = send(session->sock, buf + sent, size - sent, 0); - if (n < 0) + if (n < 0 || sent > SIZE_MAX - n) return -1; sent += n; }
Error: INTEGER_OVERFLOW (CWE-190): [#def1] [important] bluez-5.77/lib/sdp.c:1685:2: tainted_data_argument: The check "sent < size" contains the tainted expression "sent" which causes "size" to be considered tainted. bluez-5.77/lib/sdp.c:1686:3: overflow: The expression "size - sent" is deemed overflowed because at least one of its arguments has overflowed. bluez-5.77/lib/sdp.c:1686:3: overflow_sink: "size - sent", which might have underflowed, is passed to "send(session->sock, buf + sent, size - sent, 0)". 1684| 1685| while (sent < size) { 1686|-> int n = send(session->sock, buf + sent, size - sent, 0); 1687| if (n < 0) 1688| return -1; --- lib/sdp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)