linux-kernel - Re: [PATCH net-next 4/4] net: ipa: activate some commented assertions

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]

Date:   Fri, 19 Mar 2021 10:32:50 -0500
From:   Alex Elder <elder@...aro.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     davem@...emloft.net, kuba@...nel.org, bjorn.andersson@...aro.org,
        evgreen@...omium.org, cpratapa@...eaurora.org, elder@...nel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 4/4] net: ipa: activate some commented assertions

On 3/19/21 10:17 AM, Leon Romanovsky wrote:
>>>> @@ -212,7 +213,7 @@ static inline u32 ipa_reg_bcr_val(enum ipa_version version)
>>>>    			BCR_HOLB_DROP_L2_IRQ_FMASK |
>>>>    			BCR_DUAL_TX_FMASK;
>>>> -	/* assert(version != IPA_VERSION_4_5); */
>>>> +	ipa_assert(NULL, version != IPA_VERSION_4_5);
>>> This assert will fire for IPA_VERSION_4_2, I doubt that this is
>>> something you want.
>> No, it will only fail if version == IPA_VERSION_4_5.
>> The logic of an assertion is the opposite of BUG_ON().
>> It fails only if the asserted condition yields false.
> ok, this ipa_reg_bcr_val() is called in only one place and has a
> protection from IPA_VERSION_4_5, why don't you code it at the same
> .c file instead of adding useless assert?

As I mentioned in my other message, the purpose of an
assertion is *communicating with the reader*.  The
fact that an assertion may expand to code that ensures
the assertion is true is secondary.

This particular assertion says that the version will never
be 4.5.  While looking at this function, you don't need to
see if the caller ensures that, the assertion *tells* you.

Whether an assertion is warranted is really subjective.
You may not appreciate the value of that, but I do.

					-Alex