linux-kernel - Re: [PATCH net 5/5] net: ipa: avoid going past end of resource group array

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

Date:   Wed, 28 Oct 2020 09:42:23 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Alex Elder <elder@...aro.org>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, evgreen@...omium.org,
        Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>,
        cpratapa@...eaurora.org, bjorn.andersson@...aro.org,
        Network Development <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net 5/5] net: ipa: avoid going past end of resource group array

> >> +       /* We program at most 6 source or destination resource group limits */
> >> +       BUILD_BUG_ON(IPA_RESOURCE_GROUP_SRC_MAX > 6);
> >> +
> >>         group_count = ipa_resource_group_src_count(ipa->version);
> >> -       if (!group_count)
> >> +       if (!group_count || group_count >= IPA_RESOURCE_GROUP_SRC_MAX)
> >>                 return false;
> >
> > Perhaps more a comment to the previous patch, but _MAX usually denotes
> > the end of an inclusive range, here 5. The previous name COUNT better
> > reflects the number of elements in range [0, 5], which is 6.
>
> I agree with your point, but the max here represents something different
> from what you're expecting.
>
> For a given resource type (source or destination) there is some fixed
> number (count) of resources available based on the version of SoC.
> The *driver* can handle any number of them up to the maximum number
> (max) for any SoC it supports.  In that respect, it *does* represent
> the largest value in an inclusive range.
>
> I could change the suffix to something like SRC_COUNT_MAX, but in
> general the symbol names are longer than I like in this driver and
> I'm trying to shorten them where possible.

Makes sense. Can you then call this out more explicitly in the commit
message? That MAX here means max count, not max element id.