* Erick Archer <[email protected]> wrote:
> As noted in the "Deprecated Interfaces, Language Features, Attributes, > and Conventions" documentation [1], size calculations (especially > multiplication) should not be performed in memory allocator (or similar) > function arguments due to the risk of them overflowing. This could lead > to values wrapping around and a smaller allocation being made than the > caller was expecting. Using those allocations could lead to linear > overflows of heap memory and other misbehaviors. > > So, use the purpose specific kcalloc*() function instead of the argument > size * count in the kzalloc*() function. > > [1] > https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments > > Link: https://github.com/KSPP/linux/issues/162 > Reviewed-by: Gustavo A. R. Silva <[email protected]> > Signed-off-by: Erick Archer <[email protected]> > --- > Changes in v2: > - Add the "Reviewed-by:" tag. > - Rebase against linux-next. > > Previous versions: > v1 -> > https://lore.kernel.org/linux-hardening/[email protected]/ > > Hi everyone, > > This patch seems to be lost. Gustavo reviewed it on January 16, 2024 > but the patch has not been applied since. > > Thanks, > Erick > --- > arch/x86/events/amd/uncore.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c > index 4ccb8fa483e6..61c0a2114183 100644 > --- a/arch/x86/events/amd/uncore.c > +++ b/arch/x86/events/amd/uncore.c > @@ -479,8 +479,8 @@ static int amd_uncore_ctx_init(struct amd_uncore *uncore, > unsigned int cpu) > goto fail; > > curr->cpu = cpu; > - curr->events = kzalloc_node(sizeof(*curr->events) * > - pmu->num_counters, > + curr->events = kcalloc_node(pmu->num_counters, > + sizeof(*curr->events), > GFP_KERNEL, node); > if (!curr->events) { > kfree(curr); > @@ -928,7 +928,7 @@ int amd_uncore_umc_ctx_init(struct amd_uncore *uncore, > unsigned int cpu) > uncore->num_pmus += group_num_pmus[gid]; > } > > - uncore->pmus = kzalloc(sizeof(*uncore->pmus) * uncore->num_pmus, > + uncore->pmus = kcalloc(uncore->num_pmus, sizeof(*uncore->pmus), > GFP_KERNEL); > if (!uncore->pmus) { > uncore->num_pmus = 0; This change is nonsense, kzalloc() is a perfectly usable interface, and none of the arguments are user-controlled, so I don't see how there could be a real overflow bug here. Thanks, Ingo
