netdev - [bbr3] Suspicious use of bbr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <4616579.LvFx2qVVIh@natalenko.name>
Date: Tue, 24 Dec 2024 22:04:53 +0100
From: Oleksandr Natalenko <oleksandr@...alenko.name>
To: Neal Cardwell <ncardwell@...gle.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [bbr3] Suspicious use of bbr_param

Hello Neal.

One of my users reports [1] that BBRv3 from [2] cannot be built with LLVM=1 and WERROR=y because of the following warnings:

net/ipv4/tcp_bbr.c:1079:48: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
 1079 |         if (!bbr->ecn_eligible && bbr_can_use_ecn(sk) &&
      |                                                       ^
 1080 |             bbr_param(sk, ecn_factor) &&
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1079:48: note: use '&' for a bitwise operation
 1079 |         if (!bbr->ecn_eligible && bbr_can_use_ecn(sk) &&
      |                                                       ^~
      |                                                       &
net/ipv4/tcp_bbr.c:1079:48: note: remove constant to silence this warning
 1079 |         if (!bbr->ecn_eligible && bbr_can_use_ecn(sk) &&
      |                                                       ^~
 1080 |             bbr_param(sk, ecn_factor) &&
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1187:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
 1187 |             bbr->ecn_eligible && bbr_param(sk, ecn_thresh)) {
      |                               ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1187:24: note: use '&' for a bitwise operation
 1187 |             bbr->ecn_eligible && bbr_param(sk, ecn_thresh)) {
      |                               ^~
      |                               &
net/ipv4/tcp_bbr.c:1187:24: note: remove constant to silence this warning
 1187 |             bbr->ecn_eligible && bbr_param(sk, ecn_thresh)) {
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1385:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
 1385 |         if (bbr->ecn_in_round && bbr_param(sk, ecn_factor)) {
      |                               ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1385:24: note: use '&' for a bitwise operation
 1385 |         if (bbr->ecn_in_round && bbr_param(sk, ecn_factor)) {
      |                               ^~
      |                               &
net/ipv4/tcp_bbr.c:1385:24: note: remove constant to silence this warning
 1385 |         if (bbr->ecn_in_round && bbr_param(sk, ecn_factor)) {
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.

The usage of bbr_param() with ecn_thresh and ecn_factor here does indeed look suspicious. In both cases, the bbr_param() macro gets evaluated to `static const u32` values, and those get &&'ed in the if statements. The consts are positive, so they do not have any impact in the conditional expressions. FWIW, the sk argument is dropped by the macro altogether, so I'm not sure what was the intention here.

Interestingly, unlike Clang, GCC stays silent.

Could you please comment on this?

Appreciate your time, and looking forward to your reply.

Thank you.

[1] https://codeberg.org/pf-kernel/linux/issues/11
[2] https://github.com/google/bbr

-- 
Oleksandr Natalenko, MSE
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)