From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by passt.top (Postfix, from userid 1000) id A82635A026B; Mon, 27 Feb 2023 10:59:41 +0100 (CET) From: Stefano Brivio 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 Message-Id: <20230227095941.225672-2-sbrivio@redhat.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230227095941.225672-1-sbrivio@redhat.com> References: <20230227095941.225672-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: SALHX4ESXK4FC4BQXN2WBJ6FFAAVPQYA X-Message-ID-Hash: SALHX4ESXK4FC4BQXN2WBJ6FFAAVPQYA X-MailFrom: sbrivio@passt.top X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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]); } } -- 2.39.1