linux.git/tools/testing/selftests/resctrl/resctrl_val.c, branch v6.12.80

selftests/resctrl: Protect against array overflow when reading strings

2024-12-14T19:03:41+00:00

[ Upstream commit 46058430fc5d39c114f7e1b9c6ff14c9f41bd531 ]

resctrl selftests discover system properties via a variety of sysfs files.
The MBM and MBA tests need to discover the event and umask with which to
configure the performance event used to measure read memory bandwidth.
This is done by parsing the contents of
/sys/bus/event_source/devices/uncore_imc_/events/cas_count_read
Similarly, the resctrl selftests discover the cache size via
/sys/bus/cpu/devices/cpu/cache/index/size.

Take care to do bounds checking when using fscanf() to read the
contents of files into a string buffer because by default fscanf() assumes
arbitrarily long strings. If the file contains more bytes than the array
can accommodate then an overflow will occur.

Provide a maximum field width to the conversion specifier to protect
against array overflow. The maximum is one less than the array size because
string input stores a terminating null byte that is not covered by the
maximum field width.

Signed-off-by: Reinette Chatre 
Reviewed-by: Ilpo Järvinen 
Signed-off-by: Shuah Khan 
Signed-off-by: Sasha Levin

selftests/resctrl: Protect against array overrun during iMC config parsing

2024-12-05T13:01:33+00:00

[ Upstream commit 48ed4e799e8fbebae838dca404a8527763d41191 ]

The MBM and MBA tests need to discover the event and umask with which to
configure the performance event used to measure read memory bandwidth.
This is done by parsing the
/sys/bus/event_source/devices/uncore_imc_/events/cas_count_read
file for each iMC instance that contains the formatted
output: "event=,umask="

Parsing of cas_count_read contents is done by initializing an array of
MAX_TOKENS elements with tokens (deliminated by "=,") from this file.
Remove the unnecessary append of a delimiter to the string needing to be
parsed. Per the strtok() man page: "delimiter bytes at the start or end of
the string are ignored". This has no impact on the token placement within
the array.

After initialization, the actual event and umask is determined by
parsing the tokens directly following the "event" and "umask" tokens
respectively.

Iterating through the array up to index "i < MAX_TOKENS" but then
accessing index "i + 1" risks array overrun during the final iteration.
Avoid array overrun by ensuring that the index used within for
loop will always be valid.

Fixes: 1d3f08687d76 ("selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system")
Signed-off-by: Reinette Chatre 
Reviewed-by: Ilpo Järvinen 
Signed-off-by: Shuah Khan 
Signed-off-by: Sasha Levin

selftests/resctrl: Remove test name comparing from write_bm_pid_to_resctrl()

2024-07-11T17:23:54+00:00

write_bm_pid_to_resctrl() uses resctrl_val to check test name which is
not a good interface generic resctrl FS functions should provide.

Tests define mongrp when needed. Remove the test name check in
write_bm_pid_to_resctrl() to only rely on the mongrp parameter being
non-NULL.

Remove write_bm_pid_to_resctrl() resctrl_val parameter and resctrl_val
member from the struct resctrl_val_param that are not used anymore.
Similarly, remove the test name constants that are no longer used.

Signed-off-by: Ilpo Järvinen 
Tested-by: Babu Moger 
Reviewed-by: Reinette Chatre 
Signed-off-by: Shuah Khan

selftests/resctrl: Simplify bandwidth report type handling

2024-07-11T17:23:54+00:00

bw_report is only needed for selecting the correct value from the
values IMC measured. It is a member in the resctrl_val_param struct and
is always set to "reads". The value is then checked in resctrl_val()
using validate_bw_report_request() that besides validating the input,
assumes it can mutate the string which is questionable programming
practice.

Simplify handling bw_report:

- Convert validate_bw_report_request() into get_bw_report_type() that
  inputs and returns const char *. Use NULL to indicate error.

- Validate the report types inside measure_mem_bw(), not in
  resctrl_val().

- Pass bw_report to measure_mem_bw() from ->measure() hook because
  resctrl_val() no longer needs bw_report for anything.

