Re: [PATCH] perf/x86/rapl: Prefer struct_size over open coded arithmetic

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 18/03/24 17:40, Kees Cook wrote:
> On Sun, Mar 17, 2024 at 05:44:42PM +0100, Erick Archer wrote:
>> This is an effort to get rid of all multiplications from allocation
>> functions in order to prevent integer overflows [1][2].
>>
>> As the "rapl_pmus" variable is a pointer to "struct rapl_pmus" and
>> this structure ends in a flexible array:
>>
>> struct rapl_pmus {
>> 	[...]
>> 	struct rapl_pmu *pmus[] __counted_by(maxdie);
>> };
>>
>> the preferred way in the kernel is to use the struct_size() helper to
>> do the arithmetic instead of the calculation "size + count * size" in
>> the kzalloc() function.
>>
>> This way, the code is more readable and safer.
>>
>> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [1]
>> Link: https://github.com/KSPP/linux/issues/160 [2]
>> Signed-off-by: Erick Archer <erick.archer@gmx.com>
> 
> Thanks!
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> I was inspired to come up with a Coccinelle script to find this pattern.
> This seems to do it, though it also removes the blank line. I'm not sure
> how to stop it from doing that. I'm running this treewide to see if I
> can find others...
> 
> // Options: --no-includes --include-headers

with --no-includes this one is missed in arch/x86/events/:

diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
index 258e2cdf28fa..213fe48f9391 100644
--- a/arch/x86/events/intel/uncore.c
+++ b/arch/x86/events/intel/uncore.c
@@ -350,12 +350,11 @@ static void uncore_pmu_init_hrtimer(struct intel_uncore_box *box)
  static struct intel_uncore_box *uncore_alloc_box(struct intel_uncore_type *type,
                                                  int node)
  {
-       int i, size, numshared = type->num_shared_regs ;
+       int i, numshared = type->num_shared_regs ;
         struct intel_uncore_box *box;

-       size = sizeof(*box) + numshared * sizeof(struct intel_uncore_extra_reg);
-
-       box = kzalloc_node(size, GFP_KERNEL, node);
+       box = kzalloc_node(struct_size(box, shared_regs, numshared),
+                          GFP_KERNEL, node);
         if (!box)
                 return NULL;

Funny thing is that in this case it's quite convenient that the script removes
the blank like. :P

--
Gustavo