On Thu, 17 Oct 2024, Reinette Chatre wrote:
> 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_<imc instance>/events/cas_count_read
> Similarly, the resctrl selftests discover the cache size via
> /sys/bus/cpu/devices/cpu<id>/cache/index<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 <reinette.chatre@xxxxxxxxx>
> ---
> This makes the code robust against any changes in information read
> from sysfs. The existing sysfs content fit well into the arrays, thus
> this is not considered a bugfix.
>
> Changes since V2:
> - New patch
> ---
> tools/testing/selftests/resctrl/resctrl_val.c | 4 ++--
> tools/testing/selftests/resctrl/resctrlfs.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index e88d5ca30517..c9dd70ce3ea8 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -159,7 +159,7 @@ static int read_from_imc_dir(char *imc_dir, int count)
>
> return -1;
> }
> - if (fscanf(fp, "%s", cas_count_cfg) <= 0) {
> + if (fscanf(fp, "%1023s", cas_count_cfg) <= 0) {
> ksft_perror("Could not get iMC cas count read");
> fclose(fp);
>
> @@ -177,7 +177,7 @@ static int read_from_imc_dir(char *imc_dir, int count)
>
> return -1;
> }
> - if (fscanf(fp, "%s", cas_count_cfg) <= 0) {
> + if (fscanf(fp, "%1023s", cas_count_cfg) <= 0) {
> ksft_perror("Could not get iMC cas count write");
> fclose(fp);
>
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
> index 250c320349a7..a53cd1cb6e0c 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -182,7 +182,7 @@ int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size
>
> return -1;
> }
> - if (fscanf(fp, "%s", cache_str) <= 0) {
> + if (fscanf(fp, "%63s", cache_str) <= 0) {
> ksft_perror("Could not get cache_size");
> fclose(fp);
>
>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
--
i.
