From 2b14b84e252d2cb4e79060dcdd13cf7fdc0792e3 Mon Sep 17 00:00:00 2001 From: darwvin-dev <178835399+darwvin-dev@users.noreply.github.com> Date: Wed, 17 Jun 2026 11:52:55 +0330 Subject: [PATCH 1/4] feat: add multi-instance launcher --- docs/index.rst | 1 + docs/multi_instance.rst | 34 ++++ include/multi_instance.hpp | 46 +++++ src/multi_instance.cpp | 392 +++++++++++++++++++++++++++++++++++++ src/sipp.cpp | 73 +++++++ src/sipp_unittest.cpp | 43 ++++ 6 files changed, 589 insertions(+) create mode 100644 docs/multi_instance.rst create mode 100644 include/multi_instance.hpp create mode 100644 src/multi_instance.cpp diff --git a/docs/index.rst b/docs/index.rst index 34fdd4995..f797799ee 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -18,6 +18,7 @@ Welcome to SIPp reference documentation! 3PCC_extended controlling transport + multi_instance media statistics error diff --git a/docs/multi_instance.rst b/docs/multi_instance.rst new file mode 100644 index 000000000..0ba1e8ed2 --- /dev/null +++ b/docs/multi_instance.rst @@ -0,0 +1,34 @@ +Multi-instance launcher +======================= + +SIPp can launch several SIPp processes from one CSV configuration file. This +is useful when a test needs matching groups of UAC and UAS instances. + +Use ``-multi`` with a CSV file: + +.. code-block:: bash + + ./sipp -multi multi.csv -multi_base_port 5060 + +The CSV format is: + +.. code-block:: text + + role,count,args + uas,2,"-sn uas -p {instance_port} -nostdin" + uac,2,"-sn uac 127.0.0.1:{instance_port} -m 100 -nostdin" + +Each row creates ``count`` child processes. The ``args`` field is split like +command-line arguments and passed to each child ``sipp`` process. + +The following placeholders are expanded in the ``args`` field: + +* ``{role}``: the role column value. +* ``{instance}``: the zero-based instance number within that role. +* ``{base_port}``: the value passed with ``-multi_base_port``. +* ``{instance_port}``: ``base_port + instance``. Use this to pair UAC and UAS + rows by instance number. +* ``{port}``: a globally increasing port number for every child process. + +The launcher waits until all children exit and returns the first non-zero child +exit code. If all children exit successfully, the launcher exits with zero. diff --git a/include/multi_instance.hpp b/include/multi_instance.hpp new file mode 100644 index 000000000..a5b93d18c --- /dev/null +++ b/include/multi_instance.hpp @@ -0,0 +1,46 @@ +/* + * Multi-instance launcher support for SIPp. + */ + +#ifndef __MULTI_INSTANCE__ +#define __MULTI_INSTANCE__ + +#include +#include +#include + +struct MultiInstanceSpec { + std::string role; + int count; + std::string args; +}; + +struct MultiInstanceCommand { + std::string role; + int instance; + int port; + std::string executable_path; + std::vector argv; +}; + +std::string resolve_current_executable_path(); + +bool parse_multi_instance_csv(const std::string &csv, + const std::string &source_name, + std::vector *specs, + std::string *error); + +bool parse_multi_instance_csv_file(const std::string &path, + std::vector *specs, + std::string *error); + +std::vector +build_multi_instance_commands(const std::string &program_path, + const std::vector &specs, + int base_port); + +int run_multi_instance_commands(const std::vector &commands, + std::ostream &out, + std::ostream &err); + +#endif diff --git a/src/multi_instance.cpp b/src/multi_instance.cpp new file mode 100644 index 000000000..45e459684 --- /dev/null +++ b/src/multi_instance.cpp @@ -0,0 +1,392 @@ +/* + * Multi-instance launcher support for SIPp. + */ + +#include "multi_instance.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifdef __APPLE__ +#include +#endif + +#include +#include +#include + +static std::string trim_copy(const std::string &value) +{ + size_t first = value.find_first_not_of(" \t\r\n"); + if (first == std::string::npos) { + return ""; + } + size_t last = value.find_last_not_of(" \t\r\n"); + return value.substr(first, last - first + 1); +} + +static bool parse_csv_line(const std::string &line, + std::vector *fields, + std::string *error) +{ + fields->clear(); + std::string current; + bool in_quotes = false; + + size_t i = 0; + while (i < line.size()) { + char ch = line[i]; + if (in_quotes) { + if (ch == '"') { + if ((i + 1 < line.size()) && line[i + 1] == '"') { + current.push_back('"'); + i += 2; + continue; + } else { + in_quotes = false; + } + } else { + current.push_back(ch); + } + } else if (ch == '"') { + in_quotes = true; + } else if (ch == ',') { + fields->push_back(trim_copy(current)); + current.clear(); + } else { + current.push_back(ch); + } + ++i; + } + + if (in_quotes) { + *error = "unterminated quoted CSV field"; + return false; + } + + fields->push_back(trim_copy(current)); + return true; +} + +static bool split_args(const std::string &args, + std::vector *words, + std::string *error) +{ + words->clear(); + std::string current; + char quote = 0; + bool escaping = false; + + for (char ch : args) { + if (escaping) { + current.push_back(ch); + escaping = false; + continue; + } + if (ch == '\\') { + escaping = true; + continue; + } + if (quote) { + if (ch == quote) { + quote = 0; + } else { + current.push_back(ch); + } + continue; + } + if (ch == '\'' || ch == '"') { + quote = ch; + } else if (ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n') { + if (!current.empty()) { + words->push_back(current); + current.clear(); + } + } else { + current.push_back(ch); + } + } + + if (escaping) { + current.push_back('\\'); + } + if (quote) { + *error = "unterminated quoted argument"; + return false; + } + if (!current.empty()) { + words->push_back(current); + } + + return true; +} + +static void replace_all(std::string *value, + const std::string &needle, + const std::string &replacement) +{ + size_t pos = 0; + while ((pos = value->find(needle, pos)) != std::string::npos) { + value->replace(pos, needle.size(), replacement); + pos += replacement.size(); + } +} + +static bool canonical_regular_file_path(const std::string &path, + std::string *canonical_path, + std::string *error) +{ + char resolved[PATH_MAX]; + if (!realpath(path.c_str(), resolved)) { + *error = "unable to resolve multi-instance file: " + path + ": " + + strerror(errno); + return false; + } + + struct stat st; + if (stat(resolved, &st) != 0) { + *error = "unable to stat multi-instance file: " + std::string(resolved) + + ": " + strerror(errno); + return false; + } + if (!S_ISREG(st.st_mode)) { + *error = "multi-instance file is not a regular file: " + + std::string(resolved); + return false; + } + + *canonical_path = resolved; + return true; +} + +std::string resolve_current_executable_path() +{ +#ifdef __linux__ + char path[PATH_MAX]; + ssize_t length = readlink("/proc/self/exe", path, sizeof(path) - 1); + if (length > 0) { + path[length] = '\0'; + return path; + } +#elif defined(__APPLE__) + char path[PATH_MAX]; + uint32_t size = sizeof(path); + if (_NSGetExecutablePath(path, &size) == 0) { + char resolved[PATH_MAX]; + if (realpath(path, resolved)) { + return resolved; + } + return path; + } +#endif + + return "sipp"; +} + +bool parse_multi_instance_csv(const std::string &csv, + const std::string &source_name, + std::vector *specs, + std::string *error) +{ + specs->clear(); + std::istringstream input(csv); + std::string line; + int line_number = 0; + + while (std::getline(input, line)) { + ++line_number; + if (!line.empty() && line[line.size() - 1] == '\r') { + line.erase(line.size() - 1); + } + std::string trimmed = trim_copy(line); + if (trimmed.empty() || trimmed[0] == '#') { + continue; + } + + std::vector fields; + std::string parse_error; + if (!parse_csv_line(line, &fields, &parse_error)) { + *error = source_name + ":" + std::to_string(line_number) + ": " + parse_error; + return false; + } + + if (fields.size() != 3) { + *error = source_name + ":" + std::to_string(line_number) + + ": expected role,count,args"; + return false; + } + + if (line_number == 1 && fields[0] == "role" && fields[1] == "count" && + fields[2] == "args") { + continue; + } + + char *end = nullptr; + long count = strtol(fields[1].c_str(), &end, 10); + if (*end != '\0') { + *error = source_name + ":" + std::to_string(line_number) + + ": count must be a number"; + return false; + } + if (count <= 0) { + *error = source_name + ":" + std::to_string(line_number) + + ": count must be greater than zero"; + return false; + } + if (fields[0].empty()) { + *error = source_name + ":" + std::to_string(line_number) + + ": role must not be empty"; + return false; + } + + std::vector unused_words; + if (!split_args(fields[2], &unused_words, error)) { + *error = source_name + ":" + std::to_string(line_number) + ": " + *error; + return false; + } + + MultiInstanceSpec spec; + spec.role = fields[0]; + spec.count = static_cast(count); + spec.args = fields[2]; + specs->push_back(spec); + } + + if (specs->empty()) { + *error = source_name + ": no multi-instance rows found"; + return false; + } + + return true; +} + +bool parse_multi_instance_csv_file(const std::string &path, + std::vector *specs, + std::string *error) +{ + std::string canonical_path; + if (!canonical_regular_file_path(path, &canonical_path, error)) { + return false; + } + + std::ifstream file(canonical_path); + if (!file.good()) { + *error = "unable to open multi-instance file: " + canonical_path; + return false; + } + + std::ostringstream contents; + contents << file.rdbuf(); + return parse_multi_instance_csv(contents.str(), canonical_path, specs, error); +} + +std::vector +build_multi_instance_commands(const std::string &program_path, + const std::vector &specs, + int base_port) +{ + std::vector commands; + int next_port = base_port; + + for (const MultiInstanceSpec &spec : specs) { + for (int instance = 0; instance < spec.count; ++instance) { + std::string expanded = spec.args; + int instance_port = base_port + instance; + replace_all(&expanded, "{role}", spec.role); + replace_all(&expanded, "{instance}", std::to_string(instance)); + replace_all(&expanded, "{instance_port}", std::to_string(instance_port)); + replace_all(&expanded, "{base_port}", std::to_string(base_port)); + replace_all(&expanded, "{port}", std::to_string(next_port)); + + std::vector words; + std::string error; + if (!split_args(expanded, &words, &error)) { + words.clear(); + } + + MultiInstanceCommand command; + command.role = spec.role; + command.instance = instance; + command.port = next_port; + command.executable_path = program_path; + command.argv.push_back(program_path); + command.argv.insert(command.argv.end(), words.begin(), words.end()); + commands.push_back(command); + ++next_port; + } + } + + return commands; +} + +int run_multi_instance_commands(const std::vector &commands, + std::ostream &out, + std::ostream &err) +{ + std::vector children; + int exit_code = 0; + + for (const MultiInstanceCommand &command : commands) { + out << "Starting " << command.role << "[" << command.instance << "]"; + if (command.port > 0) { + out << " port=" << command.port; + } + out << ":"; + for (const std::string &arg : command.argv) { + out << " " << arg; + } + out << "\n"; + out.flush(); + + pid_t child = fork(); + if (child < 0) { + err << "fork failed: " << strerror(errno) << "\n"; + exit_code = 1; + break; + } + if (child == 0) { + std::vector argv; + argv.reserve(command.argv.size() + 1); + for (const std::string &arg : command.argv) { + argv.push_back(const_cast(arg.c_str())); + } + argv.push_back(nullptr); + execv(command.executable_path.c_str(), argv.data()); + std::cerr << "exec failed for " << command.executable_path << ": " + << strerror(errno) << "\n"; + _exit(127); + } + children.push_back(child); + } + + for (pid_t child : children) { + int status = 0; + while (waitpid(child, &status, 0) < 0) { + if (errno != EINTR) { + err << "waitpid failed for " << child << ": " << strerror(errno) << "\n"; + exit_code = 1; + break; + } + } + if (WIFEXITED(status)) { + int child_exit = WEXITSTATUS(status); + if (child_exit != 0 && exit_code == 0) { + exit_code = child_exit; + } + } else if (WIFSIGNALED(status)) { + int signal_number = WTERMSIG(status); + if (exit_code == 0) { + exit_code = 128 + signal_number; + } + } + } + + return exit_code; +} diff --git a/src/sipp.cpp b/src/sipp.cpp index 49586742f..b3afa51a8 100644 --- a/src/sipp.cpp +++ b/src/sipp.cpp @@ -60,6 +60,7 @@ extern char** environ; #define GLOBALS_FULL_DEFINITION #include "sipp.hpp" +#include "multi_instance.hpp" #include "sip_parser.hpp" #include "socket.hpp" #include "logger.hpp" @@ -126,6 +127,8 @@ struct sipp_option { #define SIPP_OPTION_NEED_SCTP 39 #define SIPP_OPTION_RX_SCENARIO 40 #define SIPP_OPTION_RX_INPUT_FILE 41 +#define SIPP_OPTION_CID_TYPE 42 +#define SIPP_OPTION_MULTI 43 #define SIPP_HELP_TEXT_HEADER 255 static char *call_id_mode_string = nullptr; @@ -623,6 +626,10 @@ struct sipp_option options_table[] = { {"buff_size", "Set the send and receive buffer size.", SIPP_OPTION_INT, &buff_size, 1}, {"sendbuffer_warn", "Produce warnings instead of errors on SendBuffer failures.", SIPP_OPTION_BOOL, &sendbuffer_warn, 1}, {"lost", "Set the number of packets to lose by default (scenario specifications override this value).", SIPP_OPTION_FLOAT, &global_lost, 1}, + {"multi", "Launch multiple SIPp instances from a CSV file and wait for all of them to exit.\n" + "CSV format: role,count,args. The args field is split like shell arguments and supports {role}, {instance}, {base_port}, {instance_port}, and {port} placeholders.\n" + "Example row: uas,2,\"-sn uas -p {port}\"", SIPP_OPTION_MULTI, nullptr, 0}, + {"multi_base_port", "Set the first {port} value used by -multi. Default is 5060.", SIPP_OPTION_MULTI, nullptr, 0}, {"key", "keyword value\nSet the generic parameter named \"keyword\" to \"value\".", SIPP_OPTION_KEY, nullptr, 1}, {"set", "variable value\nSet the global variable parameter named \"variable\" to \"value\".", SIPP_OPTION_VAR, nullptr, 3}, {"tdmmap", "Generate and handle a table of TDM circuits.\n" @@ -1760,6 +1767,67 @@ void randomseed(void) srand(seed); } +static bool get_multi_arg(int argc, + char *argv[], + int *argi, + const char *option, + std::string *value) +{ + if ((*argi + 1) >= argc) { + std::cerr << "Missing argument for " << option << "\n"; + return false; + } + ++(*argi); + *value = argv[*argi]; + return true; +} + +static bool maybe_run_multi_instance(int argc, char *argv[], int *exit_code) +{ + std::string config_path; + int base_port = DEFAULT_PORT; + + for (int argi = 1; argi < argc; ++argi) { + if (!strcmp(argv[argi], "-multi")) { + if (!get_multi_arg(argc, argv, &argi, "-multi", &config_path)) { + *exit_code = EXIT_OTHER; + return true; + } + } else if (!strcmp(argv[argi], "-multi_base_port")) { + std::string value; + if (!get_multi_arg(argc, argv, &argi, "-multi_base_port", &value)) { + *exit_code = EXIT_OTHER; + return true; + } + char *end = nullptr; + long parsed_port = strtol(value.c_str(), &end, 10); + if (*end != '\0' || parsed_port <= 0 || parsed_port > 65535) { + std::cerr << "Invalid -multi_base_port value: " << value << "\n"; + *exit_code = EXIT_OTHER; + return true; + } + base_port = static_cast(parsed_port); + } + } + + if (config_path.empty()) { + return false; + } + + std::vector specs; + std::string error; + if (!parse_multi_instance_csv_file(config_path, &specs, &error)) { + std::cerr << error << "\n"; + *exit_code = EXIT_OTHER; + return true; + } + + std::vector commands = + build_multi_instance_commands(resolve_current_executable_path(), specs, base_port); + *exit_code = run_multi_instance_commands(commands, std::cout, std::cerr); + return true; +} + /* Main */ int main(int argc, char *argv[]) { @@ -1776,6 +1844,11 @@ int main(int argc, char *argv[]) randomseed(); + int multi_exit_code = 0; + if (maybe_run_multi_instance(argc, argv, &multi_exit_code)) { + return multi_exit_code; + } + if (should_launch_startup_wizard(argc)) { wizard_args_storage = launch_startup_wizard(argv[0]); if (wizard_args_storage.empty()) { diff --git a/src/sipp_unittest.cpp b/src/sipp_unittest.cpp index af7787997..23fb59dc8 100644 --- a/src/sipp_unittest.cpp +++ b/src/sipp_unittest.cpp @@ -19,6 +19,8 @@ #define GLOBALS_FULL_DEFINITION #include "sipp.hpp" +#include "multi_instance.hpp" + #include "gtest/gtest.h" #include @@ -39,3 +41,44 @@ void sipp_exit(int rc, int rtp_errors, int echo_errors) { exit(rc); } + +TEST(MultiInstanceConfig, ParsesQuotedCsvAndExpandsInstanceArguments) +{ + const std::string csv = + "role,count,args\n" + "uas,2,\"-sn uas -p {port}\"\n" + "uac,2,\"-sn uac 127.0.0.1:{instance_port} -key role {role} -key idx {instance}\"\n"; + + std::string error; + std::vector specs; + + ASSERT_TRUE(parse_multi_instance_csv(csv, "inline.csv", &specs, &error)) << error; + ASSERT_EQ(2u, specs.size()); + EXPECT_EQ("uas", specs[0].role); + EXPECT_EQ(2, specs[0].count); + EXPECT_EQ("-sn uas -p {port}", specs[0].args); + EXPECT_EQ("uac", specs[1].role); + EXPECT_EQ(2, specs[1].count); + + std::vector commands = + build_multi_instance_commands("./sipp", specs, 5060); + + ASSERT_EQ(4u, commands.size()); + EXPECT_EQ(std::vector({"./sipp", "-sn", "uas", "-p", "5060"}), commands[0].argv); + EXPECT_EQ(std::vector({"./sipp", "-sn", "uas", "-p", "5061"}), commands[1].argv); + EXPECT_EQ(std::vector({"./sipp", "-sn", "uac", "127.0.0.1:5060", "-key", "role", "uac", "-key", "idx", "0"}), commands[2].argv); + EXPECT_EQ(std::vector({"./sipp", "-sn", "uac", "127.0.0.1:5061", "-key", "role", "uac", "-key", "idx", "1"}), commands[3].argv); +} + +TEST(MultiInstanceConfig, ReportsInvalidCsvRows) +{ + const std::string csv = + "role,count,args\n" + "uas,0,\"-sn uas\"\n"; + + std::string error; + std::vector specs; + + EXPECT_FALSE(parse_multi_instance_csv(csv, "bad.csv", &specs, &error)); + EXPECT_NE(std::string::npos, error.find("count must be greater than zero")); +} From 9fc0817bbd0f0c038f5caea4f11375b283a2f86d Mon Sep 17 00:00:00 2001 From: Darwvin Date: Tue, 25 Aug 2026 13:12:56 +0330 Subject: [PATCH 2/4] fix: address multi-instance launcher review --- docs/multi_instance.rst | 35 +++- include/multi_instance.hpp | 43 ++++- src/multi_instance.cpp | 379 +++++++++++++++++++++++++++++++------ src/sipp.cpp | 94 +++------ src/sipp_unittest.cpp | 189 +++++++++++++++++- 5 files changed, 597 insertions(+), 143 deletions(-) diff --git a/docs/multi_instance.rst b/docs/multi_instance.rst index 0ba1e8ed2..f8c35cb5e 100644 --- a/docs/multi_instance.rst +++ b/docs/multi_instance.rst @@ -10,6 +10,11 @@ Use ``-multi`` with a CSV file: ./sipp -multi multi.csv -multi_base_port 5060 +``-multi`` is a launcher mode. When it is present, the launcher accepts only +``-multi`` and ``-multi_base_port``; put normal SIPp options in the CSV +``args`` field instead. ``-multi_base_port`` cannot be used without +``-multi``. + The CSV format is: .. code-block:: text @@ -18,8 +23,14 @@ The CSV format is: uas,2,"-sn uas -p {instance_port} -nostdin" uac,2,"-sn uac 127.0.0.1:{instance_port} -m 100 -nostdin" -Each row creates ``count`` child processes. The ``args`` field is split like -command-line arguments and passed to each child ``sipp`` process. +Blank lines and lines whose first non-whitespace character is ``#`` are +ignored. The header is optional and, when present, may use any letter case. +A configuration may launch at most 256 child processes in total. + +Each row creates ``count`` child processes. The ``args`` field is split once +into command-line arguments before placeholders are expanded. This keeps +placeholder values containing spaces or quote characters as a single +argument instead of re-parsing them as shell syntax. The following placeholders are expanded in the ``args`` field: @@ -30,5 +41,21 @@ The following placeholders are expanded in the ``args`` field: rows by instance number. * ``{port}``: a globally increasing port number for every child process. -The launcher waits until all children exit and returns the first non-zero child -exit code. If all children exit successfully, the launcher exits with zero. +The launcher validates that all generated ports stay in the range 1 through +65535. It waits until all children exit and returns the first non-zero child +exit code. If a child cannot be forked, children already started by the +launcher are terminated and reaped before the launcher exits with failure. +If all children exit successfully, the launcher exits with zero. + +All children inherit the launcher's standard input, output, and error streams. +Multiple interactive SIPp screens will therefore interleave on one terminal. +Use ``-nostdin`` for children and redirect the launcher's output when a clean +terminal is needed, for example: + +.. code-block:: bash + + ./sipp -multi multi.csv >multi.log 2>&1 + +A child may also use SIPp's ``-bg`` option when independent backgrounding is +desired; in that case the launcher only supervises the process until that +child backgrounds itself. diff --git a/include/multi_instance.hpp b/include/multi_instance.hpp index a5b93d18c..72c452d2f 100644 --- a/include/multi_instance.hpp +++ b/include/multi_instance.hpp @@ -2,17 +2,30 @@ * Multi-instance launcher support for SIPp. */ -#ifndef __MULTI_INSTANCE__ -#define __MULTI_INSTANCE__ +#ifndef __SIPP_MULTI_INSTANCE_H__ +#define __SIPP_MULTI_INSTANCE_H__ #include #include #include +constexpr int MAX_MULTI_INSTANCE_CHILDREN = 256; + +enum class MultiInstanceArgParseResult { + NOT_REQUESTED, + READY, + INVALID +}; + +struct MultiInstanceOptions { + std::string config_path; + int base_port; +}; + struct MultiInstanceSpec { std::string role; int count; - std::string args; + std::vector args; }; struct MultiInstanceCommand { @@ -23,7 +36,20 @@ struct MultiInstanceCommand { std::vector argv; }; -std::string resolve_current_executable_path(); +std::string trim_copy(const std::string &value); + +bool split_command_args(const std::string &args, + std::vector *words, + std::string *error); + +MultiInstanceArgParseResult +parse_multi_instance_launcher_args(int argc, + char *argv[], + int default_base_port, + MultiInstanceOptions *options, + std::string *error); + +std::string resolve_current_executable_path(const char *argv0); bool parse_multi_instance_csv(const std::string &csv, const std::string &source_name, @@ -34,10 +60,11 @@ bool parse_multi_instance_csv_file(const std::string &path, std::vector *specs, std::string *error); -std::vector -build_multi_instance_commands(const std::string &program_path, - const std::vector &specs, - int base_port); +bool build_multi_instance_commands(const std::string &program_path, + const std::vector &specs, + int base_port, + std::vector *commands, + std::string *error); int run_multi_instance_commands(const std::vector &commands, std::ostream &out, diff --git a/src/multi_instance.cpp b/src/multi_instance.cpp index 45e459684..188e4aa1a 100644 --- a/src/multi_instance.cpp +++ b/src/multi_instance.cpp @@ -18,11 +18,12 @@ #include #endif +#include #include #include #include -static std::string trim_copy(const std::string &value) +std::string trim_copy(const std::string &value) { size_t first = value.find_first_not_of(" \t\r\n"); if (first == std::string::npos) { @@ -32,6 +33,120 @@ static std::string trim_copy(const std::string &value) return value.substr(first, last - first + 1); } +static std::string lowercase_copy(std::string value) +{ + for (char &ch : value) { + ch = static_cast(std::tolower(static_cast(ch))); + } + return value; +} + +static bool multi_option_matches(const char *value, const char *name) +{ + if (!value) { + return false; + } + + std::string short_option = "-" + std::string(name); + std::string long_option = "--" + std::string(name); + return value == short_option || value == long_option; +} + +static bool parse_port(const std::string &value, int *port) +{ + errno = 0; + char *end = nullptr; + long parsed = strtol(value.c_str(), &end, 10); + if (errno == ERANGE || end == value.c_str() || *end != '\0' || + parsed <= 0 || parsed > 65535) { + return false; + } + + *port = static_cast(parsed); + return true; +} + +MultiInstanceArgParseResult +parse_multi_instance_launcher_args(int argc, + char *argv[], + int default_base_port, + MultiInstanceOptions *options, + std::string *error) +{ + options->config_path.clear(); + options->base_port = default_base_port; + + bool saw_multi = false; + bool saw_base_port = false; + std::string unexpected_argument; + + for (int argi = 1; argi < argc; ++argi) { + if (multi_option_matches(argv[argi], "multi")) { + if (saw_multi) { + *error = "-multi may only be specified once"; + return MultiInstanceArgParseResult::INVALID; + } + if ((argi + 1) >= argc) { + *error = "Missing argument for -multi"; + return MultiInstanceArgParseResult::INVALID; + } + saw_multi = true; + options->config_path = argv[++argi]; + } else if (multi_option_matches(argv[argi], "multi_base_port")) { + if (saw_base_port) { + *error = "-multi_base_port may only be specified once"; + return MultiInstanceArgParseResult::INVALID; + } + if ((argi + 1) >= argc) { + *error = "Missing argument for -multi_base_port"; + return MultiInstanceArgParseResult::INVALID; + } + saw_base_port = true; + std::string value = argv[++argi]; + if (!parse_port(value, &options->base_port)) { + *error = "Invalid -multi_base_port value: " + value; + return MultiInstanceArgParseResult::INVALID; + } + } else if (unexpected_argument.empty()) { + unexpected_argument = argv[argi]; + } + } + + if (!saw_multi && !saw_base_port) { + return MultiInstanceArgParseResult::NOT_REQUESTED; + } + + if (!saw_multi) { + *error = "-multi_base_port requires -multi"; + return MultiInstanceArgParseResult::INVALID; + } + + if (options->config_path.empty()) { + *error = "-multi requires a non-empty CSV file path"; + return MultiInstanceArgParseResult::INVALID; + } + + if (!unexpected_argument.empty()) { + *error = "Unexpected argument in -multi mode: " + unexpected_argument + + ". Only -multi and -multi_base_port are accepted by the launcher"; + return MultiInstanceArgParseResult::INVALID; + } + + return MultiInstanceArgParseResult::READY; +} + +static void finish_csv_field(std::string *current, + bool field_was_quoted, + std::vector *fields) +{ + if (field_was_quoted) { + fields->push_back(*current); + } else { + fields->push_back(trim_copy(*current)); + } + current->clear(); +} + static bool parse_csv_line(const std::string &line, std::vector *fields, std::string *error) @@ -39,31 +154,53 @@ static bool parse_csv_line(const std::string &line, fields->clear(); std::string current; bool in_quotes = false; + bool field_was_quoted = false; + bool quote_closed = false; - size_t i = 0; - while (i < line.size()) { + for (size_t i = 0; i < line.size(); ++i) { char ch = line[i]; + if (in_quotes) { if (ch == '"') { if ((i + 1 < line.size()) && line[i + 1] == '"') { current.push_back('"'); - i += 2; - continue; + ++i; } else { in_quotes = false; + quote_closed = true; } } else { current.push_back(ch); } + continue; + } + + if (quote_closed) { + if (ch == ',') { + finish_csv_field(¤t, field_was_quoted, fields); + field_was_quoted = false; + quote_closed = false; + } else if (!std::isspace(static_cast(ch))) { + *error = "unexpected character after quoted CSV field"; + return false; + } + continue; + } + + if (ch == ',') { + finish_csv_field(¤t, field_was_quoted, fields); + field_was_quoted = false; } else if (ch == '"') { - in_quotes = true; - } else if (ch == ',') { - fields->push_back(trim_copy(current)); + if (!trim_copy(current).empty()) { + *error = "unexpected quote in unquoted CSV field"; + return false; + } current.clear(); + in_quotes = true; + field_was_quoted = true; } else { current.push_back(ch); } - ++i; } if (in_quotes) { @@ -71,27 +208,30 @@ static bool parse_csv_line(const std::string &line, return false; } - fields->push_back(trim_copy(current)); + finish_csv_field(¤t, field_was_quoted, fields); return true; } -static bool split_args(const std::string &args, - std::vector *words, - std::string *error) +bool split_command_args(const std::string &args, + std::vector *words, + std::string *error) { words->clear(); std::string current; char quote = 0; bool escaping = false; + bool token_started = false; for (char ch : args) { if (escaping) { current.push_back(ch); escaping = false; + token_started = true; continue; } if (ch == '\\') { escaping = true; + token_started = true; continue; } if (quote) { @@ -100,17 +240,21 @@ static bool split_args(const std::string &args, } else { current.push_back(ch); } + token_started = true; continue; } if (ch == '\'' || ch == '"') { quote = ch; - } else if (ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n') { - if (!current.empty()) { + token_started = true; + } else if (std::isspace(static_cast(ch))) { + if (token_started) { words->push_back(current); current.clear(); + token_started = false; } } else { current.push_back(ch); + token_started = true; } } @@ -121,7 +265,7 @@ static bool split_args(const std::string &args, *error = "unterminated quoted argument"; return false; } - if (!current.empty()) { + if (token_started) { words->push_back(current); } @@ -166,7 +310,51 @@ static bool canonical_regular_file_path(const std::string &path, return true; } -std::string resolve_current_executable_path() +#ifndef __linux__ +#ifndef __APPLE__ +static std::string resolve_from_argv0(const char *argv0) +{ + if (!argv0 || !argv0[0]) { + return ""; + } + + char resolved[PATH_MAX]; + if (realpath(argv0, resolved)) { + return resolved; + } + + if (strchr(argv0, '/')) { + return argv0; + } + + const char *path_value = getenv("PATH"); + if (!path_value) { + return ""; + } + + std::istringstream paths(path_value); + std::string directory; + while (std::getline(paths, directory, ':')) { + if (directory.empty()) { + directory = "."; + } + std::string candidate = directory + "/" + argv0; + struct stat st; + if (stat(candidate.c_str(), &st) == 0 && S_ISREG(st.st_mode) && + access(candidate.c_str(), X_OK) == 0) { + if (realpath(candidate.c_str(), resolved)) { + return resolved; + } + return candidate; + } + } + + return ""; +} +#endif +#endif + +std::string resolve_current_executable_path(const char *argv0) { #ifdef __linux__ char path[PATH_MAX]; @@ -175,6 +363,7 @@ std::string resolve_current_executable_path() path[length] = '\0'; return path; } + return ""; #elif defined(__APPLE__) char path[PATH_MAX]; uint32_t size = sizeof(path); @@ -185,9 +374,10 @@ std::string resolve_current_executable_path() } return path; } + return ""; +#else + return resolve_from_argv0(argv0); #endif - - return "sipp"; } bool parse_multi_instance_csv(const std::string &csv, @@ -199,6 +389,8 @@ bool parse_multi_instance_csv(const std::string &csv, std::istringstream input(csv); std::string line; int line_number = 0; + bool first_content_row = true; + int total_children = 0; while (std::getline(input, line)) { ++line_number; @@ -223,14 +415,30 @@ bool parse_multi_instance_csv(const std::string &csv, return false; } - if (line_number == 1 && fields[0] == "role" && fields[1] == "count" && - fields[2] == "args") { - continue; + if (first_content_row) { + first_content_row = false; + if (lowercase_copy(fields[0]) == "role" && + lowercase_copy(fields[1]) == "count" && + lowercase_copy(fields[2]) == "args") { + continue; + } } + if (trim_copy(fields[0]).empty()) { + *error = source_name + ":" + std::to_string(line_number) + + ": role must not be empty"; + return false; + } + + errno = 0; char *end = nullptr; long count = strtol(fields[1].c_str(), &end, 10); - if (*end != '\0') { + if (errno == ERANGE || count > INT_MAX) { + *error = source_name + ":" + std::to_string(line_number) + + ": count is out of range"; + return false; + } + if (end == fields[1].c_str() || *end != '\0') { *error = source_name + ":" + std::to_string(line_number) + ": count must be a number"; return false; @@ -240,23 +448,26 @@ bool parse_multi_instance_csv(const std::string &csv, ": count must be greater than zero"; return false; } - if (fields[0].empty()) { + if (count > MAX_MULTI_INSTANCE_CHILDREN || + total_children > MAX_MULTI_INSTANCE_CHILDREN - count) { *error = source_name + ":" + std::to_string(line_number) + - ": role must not be empty"; + ": multi-instance configuration exceeds the maximum of " + + std::to_string(MAX_MULTI_INSTANCE_CHILDREN) + " child processes"; return false; } - std::vector unused_words; - if (!split_args(fields[2], &unused_words, error)) { - *error = source_name + ":" + std::to_string(line_number) + ": " + *error; + std::vector words; + if (!split_command_args(fields[2], &words, &parse_error)) { + *error = source_name + ":" + std::to_string(line_number) + ": " + parse_error; return false; } MultiInstanceSpec spec; spec.role = fields[0]; spec.count = static_cast(count); - spec.args = fields[2]; + spec.args = words; specs->push_back(spec); + total_children += spec.count; } if (specs->empty()) { @@ -287,43 +498,87 @@ bool parse_multi_instance_csv_file(const std::string &path, return parse_multi_instance_csv(contents.str(), canonical_path, specs, error); } -std::vector -build_multi_instance_commands(const std::string &program_path, - const std::vector &specs, - int base_port) +bool build_multi_instance_commands(const std::string &program_path, + const std::vector &specs, + int base_port, + std::vector *commands, + std::string *error) { - std::vector commands; - int next_port = base_port; + commands->clear(); + + if (program_path.empty()) { + *error = "unable to resolve SIPp executable path"; + return false; + } + if (base_port <= 0 || base_port > 65535) { + *error = "multi-instance base port must be between 1 and 65535"; + return false; + } for (const MultiInstanceSpec &spec : specs) { + if (spec.count <= 0 || + commands->size() + static_cast(spec.count) > + static_cast(MAX_MULTI_INSTANCE_CHILDREN)) { + *error = "multi-instance configuration exceeds the maximum of " + + std::to_string(MAX_MULTI_INSTANCE_CHILDREN) + " child processes"; + commands->clear(); + return false; + } + for (int instance = 0; instance < spec.count; ++instance) { - std::string expanded = spec.args; - int instance_port = base_port + instance; - replace_all(&expanded, "{role}", spec.role); - replace_all(&expanded, "{instance}", std::to_string(instance)); - replace_all(&expanded, "{instance_port}", std::to_string(instance_port)); - replace_all(&expanded, "{base_port}", std::to_string(base_port)); - replace_all(&expanded, "{port}", std::to_string(next_port)); - - std::vector words; - std::string error; - if (!split_args(expanded, &words, &error)) { - words.clear(); + long long next_port = static_cast(base_port) + + static_cast(commands->size()); + long long instance_port = static_cast(base_port) + instance; + if (next_port > 65535 || instance_port > 65535) { + *error = "multi-instance port allocation exceeds 65535"; + commands->clear(); + return false; } MultiInstanceCommand command; command.role = spec.role; command.instance = instance; - command.port = next_port; + command.port = static_cast(next_port); command.executable_path = program_path; command.argv.push_back(program_path); - command.argv.insert(command.argv.end(), words.begin(), words.end()); - commands.push_back(command); - ++next_port; + + for (std::string word : spec.args) { + replace_all(&word, "{role}", spec.role); + replace_all(&word, "{instance}", std::to_string(instance)); + replace_all(&word, "{instance_port}", std::to_string(instance_port)); + replace_all(&word, "{base_port}", std::to_string(base_port)); + replace_all(&word, "{port}", std::to_string(next_port)); + command.argv.push_back(word); + } + + commands->push_back(command); } } - return commands; + return true; +} + +static void kill_and_reap_children(const std::vector &children, + std::ostream &err) +{ + for (pid_t child : children) { + if (kill(child, SIGKILL) != 0 && errno != ESRCH) { + err << "failed to terminate child " << child << ": " << strerror(errno) << "\n"; + } + } + + for (pid_t child : children) { + int status = 0; + while (waitpid(child, &status, 0) < 0) { + if (errno == EINTR) { + continue; + } + if (errno != ECHILD) { + err << "waitpid failed for " << child << ": " << strerror(errno) << "\n"; + } + break; + } + } } int run_multi_instance_commands(const std::vector &commands, @@ -348,8 +603,8 @@ int run_multi_instance_commands(const std::vector &command pid_t child = fork(); if (child < 0) { err << "fork failed: " << strerror(errno) << "\n"; - exit_code = 1; - break; + kill_and_reap_children(children, err); + return 1; } if (child == 0) { std::vector argv; @@ -368,13 +623,17 @@ int run_multi_instance_commands(const std::vector &command for (pid_t child : children) { int status = 0; - while (waitpid(child, &status, 0) < 0) { - if (errno != EINTR) { - err << "waitpid failed for " << child << ": " << strerror(errno) << "\n"; - exit_code = 1; - break; - } + pid_t waited; + do { + waited = waitpid(child, &status, 0); + } while (waited < 0 && errno == EINTR); + + if (waited < 0) { + err << "waitpid failed for " << child << ": " << strerror(errno) << "\n"; + exit_code = 1; + continue; } + if (WIFEXITED(status)) { int child_exit = WEXITSTATUS(status); if (child_exit != 0 && exit_code == 0) { diff --git a/src/sipp.cpp b/src/sipp.cpp index b3afa51a8..b49e3591e 100644 --- a/src/sipp.cpp +++ b/src/sipp.cpp @@ -127,8 +127,7 @@ struct sipp_option { #define SIPP_OPTION_NEED_SCTP 39 #define SIPP_OPTION_RX_SCENARIO 40 #define SIPP_OPTION_RX_INPUT_FILE 41 -#define SIPP_OPTION_CID_TYPE 42 -#define SIPP_OPTION_MULTI 43 +#define SIPP_OPTION_MULTI 42 #define SIPP_HELP_TEXT_HEADER 255 static char *call_id_mode_string = nullptr; @@ -187,17 +186,6 @@ static const wizard_scenario_option wizard_scenarios[] = { {"custom", "Custom XML scenario file.", false, true}, }; -static std::string trim_copy(const std::string &value) -{ - size_t first = value.find_first_not_of(" \t\r\n"); - if (first == std::string::npos) { - return ""; - } - - size_t last = value.find_last_not_of(" \t\r\n"); - return value.substr(first, last - first + 1); -} - static bool wizard_cancelled(const std::string &value) { std::string lowered = lowercase_copy(trim_copy(value)); @@ -266,14 +254,10 @@ static bool parse_transport_choice(const std::string &value, std::string *transp static std::vector split_simple_args(const std::string &input) { std::vector result; - std::istringstream words(input); - std::string word; - - /* Keep parsing intentionally simple: this mirrors a shell-style word list. */ - while (words >> word) { - result.push_back(word); + std::string error; + if (!split_command_args(input, &result, &error)) { + return {}; } - return result; } @@ -1767,63 +1751,40 @@ void randomseed(void) srand(seed); } -static bool get_multi_arg(int argc, - char *argv[], - int *argi, - const char *option, - std::string *value) -{ - if ((*argi + 1) >= argc) { - std::cerr << "Missing argument for " << option << "\n"; - return false; - } - ++(*argi); - *value = argv[*argi]; - return true; -} - static bool maybe_run_multi_instance(int argc, char *argv[], int *exit_code) { - std::string config_path; - int base_port = DEFAULT_PORT; - - for (int argi = 1; argi < argc; ++argi) { - if (!strcmp(argv[argi], "-multi")) { - if (!get_multi_arg(argc, argv, &argi, "-multi", &config_path)) { - *exit_code = EXIT_OTHER; - return true; - } - } else if (!strcmp(argv[argi], "-multi_base_port")) { - std::string value; - if (!get_multi_arg(argc, argv, &argi, "-multi_base_port", &value)) { - *exit_code = EXIT_OTHER; - return true; - } - char *end = nullptr; - long parsed_port = strtol(value.c_str(), &end, 10); - if (*end != '\0' || parsed_port <= 0 || parsed_port > 65535) { - std::cerr << "Invalid -multi_base_port value: " << value << "\n"; - *exit_code = EXIT_OTHER; - return true; - } - base_port = static_cast(parsed_port); - } - } + MultiInstanceOptions options; + std::string error; + MultiInstanceArgParseResult parse_result = + parse_multi_instance_launcher_args(argc, argv, DEFAULT_PORT, &options, &error); - if (config_path.empty()) { + if (parse_result == MultiInstanceArgParseResult::NOT_REQUESTED) { return false; } + if (parse_result == MultiInstanceArgParseResult::INVALID) { + std::cerr << error << "\n"; + *exit_code = EXIT_OTHER; + return true; + } std::vector specs; - std::string error; - if (!parse_multi_instance_csv_file(config_path, &specs, &error)) { + if (!parse_multi_instance_csv_file(options.config_path, &specs, &error)) { + std::cerr << error << "\n"; + *exit_code = EXIT_OTHER; + return true; + } + + std::vector commands; + if (!build_multi_instance_commands(resolve_current_executable_path(argv[0]), + specs, + options.base_port, + &commands, + &error)) { std::cerr << error << "\n"; *exit_code = EXIT_OTHER; return true; } - std::vector commands = - build_multi_instance_commands(resolve_current_executable_path(), specs, base_port); *exit_code = run_multi_instance_commands(commands, std::cout, std::cerr); return true; } @@ -2429,6 +2390,9 @@ int main(int argc, char *argv[]) } } break; + case SIPP_OPTION_MULTI: + ERROR("Internal error: multi-instance options should be handled before normal option parsing"); + break; default: ERROR("Internal error: I don't recognize the option type for %s", argv[argi]); } diff --git a/src/sipp_unittest.cpp b/src/sipp_unittest.cpp index 23fb59dc8..4e59b862e 100644 --- a/src/sipp_unittest.cpp +++ b/src/sipp_unittest.cpp @@ -42,12 +42,30 @@ void sipp_exit(int rc, int rtp_errors, int echo_errors) exit(rc); } +static MultiInstanceArgParseResult parse_launcher_args( + const std::vector &arguments, + MultiInstanceOptions *options, + std::string *error) +{ + std::vector storage = arguments; + std::vector argv; + argv.reserve(storage.size()); + for (std::string &argument : storage) { + argv.push_back(argument.data()); + } + return parse_multi_instance_launcher_args(static_cast(argv.size()), + argv.data(), + 5060, + options, + error); +} + TEST(MultiInstanceConfig, ParsesQuotedCsvAndExpandsInstanceArguments) { const std::string csv = "role,count,args\n" "uas,2,\"-sn uas -p {port}\"\n" - "uac,2,\"-sn uac 127.0.0.1:{instance_port} -key role {role} -key idx {instance}\"\n"; + "uac,2,\"-sn uac 127.0.0.1:{instance_port} -m 100 -nostdin\"\n"; std::string error; std::vector specs; @@ -56,18 +74,18 @@ TEST(MultiInstanceConfig, ParsesQuotedCsvAndExpandsInstanceArguments) ASSERT_EQ(2u, specs.size()); EXPECT_EQ("uas", specs[0].role); EXPECT_EQ(2, specs[0].count); - EXPECT_EQ("-sn uas -p {port}", specs[0].args); + EXPECT_EQ(std::vector({"-sn", "uas", "-p", "{port}"}), specs[0].args); EXPECT_EQ("uac", specs[1].role); EXPECT_EQ(2, specs[1].count); - std::vector commands = - build_multi_instance_commands("./sipp", specs, 5060); + std::vector commands; + ASSERT_TRUE(build_multi_instance_commands("./sipp", specs, 5060, &commands, &error)) << error; ASSERT_EQ(4u, commands.size()); EXPECT_EQ(std::vector({"./sipp", "-sn", "uas", "-p", "5060"}), commands[0].argv); EXPECT_EQ(std::vector({"./sipp", "-sn", "uas", "-p", "5061"}), commands[1].argv); - EXPECT_EQ(std::vector({"./sipp", "-sn", "uac", "127.0.0.1:5060", "-key", "role", "uac", "-key", "idx", "0"}), commands[2].argv); - EXPECT_EQ(std::vector({"./sipp", "-sn", "uac", "127.0.0.1:5061", "-key", "role", "uac", "-key", "idx", "1"}), commands[3].argv); + EXPECT_EQ(std::vector({"./sipp", "-sn", "uac", "127.0.0.1:5060", "-m", "100", "-nostdin"}), commands[2].argv); + EXPECT_EQ(std::vector({"./sipp", "-sn", "uac", "127.0.0.1:5061", "-m", "100", "-nostdin"}), commands[3].argv); } TEST(MultiInstanceConfig, ReportsInvalidCsvRows) @@ -82,3 +100,162 @@ TEST(MultiInstanceConfig, ReportsInvalidCsvRows) EXPECT_FALSE(parse_multi_instance_csv(csv, "bad.csv", &specs, &error)); EXPECT_NE(std::string::npos, error.find("count must be greater than zero")); } + +TEST(MultiInstanceConfig, AcceptsHeaderAfterCommentsAndCaseInsensitively) +{ + const std::string csv = + "# generated test configuration\n" + "\n" + "Role,Count,Args\n" + "uas,1,\"-sn uas -nostdin\"\n"; + + std::string error; + std::vector specs; + + ASSERT_TRUE(parse_multi_instance_csv(csv, "comments.csv", &specs, &error)) << error; + ASSERT_EQ(1u, specs.size()); + EXPECT_EQ("uas", specs[0].role); +} + +TEST(MultiInstanceConfig, PreservesQuotedCsvWhitespace) +{ + const std::string csv = + "role,count,args\n" + "\" spaced role \",1,\"-key role {role}\"\n"; + + std::string error; + std::vector specs; + + ASSERT_TRUE(parse_multi_instance_csv(csv, "spaces.csv", &specs, &error)) << error; + ASSERT_EQ(1u, specs.size()); + EXPECT_EQ(" spaced role ", specs[0].role); + + std::vector commands; + ASSERT_TRUE(build_multi_instance_commands("./sipp", specs, 5060, &commands, &error)) << error; + ASSERT_EQ(1u, commands.size()); + ASSERT_EQ(4u, commands[0].argv.size()); + EXPECT_EQ(" spaced role ", commands[0].argv[3]); +} + +TEST(MultiInstanceConfig, ExpandsRoleWithQuoteWithoutResplitting) +{ + const std::string csv = + "role,count,args\n" + "ua's,1,\"-key role {role}\"\n"; + + std::string error; + std::vector specs; + ASSERT_TRUE(parse_multi_instance_csv(csv, "quote.csv", &specs, &error)) << error; + + std::vector commands; + ASSERT_TRUE(build_multi_instance_commands("./sipp", specs, 5060, &commands, &error)) << error; + ASSERT_EQ(1u, commands.size()); + ASSERT_EQ(4u, commands[0].argv.size()); + EXPECT_EQ("ua's", commands[0].argv[3]); +} + +TEST(MultiInstanceConfig, RejectsUnterminatedArgumentQuotes) +{ + const std::string csv = + "role,count,args\n" + "uas,1,\"-sn 'uas\"\n"; + + std::string error; + std::vector specs; + EXPECT_FALSE(parse_multi_instance_csv(csv, "quote.csv", &specs, &error)); + EXPECT_NE(std::string::npos, error.find("unterminated quoted argument")); +} + +TEST(MultiInstanceConfig, RejectsCountOverflowAndExcessiveChildren) +{ + std::string error; + std::vector specs; + + const std::string huge_count = + "role,count,args\n" + "uas,4294967296,\"-sn uas\"\n"; + EXPECT_FALSE(parse_multi_instance_csv(huge_count, "huge.csv", &specs, &error)); + EXPECT_NE(std::string::npos, error.find("count")); + + error.clear(); + const std::string too_many = + "role,count,args\n" + "uas,128,\"-sn uas\"\n" + "uac,129,\"-sn uac 127.0.0.1\"\n"; + EXPECT_FALSE(parse_multi_instance_csv(too_many, "many.csv", &specs, &error)); + EXPECT_NE(std::string::npos, error.find("maximum of 256 child processes")); +} + +TEST(MultiInstanceConfig, RejectsPortAllocationOverflow) +{ + MultiInstanceSpec spec; + spec.role = "uas"; + spec.count = 2; + spec.args = {"-sn", "uas", "-p", "{port}"}; + + std::string error; + std::vector commands; + EXPECT_FALSE(build_multi_instance_commands("./sipp", {spec}, 65535, &commands, &error)); + EXPECT_NE(std::string::npos, error.find("exceeds 65535")); + EXPECT_TRUE(commands.empty()); +} + +TEST(MultiInstanceArgs, BasePortWithoutMultiIsRejected) +{ + MultiInstanceOptions options; + std::string error; + EXPECT_EQ(MultiInstanceArgParseResult::INVALID, + parse_launcher_args({"sipp", "-multi_base_port", "5070"}, &options, &error)); + EXPECT_NE(std::string::npos, error.find("requires -multi")); +} + +TEST(MultiInstanceArgs, RejectsUnrelatedArgumentsInMultiMode) +{ + MultiInstanceOptions options; + std::string error; + EXPECT_EQ(MultiInstanceArgParseResult::INVALID, + parse_launcher_args({"sipp", "-multi", "multi.csv", "-m", "999"}, + &options, + &error)); + EXPECT_NE(std::string::npos, error.find("Unexpected argument")); + EXPECT_NE(std::string::npos, error.find("-m")); +} + +TEST(MultiInstanceArgs, AcceptsOnlyLauncherArguments) +{ + MultiInstanceOptions options; + std::string error; + EXPECT_EQ(MultiInstanceArgParseResult::READY, + parse_launcher_args({"sipp", "--multi_base_port", "5070", "--multi", "multi.csv"}, + &options, + &error)); + EXPECT_EQ("multi.csv", options.config_path); + EXPECT_EQ(5070, options.base_port); +} + +TEST(MultiInstanceArgs, OrdinarySippArgumentsDoNotActivateLauncher) +{ + MultiInstanceOptions options; + std::string error; + EXPECT_EQ(MultiInstanceArgParseResult::NOT_REQUESTED, + parse_launcher_args({"sipp", "-sn", "uas", "-m", "10"}, &options, &error)); +} + +TEST(MultiInstanceArgs, RejectsDuplicateLauncherOptions) +{ + MultiInstanceOptions options; + std::string error; + EXPECT_EQ(MultiInstanceArgParseResult::INVALID, + parse_launcher_args({"sipp", "-multi", "a.csv", "-multi", "b.csv"}, + &options, + &error)); + EXPECT_NE(std::string::npos, error.find("only be specified once")); +} + +TEST(MultiInstanceArgs, ShellSplitterPreservesEmptyQuotedArgument) +{ + std::vector words; + std::string error; + ASSERT_TRUE(split_command_args("-key value \"\" tail", &words, &error)) << error; + EXPECT_EQ(std::vector({"-key", "value", "", "tail"}), words); +} From ffc2f4081a7d0740388913470ac5044595fd8d58 Mon Sep 17 00:00:00 2001 From: Darwvin Date: Thu, 27 Aug 2026 01:04:13 +0330 Subject: [PATCH 3/4] fix: harden multi-instance launcher lifecycle --- docs/multi_instance.rst | 15 ++- include/multi_instance.hpp | 1 + src/multi_instance.cpp | 255 +++++++++++++++++++++++++++++++------ src/sipp_unittest.cpp | 60 +++++++++ 4 files changed, 290 insertions(+), 41 deletions(-) diff --git a/docs/multi_instance.rst b/docs/multi_instance.rst index f8c35cb5e..3d6080500 100644 --- a/docs/multi_instance.rst +++ b/docs/multi_instance.rst @@ -32,10 +32,17 @@ into command-line arguments before placeholders are expanded. This keeps placeholder values containing spaces or quote characters as a single argument instead of re-parsing them as shell syntax. +The launcher-only options ``-multi`` and ``-multi_base_port`` (including their +``--`` forms) are forbidden in child arguments. Validation is performed after +placeholder expansion, so placeholders cannot be used to create a nested +launcher. This prevents recursive configurations from bypassing the per-file +child-process limit. + The following placeholders are expanded in the ``args`` field: * ``{role}``: the role column value. -* ``{instance}``: the zero-based instance number within that role. +* ``{instance}``: the zero-based instance number within that role. If a role + appears in more than one CSV row, numbering continues across those rows. * ``{base_port}``: the value passed with ``-multi_base_port``. * ``{instance_port}``: ``base_port + instance``. Use this to pair UAC and UAS rows by instance number. @@ -47,6 +54,12 @@ exit code. If a child cannot be forked, children already started by the launcher are terminated and reaped before the launcher exits with failure. If all children exit successfully, the launcher exits with zero. +When the launcher receives ``SIGINT``, ``SIGTERM``, or ``SIGHUP``, it forwards +a graceful termination to children, waits briefly, force-terminates any child +that remains, reaps them, and exits with ``128 + signal``. This also prevents +children from being orphaned when the launcher is stopped by a service manager +or CI timeout. + All children inherit the launcher's standard input, output, and error streams. Multiple interactive SIPp screens will therefore interleave on one terminal. Use ``-nostdin`` for children and redirect the launcher's output when a clean diff --git a/include/multi_instance.hpp b/include/multi_instance.hpp index 72c452d2f..a0214356a 100644 --- a/include/multi_instance.hpp +++ b/include/multi_instance.hpp @@ -32,6 +32,7 @@ struct MultiInstanceCommand { std::string role; int instance; int port; + bool uses_port = false; std::string executable_path; std::vector argv; }; diff --git a/src/multi_instance.cpp b/src/multi_instance.cpp index 188e4aa1a..8b0127c18 100644 --- a/src/multi_instance.cpp +++ b/src/multi_instance.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #ifdef __APPLE__ @@ -22,6 +23,7 @@ #include #include #include +#include std::string trim_copy(const std::string &value) { @@ -47,9 +49,15 @@ static bool multi_option_matches(const char *value, const char *name) return false; } - std::string short_option = "-" + std::string(name); - std::string long_option = "--" + std::string(name); - return value == short_option || value == long_option; + const std::string option(value); + return option == "-" + std::string(name) || + option == "--" + std::string(name); +} + +static bool is_launcher_only_argument(const std::string &value) +{ + return value == "-multi" || value == "--multi" || + value == "-multi_base_port" || value == "--multi_base_port"; } static bool parse_port(const std::string &value, int *port) @@ -80,7 +88,8 @@ parse_multi_instance_launcher_args(int argc, bool saw_base_port = false; std::string unexpected_argument; - for (int argi = 1; argi < argc; ++argi) { + int argi = 1; + while (argi < argc) { if (multi_option_matches(argv[argi], "multi")) { if (saw_multi) { *error = "-multi may only be specified once"; @@ -91,8 +100,12 @@ parse_multi_instance_launcher_args(int argc, return MultiInstanceArgParseResult::INVALID; } saw_multi = true; - options->config_path = argv[++argi]; - } else if (multi_option_matches(argv[argi], "multi_base_port")) { + options->config_path = argv[argi + 1]; + argi += 2; + continue; + } + + if (multi_option_matches(argv[argi], "multi_base_port")) { if (saw_base_port) { *error = "-multi_base_port may only be specified once"; return MultiInstanceArgParseResult::INVALID; @@ -102,14 +115,19 @@ parse_multi_instance_launcher_args(int argc, return MultiInstanceArgParseResult::INVALID; } saw_base_port = true; - std::string value = argv[++argi]; + std::string value = argv[argi + 1]; if (!parse_port(value, &options->base_port)) { *error = "Invalid -multi_base_port value: " + value; return MultiInstanceArgParseResult::INVALID; } - } else if (unexpected_argument.empty()) { + argi += 2; + continue; + } + + if (unexpected_argument.empty()) { unexpected_argument = argv[argi]; } + ++argi; } if (!saw_multi && !saw_base_port) { @@ -157,21 +175,23 @@ static bool parse_csv_line(const std::string &line, bool field_was_quoted = false; bool quote_closed = false; - for (size_t i = 0; i < line.size(); ++i) { + size_t i = 0; + while (i < line.size()) { char ch = line[i]; if (in_quotes) { if (ch == '"') { if ((i + 1 < line.size()) && line[i + 1] == '"') { current.push_back('"'); - ++i; - } else { - in_quotes = false; - quote_closed = true; + i += 2; + continue; } + in_quotes = false; + quote_closed = true; } else { current.push_back(ch); } + ++i; continue; } @@ -184,6 +204,7 @@ static bool parse_csv_line(const std::string &line, *error = "unexpected character after quoted CSV field"; return false; } + ++i; continue; } @@ -201,6 +222,7 @@ static bool parse_csv_line(const std::string &line, } else { current.push_back(ch); } + ++i; } if (in_quotes) { @@ -310,8 +332,6 @@ static bool canonical_regular_file_path(const std::string &path, return true; } -#ifndef __linux__ -#ifndef __APPLE__ static std::string resolve_from_argv0(const char *argv0) { if (!argv0 || !argv0[0]) { @@ -351,8 +371,6 @@ static std::string resolve_from_argv0(const char *argv0) return ""; } -#endif -#endif std::string resolve_current_executable_path(const char *argv0) { @@ -363,7 +381,7 @@ std::string resolve_current_executable_path(const char *argv0) path[length] = '\0'; return path; } - return ""; + return resolve_from_argv0(argv0); #elif defined(__APPLE__) char path[PATH_MAX]; uint32_t size = sizeof(path); @@ -374,7 +392,7 @@ std::string resolve_current_executable_path(const char *argv0) } return path; } - return ""; + return resolve_from_argv0(argv0); #else return resolve_from_argv0(argv0); #endif @@ -515,6 +533,8 @@ bool build_multi_instance_commands(const std::string &program_path, return false; } + std::unordered_map next_instance_by_role; + for (const MultiInstanceSpec &spec : specs) { if (spec.count <= 0 || commands->size() + static_cast(spec.count) > @@ -525,7 +545,9 @@ bool build_multi_instance_commands(const std::string &program_path, return false; } - for (int instance = 0; instance < spec.count; ++instance) { + int &next_role_instance = next_instance_by_role[spec.role]; + for (int row_instance = 0; row_instance < spec.count; ++row_instance) { + const int instance = next_role_instance++; long long next_port = static_cast(base_port) + static_cast(commands->size()); long long instance_port = static_cast(base_port) + instance; @@ -547,10 +569,23 @@ bool build_multi_instance_commands(const std::string &program_path, replace_all(&word, "{instance}", std::to_string(instance)); replace_all(&word, "{instance_port}", std::to_string(instance_port)); replace_all(&word, "{base_port}", std::to_string(base_port)); + if (word.find("{port}") != std::string::npos) { + command.uses_port = true; + } replace_all(&word, "{port}", std::to_string(next_port)); command.argv.push_back(word); } + for (size_t argi = 1; argi < command.argv.size(); ++argi) { + if (is_launcher_only_argument(command.argv[argi])) { + *error = "launcher-only option " + command.argv[argi] + + " is not allowed in child arguments for role " + + spec.role + " instance " + std::to_string(instance); + commands->clear(); + return false; + } + } + commands->push_back(command); } } @@ -558,25 +593,136 @@ bool build_multi_instance_commands(const std::string &program_path, return true; } -static void kill_and_reap_children(const std::vector &children, - std::ostream &err) +static volatile sig_atomic_t multi_shutdown_signal = 0; + +static void multi_instance_signal_handler(int signal_number) +{ + multi_shutdown_signal = signal_number; +} + +struct SavedSignalHandlers { + struct sigaction sigint_action; + struct sigaction sigterm_action; + struct sigaction sighup_action; + bool sigint_saved = false; + bool sigterm_saved = false; + bool sighup_saved = false; +}; + +static void restore_signal_handlers(const SavedSignalHandlers &saved) +{ + if (saved.sigint_saved) { + sigaction(SIGINT, &saved.sigint_action, nullptr); + } + if (saved.sigterm_saved) { + sigaction(SIGTERM, &saved.sigterm_action, nullptr); + } + if (saved.sighup_saved) { + sigaction(SIGHUP, &saved.sighup_action, nullptr); + } +} + +static bool install_signal_handlers(SavedSignalHandlers *saved, + std::ostream &err) +{ + struct sigaction action; + memset(&action, 0, sizeof(action)); + action.sa_handler = multi_instance_signal_handler; + sigemptyset(&action.sa_mask); + + if (sigaction(SIGINT, &action, &saved->sigint_action) != 0) { + err << "sigaction failed for SIGINT: " << strerror(errno) << "\n"; + return false; + } + saved->sigint_saved = true; + + if (sigaction(SIGTERM, &action, &saved->sigterm_action) != 0) { + err << "sigaction failed for SIGTERM: " << strerror(errno) << "\n"; + restore_signal_handlers(*saved); + return false; + } + saved->sigterm_saved = true; + + if (sigaction(SIGHUP, &action, &saved->sighup_action) != 0) { + err << "sigaction failed for SIGHUP: " << strerror(errno) << "\n"; + restore_signal_handlers(*saved); + return false; + } + saved->sighup_saved = true; + return true; +} + +static void send_signal_to_children(const std::vector &children, + int signal_number, + std::ostream &err) { for (pid_t child : children) { - if (kill(child, SIGKILL) != 0 && errno != ESRCH) { - err << "failed to terminate child " << child << ": " << strerror(errno) << "\n"; + if (kill(child, signal_number) != 0 && errno != ESRCH) { + err << "failed to signal child " << child << ": " << strerror(errno) << "\n"; } } +} - for (pid_t child : children) { - int status = 0; - while (waitpid(child, &status, 0) < 0) { - if (errno == EINTR) { - continue; +static void terminate_and_reap_children(std::vector *children, + std::ostream &err) +{ + if (children->empty()) { + return; + } + + send_signal_to_children(*children, SIGTERM, err); + + const struct timespec pause_time = {0, 100000000}; + for (int attempt = 0; attempt < 10 && !children->empty(); ++attempt) { + std::vector remaining; + remaining.reserve(children->size()); + + for (pid_t child : *children) { + int status = 0; + pid_t waited = waitpid(child, &status, WNOHANG); + if (waited == 0) { + remaining.push_back(child); + } else if (waited < 0 && errno != ECHILD) { + if (errno != EINTR) { + err << "waitpid failed for " << child << ": " + << strerror(errno) << "\n"; + } + remaining.push_back(child); } - if (errno != ECHILD) { - err << "waitpid failed for " << child << ": " << strerror(errno) << "\n"; + } + + children->swap(remaining); + if (!children->empty()) { + struct timespec remaining_pause = pause_time; + while (nanosleep(&remaining_pause, &remaining_pause) != 0 && errno == EINTR) { } - break; + } + } + + if (!children->empty()) { + send_signal_to_children(*children, SIGKILL, err); + } + + for (pid_t child : *children) { + int status = 0; + pid_t waited; + do { + waited = waitpid(child, &status, 0); + } while (waited < 0 && errno == EINTR); + + if (waited < 0 && errno != ECHILD) { + err << "waitpid failed for " << child << ": " << strerror(errno) << "\n"; + } + } + children->clear(); +} + +static void erase_child(std::vector *children, pid_t child) +{ + for (auto it = children->begin(); it != children->end(); ++it) { + if (*it == child) { + children->erase(it); + return; } } } @@ -587,10 +733,23 @@ int run_multi_instance_commands(const std::vector &command { std::vector children; int exit_code = 0; + multi_shutdown_signal = 0; + + SavedSignalHandlers saved_handlers; + if (!install_signal_handlers(&saved_handlers, err)) { + return 1; + } for (const MultiInstanceCommand &command : commands) { + if (multi_shutdown_signal != 0) { + int signal_number = multi_shutdown_signal; + terminate_and_reap_children(&children, err); + restore_signal_handlers(saved_handlers); + return 128 + signal_number; + } + out << "Starting " << command.role << "[" << command.instance << "]"; - if (command.port > 0) { + if (command.uses_port && command.port > 0) { out << " port=" << command.port; } out << ":"; @@ -603,10 +762,13 @@ int run_multi_instance_commands(const std::vector &command pid_t child = fork(); if (child < 0) { err << "fork failed: " << strerror(errno) << "\n"; - kill_and_reap_children(children, err); + terminate_and_reap_children(&children, err); + restore_signal_handlers(saved_handlers); return 1; } if (child == 0) { + restore_signal_handlers(saved_handlers); + std::vector argv; argv.reserve(command.argv.size() + 1); for (const std::string &arg : command.argv) { @@ -621,19 +783,28 @@ int run_multi_instance_commands(const std::vector &command children.push_back(child); } - for (pid_t child : children) { - int status = 0; - pid_t waited; - do { - waited = waitpid(child, &status, 0); - } while (waited < 0 && errno == EINTR); + while (!children.empty()) { + if (multi_shutdown_signal != 0) { + int signal_number = multi_shutdown_signal; + terminate_and_reap_children(&children, err); + restore_signal_handlers(saved_handlers); + return 128 + signal_number; + } + pid_t child = children.front(); + int status = 0; + pid_t waited = waitpid(child, &status, 0); if (waited < 0) { + if (errno == EINTR) { + continue; + } err << "waitpid failed for " << child << ": " << strerror(errno) << "\n"; + erase_child(&children, child); exit_code = 1; continue; } + erase_child(&children, child); if (WIFEXITED(status)) { int child_exit = WEXITSTATUS(status); if (child_exit != 0 && exit_code == 0) { @@ -647,5 +818,9 @@ int run_multi_instance_commands(const std::vector &command } } + if (multi_shutdown_signal != 0) { + exit_code = 128 + multi_shutdown_signal; + } + restore_signal_handlers(saved_handlers); return exit_code; } diff --git a/src/sipp_unittest.cpp b/src/sipp_unittest.cpp index 4e59b862e..684e0a63f 100644 --- a/src/sipp_unittest.cpp +++ b/src/sipp_unittest.cpp @@ -200,6 +200,66 @@ TEST(MultiInstanceConfig, RejectsPortAllocationOverflow) EXPECT_TRUE(commands.empty()); } +TEST(MultiInstanceConfig, RejectsNestedLauncherOptionsAfterExpansion) +{ + std::string error; + std::vector specs; + std::vector commands; + + const std::string direct = + "role,count,args\n" + "rec,2,\"-multi rec.csv\"\n"; + ASSERT_TRUE(parse_multi_instance_csv(direct, "direct.csv", &specs, &error)) << error; + EXPECT_FALSE(build_multi_instance_commands("./sipp", specs, 5060, &commands, &error)); + EXPECT_NE(std::string::npos, error.find("launcher-only option -multi")); + + error.clear(); + const std::string via_placeholder = + "role,count,args\n" + "-multi,2,\"{role} rec.csv\"\n"; + ASSERT_TRUE(parse_multi_instance_csv(via_placeholder, "placeholder.csv", &specs, &error)) << error; + EXPECT_FALSE(build_multi_instance_commands("./sipp", specs, 5060, &commands, &error)); + EXPECT_NE(std::string::npos, error.find("launcher-only option -multi")); +} + +TEST(MultiInstanceConfig, ContinuesInstanceNumbersAcrossRepeatedRoleRows) +{ + const std::string csv = + "role,count,args\n" + "uas,1,\"-sn uas -p {instance_port}\"\n" + "uas,1,\"-sn uas -p {instance_port}\"\n"; + + std::string error; + std::vector specs; + ASSERT_TRUE(parse_multi_instance_csv(csv, "repeated.csv", &specs, &error)) << error; + + std::vector commands; + ASSERT_TRUE(build_multi_instance_commands("./sipp", specs, 5060, &commands, &error)) << error; + ASSERT_EQ(2u, commands.size()); + EXPECT_EQ(0, commands[0].instance); + EXPECT_EQ(1, commands[1].instance); + EXPECT_EQ("5060", commands[0].argv.back()); + EXPECT_EQ("5061", commands[1].argv.back()); +} + +TEST(MultiInstanceConfig, TracksGlobalPortPlaceholderUsage) +{ + const std::string csv = + "role,count,args\n" + "uas,1,\"-sn uas -p {instance_port}\"\n" + "uac,1,\"-sn uac -p {port}\"\n"; + + std::string error; + std::vector specs; + ASSERT_TRUE(parse_multi_instance_csv(csv, "ports.csv", &specs, &error)) << error; + + std::vector commands; + ASSERT_TRUE(build_multi_instance_commands("./sipp", specs, 5060, &commands, &error)) << error; + ASSERT_EQ(2u, commands.size()); + EXPECT_FALSE(commands[0].uses_port); + EXPECT_TRUE(commands[1].uses_port); +} + TEST(MultiInstanceArgs, BasePortWithoutMultiIsRejected) { MultiInstanceOptions options; From 833136aca4425a54950b4be32eaa0442066e074f Mon Sep 17 00:00:00 2001 From: Darwvin Date: Thu, 27 Aug 2026 01:33:37 +0330 Subject: [PATCH 4/4] fix: close multi-instance executable trust boundary --- docs/multi_instance.rst | 6 +++ src/multi_instance.cpp | 108 ++++++++++++++++++++++++---------------- src/sipp_unittest.cpp | 16 ++++++ 3 files changed, 88 insertions(+), 42 deletions(-) diff --git a/docs/multi_instance.rst b/docs/multi_instance.rst index 3d6080500..7ddb33096 100644 --- a/docs/multi_instance.rst +++ b/docs/multi_instance.rst @@ -60,6 +60,12 @@ that remains, reaps them, and exits with ``128 + signal``. This also prevents children from being orphaned when the launcher is stopped by a service manager or CI timeout. +Child processes always execute the same SIPp process image as the launcher. +The executable path is resolved from operating-system process metadata rather +than caller-controlled ``argv[0]`` or CSV data. If the current executable +cannot be resolved safely, launcher mode fails closed instead of executing a +fallback path. + All children inherit the launcher's standard input, output, and error streams. Multiple interactive SIPp screens will therefore interleave on one terminal. Use ``-nostdin`` for children and redirect the launcher's output when a clean diff --git a/src/multi_instance.cpp b/src/multi_instance.cpp index 8b0127c18..3232199f7 100644 --- a/src/multi_instance.cpp +++ b/src/multi_instance.cpp @@ -19,6 +19,10 @@ #include #endif +#ifdef __FreeBSD__ +#include +#endif + #include #include #include @@ -332,69 +336,71 @@ static bool canonical_regular_file_path(const std::string &path, return true; } -static std::string resolve_from_argv0(const char *argv0) +static std::string canonical_executable_path(const char *path) { - if (!argv0 || !argv0[0]) { + if (!path || !path[0]) { return ""; } char resolved[PATH_MAX]; - if (realpath(argv0, resolved)) { - return resolved; - } - - if (strchr(argv0, '/')) { - return argv0; - } - - const char *path_value = getenv("PATH"); - if (!path_value) { + if (!realpath(path, resolved)) { return ""; } - std::istringstream paths(path_value); - std::string directory; - while (std::getline(paths, directory, ':')) { - if (directory.empty()) { - directory = "."; - } - std::string candidate = directory + "/" + argv0; - struct stat st; - if (stat(candidate.c_str(), &st) == 0 && S_ISREG(st.st_mode) && - access(candidate.c_str(), X_OK) == 0) { - if (realpath(candidate.c_str(), resolved)) { - return resolved; - } - return candidate; - } + struct stat st; + if (stat(resolved, &st) != 0 || !S_ISREG(st.st_mode) || + access(resolved, X_OK) != 0) { + return ""; } - return ""; + return resolved; } std::string resolve_current_executable_path(const char *argv0) { + /* argv[0] is caller-controlled and must never select the executable used + * by the launcher. Keep the parameter for source compatibility, but + * resolve the current process image only through OS-provided mechanisms. */ + (void)argv0; + #ifdef __linux__ char path[PATH_MAX]; ssize_t length = readlink("/proc/self/exe", path, sizeof(path) - 1); - if (length > 0) { - path[length] = '\0'; - return path; + if (length <= 0) { + return ""; } - return resolve_from_argv0(argv0); + path[length] = '\0'; + return canonical_executable_path(path); #elif defined(__APPLE__) char path[PATH_MAX]; uint32_t size = sizeof(path); - if (_NSGetExecutablePath(path, &size) == 0) { - char resolved[PATH_MAX]; - if (realpath(path, resolved)) { - return resolved; - } - return path; + if (_NSGetExecutablePath(path, &size) != 0) { + return ""; + } + return canonical_executable_path(path); +#elif defined(__FreeBSD__) +#ifdef KERN_PROC_PATHNAME + char path[PATH_MAX]; + size_t size = sizeof(path); + int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1}; + if (sysctl(mib, 4, path, &size, nullptr, 0) != 0 || size == 0) { + return ""; } - return resolve_from_argv0(argv0); + path[sizeof(path) - 1] = '\0'; + return canonical_executable_path(path); #else - return resolve_from_argv0(argv0); + return ""; +#endif +#elif defined(__sun) + char path[PATH_MAX]; + ssize_t length = readlink("/proc/self/path/a.out", path, sizeof(path) - 1); + if (length <= 0) { + return ""; + } + path[length] = '\0'; + return canonical_executable_path(path); +#else + return ""; #endif } @@ -735,6 +741,16 @@ int run_multi_instance_commands(const std::vector &command int exit_code = 0; multi_shutdown_signal = 0; + /* Never trust MultiInstanceCommand::executable_path at the exec boundary. + * Resolve the current process image independently so caller-controlled + * argv[0] or command data cannot select a different executable. */ + const std::string trusted_executable_path = + resolve_current_executable_path(nullptr); + if (trusted_executable_path.empty()) { + err << "unable to resolve trusted SIPp executable path\n"; + return 1; + } + SavedSignalHandlers saved_handlers; if (!install_signal_handlers(&saved_handlers, err)) { return 1; @@ -748,6 +764,13 @@ int run_multi_instance_commands(const std::vector &command return 128 + signal_number; } + if (command.argv.empty()) { + err << "multi-instance child command has an empty argv\n"; + terminate_and_reap_children(&children, err); + restore_signal_handlers(saved_handlers); + return 1; + } + out << "Starting " << command.role << "[" << command.instance << "]"; if (command.uses_port && command.port > 0) { out << " port=" << command.port; @@ -774,9 +797,10 @@ int run_multi_instance_commands(const std::vector &command for (const std::string &arg : command.argv) { argv.push_back(const_cast(arg.c_str())); } + argv[0] = const_cast(trusted_executable_path.c_str()); argv.push_back(nullptr); - execv(command.executable_path.c_str(), argv.data()); - std::cerr << "exec failed for " << command.executable_path << ": " + execv(trusted_executable_path.c_str(), argv.data()); + std::cerr << "exec failed for " << trusted_executable_path << ": " << strerror(errno) << "\n"; _exit(127); } diff --git a/src/sipp_unittest.cpp b/src/sipp_unittest.cpp index 684e0a63f..f5ae9eec9 100644 --- a/src/sipp_unittest.cpp +++ b/src/sipp_unittest.cpp @@ -319,3 +319,19 @@ TEST(MultiInstanceArgs, ShellSplitterPreservesEmptyQuotedArgument) ASSERT_TRUE(split_command_args("-key value \"\" tail", &words, &error)) << error; EXPECT_EQ(std::vector({"-key", "value", "", "tail"}), words); } + +#if defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__sun) +TEST(MultiInstanceExecutable, IgnoresCallerControlledArgv0) +{ + const std::string attacker_path = "/tmp/attacker-controlled-sipp"; + const std::string other_path = "/tmp/another-attacker-controlled-sipp"; + + const std::string resolved = resolve_current_executable_path(attacker_path.c_str()); + const std::string resolved_again = resolve_current_executable_path(other_path.c_str()); + + ASSERT_FALSE(resolved.empty()); + EXPECT_EQ(resolved, resolved_again); + EXPECT_NE(attacker_path, resolved); + EXPECT_NE(other_path, resolved); +} +#endif