| 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
| ||
|
Message-ID: <CA+FuTSdGCBG0tZXfPTJqTnV7zRNv2VmuThOydwj080NWw4PU9Q@mail.gmail.com>
Date: Tue, 27 Oct 2020 20:14:51 -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
On Tue, Oct 27, 2020 at 12:38 PM Alex Elder <elder@...aro.org> wrote:
>
> The minimum and maximum limits for resources assigned to a given
> resource group are programmed in pairs, with the limits for two
> groups set in a single register.
>
> If the number of supported resource groups is odd, only half of the
> register that defines these limits is valid for the last group; that
> group has no second group in the pair.
>
> Currently we ignore this constraint, and it turns out to be harmless,
If nothing currently calls it with an odd number of registers, is this
a bugfix or a new feature (anticipating future expansion, I guess)?
> but it is not guaranteed to be. This patch addresses that, and adds
> support for programming the 5th resource group's limits.
>
> Rework how the resource group limit registers are programmed by
> having a single function program all group pairs rather than having
> one function program each pair. Add the programming of the 4-5
> resource group pair limits to this function. If a resource group is
> not supported, pass a null pointer to ipa_resource_config_common()
> for that group and have that function write zeroes in that case.
>
> Fixes: cdf2e9419dd91 ("soc: qcom: ipa: main code")
> Signed-off-by: Alex Elder <elder@...aro.org>
> ---
> drivers/net/ipa/ipa_main.c | 89 +++++++++++++++++++++++---------------
> 1 file changed, 53 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
> index 74b1e15ebd6b2..09c8a16d216df 100644
> --- a/drivers/net/ipa/ipa_main.c
> +++ b/drivers/net/ipa/ipa_main.c
> @@ -370,8 +370,11 @@ static bool ipa_resource_limits_valid(struct ipa *ipa,
> u32 i;
> u32 j;
>
> + /* 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.
> /* Return an error if a non-zero resource limit is specified
> @@ -387,7 +390,7 @@ static bool ipa_resource_limits_valid(struct ipa *ipa,
> }
>
> group_count = ipa_resource_group_dst_count(ipa->version);
> - if (!group_count)
> + if (!group_count || group_count >= IPA_RESOURCE_GROUP_DST_MAX)
> return false;
>
> for (i = 0; i < data->resource_dst_count; i++) {
> @@ -421,46 +424,64 @@ ipa_resource_config_common(struct ipa *ipa, u32 offset,
>
> val = u32_encode_bits(xlimits->min, X_MIN_LIM_FMASK);
> val |= u32_encode_bits(xlimits->max, X_MAX_LIM_FMASK);
> - val |= u32_encode_bits(ylimits->min, Y_MIN_LIM_FMASK);
> - val |= u32_encode_bits(ylimits->max, Y_MAX_LIM_FMASK);
> + if (ylimits) {
> + val |= u32_encode_bits(ylimits->min, Y_MIN_LIM_FMASK);
> + val |= u32_encode_bits(ylimits->max, Y_MAX_LIM_FMASK);
> + }
>
> iowrite32(val, ipa->reg_virt + offset);
> }
>
> -static void ipa_resource_config_src_01(struct ipa *ipa,
> - const struct ipa_resource_src *resource)
> +static void ipa_resource_config_src(struct ipa *ipa,
> + const struct ipa_resource_src *resource)
> {
> - u32 offset = IPA_REG_SRC_RSRC_GRP_01_RSRC_TYPE_N_OFFSET(resource->type);
> + u32 group_count = ipa_resource_group_src_count(ipa->version);
> + const struct ipa_resource_limits *ylimits;
> + u32 offset;
>
> - ipa_resource_config_common(ipa, offset,
> - &resource->limits[0], &resource->limits[1]);
> -}
> + offset = IPA_REG_SRC_RSRC_GRP_01_RSRC_TYPE_N_OFFSET(resource->type);
> + ylimits = group_count == 1 ? NULL : &resource->limits[1];
> + ipa_resource_config_common(ipa, offset, &resource->limits[0], ylimits);
>
> -static void ipa_resource_config_src_23(struct ipa *ipa,
> - const struct ipa_resource_src *resource)
> -{
> - u32 offset = IPA_REG_SRC_RSRC_GRP_23_RSRC_TYPE_N_OFFSET(resource->type);
> + if (group_count < 2)
> + return;
>
> - ipa_resource_config_common(ipa, offset,
> - &resource->limits[2], &resource->limits[3]);
> -}
> + offset = IPA_REG_SRC_RSRC_GRP_23_RSRC_TYPE_N_OFFSET(resource->type);
> + ylimits = group_count == 3 ? NULL : &resource->limits[3];
> + ipa_resource_config_common(ipa, offset, &resource->limits[2], ylimits);
>
> -static void ipa_resource_config_dst_01(struct ipa *ipa,
> - const struct ipa_resource_dst *resource)
> -{
> - u32 offset = IPA_REG_DST_RSRC_GRP_01_RSRC_TYPE_N_OFFSET(resource->type);
> + if (group_count < 4)
> + return;
>
> - ipa_resource_config_common(ipa, offset,
> - &resource->limits[0], &resource->limits[1]);
> + offset = IPA_REG_SRC_RSRC_GRP_45_RSRC_TYPE_N_OFFSET(resource->type);
> + ylimits = group_count == 5 ? NULL : &resource->limits[5];
Due to the check
> + if (!group_count || group_count >= IPA_RESOURCE_GROUP_DST_MAX)
> return false;
above, group_count can never be greater than 5. Should be greater than?
Powered by blists - more mailing lists