From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id A70315A0268 for ; Mon, 27 Feb 2023 14:29:57 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4PQLvg52kWz4x91; Tue, 28 Feb 2023 00:29:51 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1677504591; bh=eEiQtL5Sw6rXHlGL+D1FTMvuEFL54+4Tx+4boHfu5lI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=gAsD/eKKWzte9EoZca3Ql8KdYJ6Uapm+oPglGx6Bcriz2fN/UdrQPy2kX0rv6Uft2 zQjEvvyESANiVMCjFy5FmoUUXWlAUlRGOE2qTDuRixdSgwIYO1qAt/gdrCZC9UFoZ6 1XUq5LGvzor8gh8tJvvgor1LYbmtEoJCXM7eM7kE= Date: Mon, 27 Feb 2023 21:49:09 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/3] tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls() Message-ID: References: <20230227095941.225672-1-sbrivio@redhat.com> <20230227095941.225672-2-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="HyUzEJCYm+HYFJ1/" Content-Disposition: inline In-Reply-To: <20230227095941.225672-2-sbrivio@redhat.com> Message-ID-Hash: M4N3BYYFWWOLYYWKGE3WNPTNAG3PJSMN X-Message-ID-Hash: M4N3BYYFWWOLYYWKGE3WNPTNAG3PJSMN X-MailFrom: dgibson@gandalf.ozlabs.org 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 CC: passt-dev@passt.top 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: --HyUzEJCYm+HYFJ1/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 27, 2023 at 10:59:39AM +0100, Stefano Brivio wrote: > We use the return value of fls() as array index for debug strings. >=20 > 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. >=20 > Call fls() once, store its return value, check it, and use the stored > value as array index. >=20 > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > tcp.c | 12 ++++++++---- > tcp_splice.c | 24 ++++++++++++++++-------- > 2 files changed, 24 insertions(+), 12 deletions(-) >=20 > 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, struc= t tcp_tap_conn *conn, > unsigned long flag) > { > if (flag & (flag - 1)) { > + int flag_index =3D fls(~flag); > + > if (!(conn->flags & ~flag)) > return; > =20 > conn->flags &=3D flag; > - if (fls(~flag) >=3D 0) { > + if (flag_index >=3D 0) { > debug("TCP: index %li: %s dropped", CONN_IDX(conn), > - tcp_flag_str[fls(~flag)]); > + tcp_flag_str[flag_index]); > } > } else { > + int flag_index =3D 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, > } > =20 > conn->flags |=3D flag; > - if (fls(flag) >=3D 0) { > + if (flag_index >=3D 0) { > debug("TCP: index %li: %s", CONN_IDX(conn), > - tcp_flag_str[fls(flag)]); > + tcp_flag_str[flag_index]); > } > } > =20 > 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, struc= t tcp_splice_conn *conn, > unsigned long flag) > { > if (flag & (flag - 1)) { > + int flag_index =3D fls(~flag); > + > if (!(conn->flags & ~flag)) > return; > =20 > conn->flags &=3D flag; > - if (fls(~flag) >=3D 0) { > + if (flag_index >=3D 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 =3D fls(flag); > + > if (conn->flags & flag) > return; > =20 > conn->flags |=3D flag; > - if (fls(flag) >=3D 0) { > + if (flag_index >=3D 0) { > debug("TCP (spliced): index %li: %s", CONN_IDX(conn), > - tcp_splice_flag_str[fls(flag)]); > + tcp_splice_flag_str[flag_index]); > } > } > =20 > @@ -207,22 +211,26 @@ static void conn_event_do(const struct ctx *c, stru= ct tcp_splice_conn *conn, > unsigned long event) > { > if (event & (event - 1)) { > + int flag_index =3D fls(~event); > + > if (!(conn->events & ~event)) > return; > =20 > conn->events &=3D event; > - if (fls(~event) >=3D 0) { > + if (flag_index >=3D 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 =3D fls(event); > + > if (conn->events & event) > return; > =20 > conn->events |=3D event; > - if (fls(event) >=3D 0) { > + if (flag_index >=3D 0) { > debug("TCP (spliced): index %li, %s", CONN_IDX(conn), > - tcp_splice_event_str[fls(event)]); > + tcp_splice_event_str[flag_index]); > } > } > =20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --HyUzEJCYm+HYFJ1/ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmP8ip8ACgkQzQJF27ox 2GfIOA/8DHnoMIzX67kEDIoZ3AmRxrgYD87rMb4apzFDqfbn4JUGQwNpMcgBt7vL XCg2lgCb5vSZIwj+EXYo9q6FRyXfpqZPwSw9GrextV2FJgvF+eqB2rRLb2XW2EW6 LVpKLkhXruW2WXb8USBgSkYUsya9Z3OEKZMPK9Y3Dl7LOYpcv7xGtqDTlY1Y4u8o vmwWYA9f4iLKbPcL3FUlpCVWV5hfAtUfkvSQiI/98XgONzCdzmFyrAgoSXuzOk76 E+SRTlr1/wjK1lMI9fq40kattyMPsYVbQM9HAgJzlNJUcEoCe0lq3jlJ25iXh60Y U33uLM0f004xVnliSEgUgwnYQMwUkcG8IBKD4fxtgPCmuxORe9CogebkM7Gx096F uPYAuuYQ/T24S/udyIoWonGZfR3uk06Imw8aggDYZrPyzE97ecOtFFU2u1xfNasF KurRZjnWydh0uOZ/rUvu5eCeV3HgaVUpLhSKwAHNhRpz3YM9mOf3XSJnCVvFWbRd dFMj+ILs9DnIYAQZiTrJrtTyuBVbKQc/GF8MF+NkI+JXwVtEVexaX7OSZrn/3iS8 iSMJOcv95xlKOuVeUWEBt2hkVugnLpGIbsvR0kpwIvlim9575DNncoLIJIkJMMEO AwBQ/LCUas/5QqSUKe1zPEHniY3iR9FItuB9LtJpl81VgICCLbg= =a6Hy -----END PGP SIGNATURE----- --HyUzEJCYm+HYFJ1/--