diff mbox series

[BlueZ,v2,11/11] unit/ringbuf: Fix ineffective guard due to signedness

Message ID 20240705085935.1255725-12-hadess@hadess.net (mailing list archive)
State Accepted
Commit c44a2a233d1b1873a7d4a9085c8d6bd61835bfac
Headers show
Series Fix a number of static analysis issues #5 | expand

Checks

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 7: B1 Line exceeds max length (230>80): "bluez-5.76/src/shared/ringbuf.c:240:2: ineffective_check: The check "len - end > 0UL", which appears to be a guard against integer overflow, is not a useful guard because it is either always true, or never true. This taints "len"." 8: B1 Line exceeds max length (147>80): "bluez-5.76/src/shared/ringbuf.c:242:3: overflow: The expression "len - end" might be negative, but is used in a context that treats it as unsigned." 9: B1 Line exceeds max length (248>80): "bluez-5.76/src/shared/ringbuf.c:242:3: overflow_sink: "len - end", which might be negative, is passed to "memcpy(ringbuf->buffer, str + end, len - end)". [Note: The source code implementation of the function has been overridden by a builtin model.]" 10: B3 Line contains hard tab characters (\t): "240| if (len - end > 0) {" 11: B3 Line contains hard tab characters (\t): "241| /* Put the remainder of string at the beginning */" 12: B3 Line contains hard tab characters (\t): "242|-> memcpy(ringbuf->buffer, str + end, len - end);" 14: B3 Line contains hard tab characters (\t): "244| if (ringbuf->in_tracing)"
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Bastien Nocera July 5, 2024, 8:57 a.m. UTC
"len - end > 0" can never be false because "end" is unsigned, so the
whole left handside of the expression is unsigned, so always positive.

Error: INTEGER_OVERFLOW (CWE-190): [#def22] [important]
bluez-5.76/src/shared/ringbuf.c:240:2: ineffective_check: The check "len - end > 0UL", which appears to be a guard against integer overflow, is not a useful guard because it is either always true, or never true. This taints "len".
bluez-5.76/src/shared/ringbuf.c:242:3: overflow: The expression "len - end" might be negative, but is used in a context that treats it as unsigned.
bluez-5.76/src/shared/ringbuf.c:242:3: overflow_sink: "len - end", which might be negative, is passed to "memcpy(ringbuf->buffer, str + end, len - end)". [Note: The source code implementation of the function has been overridden by a builtin model.]
240|	if (len - end > 0) {
241|		/* Put the remainder of string at the beginning */
242|->		memcpy(ringbuf->buffer, str + end, len - end);
243|
244|		if (ringbuf->in_tracing)
---
 src/shared/ringbuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/src/shared/ringbuf.c b/src/shared/ringbuf.c
index 3dc7ed71b2b2..1b7adbb4f513 100644
--- a/src/shared/ringbuf.c
+++ b/src/shared/ringbuf.c
@@ -237,7 +237,7 @@  int ringbuf_vprintf(struct ringbuf *ringbuf, const char *format, va_list ap)
 		ringbuf->in_tracing(ringbuf->buffer + offset, end,
 							ringbuf->in_data);
 
-	if (len - end > 0) {
+	if ((size_t) len > end) {
 		/* Put the remainder of string at the beginning */
 		memcpy(ringbuf->buffer, str + end, len - end);