Hi Ilpo,
On 8/8/2023 2:16 AM, Ilpo Järvinen wrote:
Benchmark command is copied into an array in the stack. The array is BENCHMARK_ARGS items long but the command line could try to provide a longer command.
Return error in case the benchmark command does not fit to its array.
Fixes: ecdbb911f22d ("selftests/resctrl: Add MBM test") Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
tools/testing/selftests/resctrl/resctrl_tests.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index d511daeb6851..eef9e02516ad 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -255,6 +255,9 @@ int main(int argc, char **argv) return ksft_exit_skip("Not running as root. Skipping...\n"); if (has_ben) {
if (argc - ben_ind >= BENCHMARK_ARGS - 1)ksft_exit_fail_msg("Too long benchmark command");
I think there are two potential issues here: too many arguments and too long arguments. Current code can handle 64 (63 with last required to be NULL) arguments each expected to be 64 bytes (63 to end with \0). The above fix looks to be handling the first issue, with this the error message could be more accurate if it refers to the number of arguments instead. It looks to me as though the latter issue still needs to be handled. I understand that this becomes unnecessary via the refactor in following patches but I expect that this fix needs to be backported (cc. stable also) and thus it may benefit from a precision added to the sprintf() that follows the snippet below?
/* Extract benchmark command from command line. */ for (i = ben_ind; i < argc; i++) { benchmark_cmd[i - ben_ind] = benchmark_cmd_area[i];
Reinette
ps. Unless you have an updated email address that works, could you please remove Sai's email from future submissions?