linux-kernel - Re: [PATCH net v2] ice: use proper macro for testing bit

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <da984106-43eb-42cc-a8c0-be859c6e84e9@intel.com>
Date: Mon, 17 Jun 2024 14:58:59 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Simon Horman <horms@...nel.org>, Petr Oros <poros@...hat.com>
CC: <netdev@...r.kernel.org>, <ivecera@...hat.com>,
	<przemyslaw.kitszel@...el.com>, Jesse Brandeburg
	<jesse.brandeburg@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.com>,
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "Marcin
 Szycik" <marcin.szycik@...ux.intel.com>, Konrad Knitter
	<konrad.knitter@...el.com>, Marcin Domagala <marcinx.domagala@...el.com>,
	"moderated list:INTEL ETHERNET DRIVERS" <intel-wired-lan@...ts.osuosl.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net v2] ice: use proper macro for testing bit

From: Simon Horman <horms@...nel.org>
Date: Sat, 15 Jun 2024 16:16:41 +0100

> On Fri, Jun 14, 2024 at 11:43:38AM +0200, Petr Oros wrote:
>> Do not use _test_bit() macro for testing bit. The proper macro for this
>> is one without underline.
> 
> Hi Petr,
> 
> it might be nice to include a brief explanation as to
> why test_bit() is correct.

Let me explain this as the author of all those bitops wrappers :D
Petr is free to include either this or his own brief into v2.

_test_bit() is what test_bit() was prior to my const-optimization. It
directly calls arch_test_bit(), i.e. the arch-specific implementation
(or the generic one). It's strictly _internal_ and shouldn't be used
anywhere outside the actual test_bit() macro.

test_bit() is a wrapper which checks whether the bitmap and the bit
number are compile-time constants and if so, it calls the optimized
function which evaluates this call to a compile-time constant as well.
If either of them is not a compile-time constant, it just calls _test_bit().
test_bit() is the actual function to use anywhere in the kernel.

IOW, calling _test_bit() avoids potential compile-time optimizations.

>From what I see in the code, &sensors is not a compile-time constant,
thus most probably there are no object code changes before and after
the patch. But anyway, we shouldn't call internal wrappers instead of
the actual API, so this fix is correct.

> 
>>
>> Fixes: 4da71a77fc3b ("ice: read internal temperature sensor")
>> Signed-off-by: Petr Oros <poros@...hat.com>
>> Acked-by: Ivan Vecera <ivecera@...hat.com>

To be added to v2:

Reviewed-by: Alexander Lobakin <aleksander.lobakin@...el.com>

Thanks,
Olek