public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: passt-dev@passt.top
Subject: [PATCH 1/3] tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls()
Date: Mon, 27 Feb 2023 10:59:39 +0100	[thread overview]
Message-ID: <20230227095941.225672-2-sbrivio@redhat.com> (raw)
In-Reply-To: <20230227095941.225672-1-sbrivio@redhat.com>

We use the return value of fls() as array index for debug strings.

While fls() can return -1 (if no bit is set), Coverity Scan doesn't
see that we're first checking the return value of another fls() call
with the same bitmask, before using it.

Call fls() once, store its return value, check it, and use the stored
value as array index.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 tcp.c        | 12 ++++++++----
 tcp_splice.c | 24 ++++++++++++++++--------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/tcp.c b/tcp.c
index cbd537e..41210a3 100644
--- a/tcp.c
+++ b/tcp.c
@@ -743,15 +743,19 @@ static void conn_flag_do(const struct ctx *c, struct tcp_tap_conn *conn,
 			 unsigned long flag)
 {
 	if (flag & (flag - 1)) {
+		int flag_index = fls(~flag);
+
 		if (!(conn->flags & ~flag))
 			return;
 
 		conn->flags &= flag;
-		if (fls(~flag) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP: index %li: %s dropped", CONN_IDX(conn),
-			      tcp_flag_str[fls(~flag)]);
+			      tcp_flag_str[flag_index]);
 		}
 	} else {
+		int flag_index = fls(~flag);
+
 		if (conn->flags & flag) {
 			/* Special case: setting ACK_FROM_TAP_DUE on a
 			 * connection where it's already set is used to
@@ -766,9 +770,9 @@ static void conn_flag_do(const struct ctx *c, struct tcp_tap_conn *conn,
 		}
 
 		conn->flags |= flag;
-		if (fls(flag) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP: index %li: %s", CONN_IDX(conn),
-			      tcp_flag_str[fls(flag)]);
+			      tcp_flag_str[flag_index]);
 		}
 	}
 
diff --git a/tcp_splice.c b/tcp_splice.c
index 84f855e..67af46b 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -127,22 +127,26 @@ static void conn_flag_do(const struct ctx *c, struct tcp_splice_conn *conn,
 			 unsigned long flag)
 {
 	if (flag & (flag - 1)) {
+		int flag_index = fls(~flag);
+
 		if (!(conn->flags & ~flag))
 			return;
 
 		conn->flags &= flag;
-		if (fls(~flag) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP (spliced): index %li: %s dropped", CONN_IDX(conn),
-			      tcp_splice_flag_str[fls(~flag)]);
+			      tcp_splice_flag_str[flag_index]);
 		}
 	} else {
+		int flag_index = fls(flag);
+
 		if (conn->flags & flag)
 			return;
 
 		conn->flags |= flag;
-		if (fls(flag) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP (spliced): index %li: %s", CONN_IDX(conn),
-			      tcp_splice_flag_str[fls(flag)]);
+			      tcp_splice_flag_str[flag_index]);
 		}
 	}
 
@@ -207,22 +211,26 @@ static void conn_event_do(const struct ctx *c, struct tcp_splice_conn *conn,
 			  unsigned long event)
 {
 	if (event & (event - 1)) {
+		int flag_index = fls(~event);
+
 		if (!(conn->events & ~event))
 			return;
 
 		conn->events &= event;
-		if (fls(~event) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP (spliced): index %li, ~%s", CONN_IDX(conn),
-			      tcp_splice_event_str[fls(~event)]);
+			      tcp_splice_event_str[flag_index]);
 		}
 	} else {
+		int flag_index = fls(event);
+
 		if (conn->events & event)
 			return;
 
 		conn->events |= event;
-		if (fls(event) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP (spliced): index %li, %s", CONN_IDX(conn),
-			      tcp_splice_event_str[fls(event)]);
+			      tcp_splice_event_str[flag_index]);
 		}
 	}
 
-- 
@@ -127,22 +127,26 @@ static void conn_flag_do(const struct ctx *c, struct tcp_splice_conn *conn,
 			 unsigned long flag)
 {
 	if (flag & (flag - 1)) {
+		int flag_index = fls(~flag);
+
 		if (!(conn->flags & ~flag))
 			return;
 
 		conn->flags &= flag;
-		if (fls(~flag) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP (spliced): index %li: %s dropped", CONN_IDX(conn),
-			      tcp_splice_flag_str[fls(~flag)]);
+			      tcp_splice_flag_str[flag_index]);
 		}
 	} else {
+		int flag_index = fls(flag);
+
 		if (conn->flags & flag)
 			return;
 
 		conn->flags |= flag;
-		if (fls(flag) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP (spliced): index %li: %s", CONN_IDX(conn),
-			      tcp_splice_flag_str[fls(flag)]);
+			      tcp_splice_flag_str[flag_index]);
 		}
 	}
 
@@ -207,22 +211,26 @@ static void conn_event_do(const struct ctx *c, struct tcp_splice_conn *conn,
 			  unsigned long event)
 {
 	if (event & (event - 1)) {
+		int flag_index = fls(~event);
+
 		if (!(conn->events & ~event))
 			return;
 
 		conn->events &= event;
-		if (fls(~event) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP (spliced): index %li, ~%s", CONN_IDX(conn),
-			      tcp_splice_event_str[fls(~event)]);
+			      tcp_splice_event_str[flag_index]);
 		}
 	} else {
+		int flag_index = fls(event);
+
 		if (conn->events & event)
 			return;
 
 		conn->events |= event;
-		if (fls(event) >= 0) {
+		if (flag_index >= 0) {
 			debug("TCP (spliced): index %li, %s", CONN_IDX(conn),
-			      tcp_splice_event_str[fls(event)]);
+			      tcp_splice_event_str[flag_index]);
 		}
 	}
 
-- 
2.39.1


  reply	other threads:[~2023-02-27  9:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-27  9:59 [PATCH 0/3] Avoid some warnings reported by Coverity Stefano Brivio
2023-02-27  9:59 ` Stefano Brivio [this message]
2023-02-27 10:49   ` [PATCH 1/3] tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls() David Gibson
2023-02-27  9:59 ` [PATCH 2/3] tcp: Avoid false (but convoluted) positive Coverity CWE-476 warning Stefano Brivio
2023-02-27 10:50   ` David Gibson
2023-02-27  9:59 ` [PATCH 3/3] tcp: Avoid (theoretical) resource leak (CWE-772) Coverity warning Stefano Brivio
2023-02-27 10:51   ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230227095941.225672-2-sbrivio@redhat.com \
    --to=sbrivio@redhat.com \
    --cc=passt-dev@passt.top \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).