[PATCH v4 0/3] Hardening perf subsystem

Erick Archer
2024-06-01T16:56:49Z

Hi everyone,

This is an effort to get rid of all multiplications from allocation
functions in order to prevent integer overflows [1][2].

In the first patch, the "struct amd_uncore_ctx" can be refactored to
use a flex array for the "events" member. This way, the allocation/
freeing of the memory can be simplified. Then, the struct_size()
helper can be used to do the arithmetic calculation for the memory
to be allocated.

In the second patch, as the "struct intel_uncore_box" ends in a
flexible array, 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_node() function.

In the third patch, as the "struct perf_buffer" also ends in a
flexible array, 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_node() functions. At the same
time, prepare for the coming implementation by GCC and Clang of the
__counted_by attribute.

This time, I have decided to send these three patches in the same serie
since all of them has been rejected by the maintainers. I have used
the v4 tag since it is the latest iteration in one of the patches.

The reason these patches were rejected is that Peter Zijlstra detest
the struct_size() helper [3][4]. However, Kees Cook and I consider that
the use of this helper improves readability. But we can all say that it
is a matter of preference.

Anyway, leaving aside personal preferences, these patches has the
following pros:

- Code robustness improvement (__counted_by coverage).
- Code robustness improvement (use of struct_size() to do calculations
on memory allocator functions).
- Fewer lines of code.
- Follow the recommendations made in "Deprecated Interfaces, Language
Features, Attributes, and Conventions" [5] as the preferred method
of doing things in the kernel.
- Widely used in the kernel.
- Widely accepted in the kernel.

There are also patches in this subsystem that use the struct_size()
helper:

6566f907bf31 ("Convert intel uncore to struct_size") by Matthew Wilcox
dfbc411e0a5e ("perf/x86/rapl: Prefer struct_size() over open coded arithmetic") by me

Therefore, I would like these patches to be applied this time.

Best regards,
Erick

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]
Link: https://lore.kernel.org/linux-hardening/20240430091833.GD40213@noisy.programming.kicks-ass.net [3]
Link: https://lore.kernel.org/linux-hardening/20240430091504.GC40213@noisy.programming.kicks-ass.net [4]
Link: https://docs.kernel.org/process/deprecated.html [5]
---
Changes in v4:

- Add the "Reviewed-by:" tag to the three patches.
- Rebase against linux next (tag next-20240531).

Previous versions:

perf/x86/amd/uncore: Add flex array to struct amd_uncore_ctx
v1 -> https://lore.kernel.org/linux-hardening/AS8PR02MB7237E4848B44A5226BD3551C8BE02@AS8PR02MB7237.eurprd02.prod.outlook.com/

perf/x86/intel/uncore: Prefer struct_size over open coded arithmetic
v1 -> https://lore.kernel.org/linux-hardening/AS8PR02MB7237F4D39BF6AA0FF40E91638B392@AS8PR02MB7237.eurprd02.prod.outlook.com/

perf/ring_buffer: Prefer struct_size over open coded arithmetic
v3 -> https://lore.kernel.org/linux-hardening/AS8PR02MB72379B4807F3951A1B926BA58BE02@AS8PR02MB7237.eurprd02.prod.outlook.com/
v2 -> https://lore.kernel.org/linux-hardening/AS8PR02MB7237569E4FBE0B26F62FDFDB8B1D2@AS8PR02MB7237.eurprd02.prod.outlook.com/
v1 -> https://lore.kernel.org/linux-hardening/AS8PR02MB72372AB065EA8340D960CCC48B1B2@AS8PR02MB7237.eurprd02.prod.outlook.com/

Changes in v3:
- Refactor the logic, compared to the previous version, of the second
rb_alloc() function to gain __counted_by() coverage (Kees Cook and
Christophe JAILLET).

Changes in v2:
- Annotate "struct perf_buffer" with __counted_by() attribute (Kees Cook).
- Refactor the logic to gain __counted_by() coverage (Kees Cook).
---
Erick Archer (3):
perf/x86/amd/uncore: Add flex array to struct amd_uncore_ctx
perf/x86/intel/uncore: Prefer struct_size over open coded arithmetic
perf/ring_buffer: Prefer struct_size over open coded arithmetic

arch/x86/events/amd/uncore.c | 18 +++++-------------
arch/x86/events/intel/uncore.c | 7 +++----
kernel/events/internal.h | 2 +-
kernel/events/ring_buffer.c | 15 ++++-----------
4 files changed, 13 insertions(+), 29 deletions(-)

--
2.25.1