Signed-off-by: Ilpo Järvinen 
Tested-by: Babu Moger 
Reviewed-by: Reinette Chatre 
Signed-off-by: Shuah Khan

selftests/resctrl: Add ->init() callback into resctrl_val_param

2024-07-11T17:23:54+00:00

The struct resctrl_val_param is there to customize behavior inside
resctrl_val() which is currently not used to full extent and there are
number of strcmp()s for test name in resctrl_val done by resctrl_val().

Create ->init() hook into the struct resctrl_val_param to cleanly
do per test initialization.

Remove also unused branches to setup paths and the related #defines
for CMT test.

While touching kerneldoc, make the adjacent line consistent with the
newly added form (callback vs call back).

Signed-off-by: Ilpo Järvinen 
Tested-by: Babu Moger 
Reviewed-by: Reinette Chatre 
Signed-off-by: Shuah Khan

selftests/resctrl: Add ->measure() callback to resctrl_val_param

2024-07-11T17:23:54+00:00

The measurement done in resctrl_val() varies depending on test type.
The decision for how to measure is decided based on the string compare
to test name which is quite inflexible.

Add ->measure() callback into the struct resctrl_val_param to allow
each test to provide necessary code as a function which simplifies what
resctrl_val() has to do.

Signed-off-by: Ilpo Järvinen 
Tested-by: Babu Moger 
Reviewed-by: Reinette Chatre 
Signed-off-by: Shuah Khan

selftests/resctrl: Simplify mem bandwidth file code for MBA & MBM tests

2024-07-11T17:23:54+00:00

initialize_mem_bw_resctrl() and set_mbm_path() contain complicated set
of conditions, each yielding different file to be opened to measure
memory bandwidth through resctrl FS. In practice, only two of them are
used. For MBA test, ctrlgrp is always provided, and for MBM test both
ctrlgrp and mongrp are set.

The file used differ between MBA/MBM test, however, MBM test
unnecessarily create monitor group because resctrl FS already provides
monitoring interface underneath any ctrlgrp too, which is what the MBA
selftest uses.

Consolidate memory bandwidth file used to the one used by the MBA
selftest. Remove all unused branches opening other files to simplify
the code.

Suggested-by: Reinette Chatre 
Signed-off-by: Ilpo Järvinen 
Tested-by: Babu Moger 
Reviewed-by: Reinette Chatre 
Signed-off-by: Shuah Khan

selftests/resctrl: Rename measure_vals() to measure_mem_bw_vals() & document

2024-07-11T17:23:54+00:00

measure_vals() is awfully generic name so rename it to measure_mem_bw()
to describe better what it does and document the function parameters.

Signed-off-by: Ilpo Järvinen 
Tested-by: Babu Moger 
Reviewed-by: Reinette Chatre 
Signed-off-by: Shuah Khan

selftests/resctrl: Cleanup bm_pid and ppid usage & limit scope

2024-07-11T17:23:54+00:00

'bm_pid' and 'ppid' are global variables. As they are used by different
processes and in signal handler, they cannot be entirely converted into
local variables.

The scope of those variables can still be reduced into resctrl_val.c
only. As PARENT_EXIT() macro is using 'ppid', make it a function in
resctrl_val.c and pass ppid to it as an argument because it is easier
to understand than using the global variable directly.

Pass 'bm_pid' into measure_vals() instead of relying on the global
variable which helps to make the call signatures of measure_vals() and
measure_llc_resctrl() more similar to each other.

Signed-off-by: Ilpo Järvinen 
Tested-by: Babu Moger 
Reviewed-by: Reinette Chatre 
Signed-off-by: Shuah Khan

selftests/resctrl: Use correct type for pids

2024-07-11T17:23:53+00:00

A few functions receive PIDs through int arguments. PIDs variables
should be of type pid_t, not int.

Convert pid arguments from int to pid_t.

Before printing PID, match the type to %d by casting to int which is
enough for Linux (standard would allow using a longer integer type but
generalizing for that would complicate the code unnecessarily, the
selftest code does not need to be portable).

Signed-off-by: Ilpo Järvinen 
Tested-by: Babu Moger 
Reviewed-by: Reinette Chatre 
Signed-off-by: Shuah Khan