From: James Jones <jejones3141@gmail.com>
Date: Tue, 13 Aug 2024 21:11:41 +0000 (-0500)
Subject: Annotate overflow issues in lo_read() (CID #1604601)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=604f6d30c91b149d7c5a5d0456f2ad494ca8575e;p=thirdparty%2Ffreeradius-server.git

Annotate overflow issues in lo_read() (CID #1604601)

Coverity sets itself up in a vicious cycle:
1. It considers the loop check expression to be tainted because
   total is tainted, so outlen is tainted, too.
2. Because of that, outlen - total (passed to read()) is deemed
   overflowed, so the return value r is considered overflowed.
3. Returning total, which is considered overflowed, is another issue.
4. r, which is considered overflowed, is added to r--which is why
   total considered to have overflowed and hence be tainted.

Once we changed the code to not add r to total in the EINTR case,
one can, but Coverity cannot, infer that total will only take on
values in {0,1,...,outlen}, and since both have the same type, total
can represent all such values. read(), as a standard function, is
one it should have a model for, but it doesn't seem to include the
property that the returned value is less than or equal to the passed
number of bytes to read(), and it doesn't have a way to let us
represent it in a custom model.
---

diff --git a/src/listen/control/conduit.c b/src/listen/control/conduit.c
index f23ef7c96b9..c53a909e894 100644
--- a/src/listen/control/conduit.c
+++ b/src/listen/control/conduit.c
@@ -39,8 +39,10 @@ static ssize_t lo_read(int fd, void *out, size_t outlen)
 	uint8_t *p = out;
 
 	for (total = 0; total < outlen; ) {
+		/* coverity[overflow_sink] */
 		r = read(fd, p + total, outlen - total);
 
+		/* coverity[return_overflow] */
 		if (r == 0) return total;
 
 		if (r < 0) {