Skip to content

Commit

Permalink
cleanup(libsinsp): throw exception for invalid parsed string vectors
Browse files Browse the repository at this point in the history
Signed-off-by: Luca Guerra <[email protected]>
  • Loading branch information
LucaGuerra authored and poiana committed Apr 23, 2024
1 parent 3793b10 commit 739d3b9
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 61 deletions.
7 changes: 7 additions & 0 deletions userspace/libsinsp/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ limitations under the License.
#include <libsinsp/settings.h>
#include <libsinsp/sinsp_exception.h>
#include <libsinsp/fdinfo.h>
#include <libsinsp/utils.h>

class sinsp;
class sinsp_threadinfo;
Expand Down Expand Up @@ -150,6 +151,12 @@ inline std::string_view get_event_param_as<std::string_view>(const sinsp_evt_par
return {param.m_val, string_len};
}

template<>
inline std::vector<std::string> get_event_param_as<std::vector<std::string>>(const sinsp_evt_param& param)
{
return sinsp_split(param.m_val, static_cast<size_t>(param.m_len), '\0');
}

/*!
\brief Event class.
This class is returned by \ref sinsp::next() and encapsulates the state
Expand Down
26 changes: 8 additions & 18 deletions userspace/libsinsp/parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,6 @@ bool sinsp_parser::retrieve_enter_event(sinsp_evt *enter_evt, sinsp_evt *exit_ev

void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
{
const sinsp_evt_param* parinfo = nullptr;
uint16_t etype = evt->get_type();
int64_t caller_tid = evt->get_tid();

Expand Down Expand Up @@ -1229,8 +1228,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
child_tinfo->m_exe = evt->get_param(1)->as<std::string_view>();

/* args */
parinfo = evt->get_param(2);
child_tinfo->set_args(parinfo->m_val, parinfo->m_len);
child_tinfo->set_args(evt->get_param(2)->as<std::vector<std::string>>());

/* comm */
switch(etype)
Expand Down Expand Up @@ -1354,8 +1352,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
case PPME_SYSCALL_VFORK_20_X:
case PPME_SYSCALL_CLONE_20_X:
case PPME_SYSCALL_CLONE3_X:
parinfo = evt->get_param(14);
child_tinfo->set_cgroups(parinfo->m_val, parinfo->m_len);
child_tinfo->set_cgroups(evt->get_param(14)->as<std::vector<std::string>>());
m_inspector->m_container_manager.resolve_container(child_tinfo.get(), m_inspector->is_live() || m_inspector->is_syscall_plugin());
break;
}
Expand Down Expand Up @@ -1411,8 +1408,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
caller_tinfo->m_comm = child_tinfo->m_comm;

/* args */
parinfo = evt->get_param(2);
caller_tinfo->set_args(parinfo->m_val, parinfo->m_len);
caller_tinfo->set_args(evt->get_param(2)->as<std::vector<std::string>>());
}

/*=============================== CREATE CHILD ===========================*/
Expand Down Expand Up @@ -1459,7 +1455,6 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)

void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt)
{
const sinsp_evt_param *parinfo = nullptr;
uint16_t etype = evt->get_type();
int64_t child_tid = evt->get_tid();

Expand Down Expand Up @@ -1688,8 +1683,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt)
}

/* args */
parinfo = evt->get_param(2);
child_tinfo->set_args(parinfo->m_val, parinfo->m_len);
child_tinfo->set_args(evt->get_param(2)->as<std::vector<std::string>>());

if(valid_lookup_thread)
{
Expand Down Expand Up @@ -1799,8 +1793,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt)
lookup_tinfo->m_comm = child_tinfo->m_comm;

/* args */
parinfo = evt->get_param(2);
lookup_tinfo->set_args(parinfo->m_val, parinfo->m_len);
lookup_tinfo->set_args(evt->get_param(2)->as<std::vector<std::string>>());
}
}

Expand Down Expand Up @@ -1904,8 +1897,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt)
case PPME_SYSCALL_VFORK_20_X:
case PPME_SYSCALL_CLONE_20_X:
case PPME_SYSCALL_CLONE3_X:
parinfo = evt->get_param(14);
child_tinfo->set_cgroups(parinfo->m_val, parinfo->m_len);
child_tinfo->set_cgroups(evt->get_param(14)->as<std::vector<std::string>>());
m_inspector->m_container_manager.resolve_container(child_tinfo.get(), m_inspector->is_live());
break;
}
Expand Down Expand Up @@ -2080,8 +2072,7 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt)
}

// Get the command arguments
parinfo = evt->get_param(2);
evt->get_tinfo()->set_args(parinfo->m_val, parinfo->m_len);
evt->get_tinfo()->set_args(evt->get_param(2)->as<std::vector<std::string>>());

// Get the pid
evt->get_tinfo()->m_pid = evt->get_param(4)->as<uint64_t>();
Expand Down Expand Up @@ -2168,8 +2159,7 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt)
//
// Set cgroups and heuristically detect container id
//
parinfo = evt->get_param(14);
evt->get_tinfo()->set_cgroups(parinfo->m_val, parinfo->m_len);
evt->get_tinfo()->set_cgroups(evt->get_param(14)->as<std::vector<std::string>>());

//
// Resync container status after an execve, we need to do it
Expand Down
11 changes: 11 additions & 0 deletions userspace/libsinsp/test/sinsp_utils.ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,14 @@ TEST(sinsp_utils_test, concatenate_paths)
res = sinsp_utils::concatenate_paths(path1, path2);
EXPECT_EQ("/root/c:/hello/world", res); */
}

TEST(sinsp_utils_test, sinsp_split_buf)
{
const char *in = "hello\0world\0";
size_t len = 12;
auto split = sinsp_split(in, len, '\0');

EXPECT_EQ(split.size(), 2);
EXPECT_EQ(split[0], "hello");
EXPECT_EQ(split[1], "world");
}
59 changes: 16 additions & 43 deletions userspace/libsinsp/threadinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -616,14 +616,12 @@ std::string sinsp_threadinfo::get_exepath() const

void sinsp_threadinfo::set_args(const char* args, size_t len)
{
m_args.clear();
set_args(sinsp_split(args, len, '\0'));
}

size_t offset = 0;
while(offset < len)
{
m_args.push_back(args + offset);
offset += m_args.back().length() + 1;
}
void sinsp_threadinfo::set_args(std::vector<std::string> args)
{
m_args = args;
}

void sinsp_threadinfo::set_env(const char* env, size_t len)
Expand All @@ -641,32 +639,7 @@ void sinsp_threadinfo::set_env(const char* env, size_t len)
}
}

m_env.clear();
size_t offset = 0;
while(offset < len)
{
const char* left = env + offset;
// environment string may actually be shorter than indicated by len
// if the rest is empty, we bail out early
if(!strlen(left))
{
size_t sz = len - offset;
void* zero = calloc(sz, sizeof(char));
if(zero == NULL)
{
throw sinsp_exception("memory allocation error in sinsp_threadinfo::set_env");
}
if(!memcmp(left, zero, sz))
{
free(zero);
return;
}
free(zero);
}
m_env.push_back(left);

offset += m_env.back().length() + 1;
}
m_env = sinsp_split(env, len, '\0');
}

bool sinsp_threadinfo::set_env_from_proc() {
Expand Down Expand Up @@ -756,24 +729,25 @@ std::string sinsp_threadinfo::concatenate_all_env()
}

void sinsp_threadinfo::set_cgroups(const char* cgroups, size_t len)
{
set_cgroups(sinsp_split(cgroups, len, '\0'));
}

void sinsp_threadinfo::set_cgroups(std::vector<std::string> cgroups)
{
decltype(m_cgroups) tmp_cgroups(new cgroups_t);

size_t offset = 0;
while(offset < len)
for( auto &def : cgroups)
{
const char* str = cgroups + offset;
const char* sep = strrchr(str, '=');
if(sep == NULL)
std::string::size_type eq_pos = def.find("=");
if (eq_pos == std::string::npos)
{
ASSERT(false);
return;
}

std::string subsys(str, sep - str);
std::string cgroup(sep + 1);
std::string subsys = def.substr(0, eq_pos);
std::string cgroup = def.substr(eq_pos + 1);

size_t subsys_length = subsys.length();
size_t pos = subsys.find("_cgroup");
if(pos != std::string::npos)
{
Expand All @@ -797,7 +771,6 @@ void sinsp_threadinfo::set_cgroups(const char* cgroups, size_t len)
}

tmp_cgroups->push_back(std::make_pair(subsys, cgroup));
offset += subsys_length + 1 + cgroup.length() + 1;
}

m_cgroups.swap(tmp_cgroups);
Expand Down
2 changes: 2 additions & 0 deletions userspace/libsinsp/threadinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -638,8 +638,10 @@ class SINSP_PUBLIC sinsp_threadinfo : public libsinsp::state::table_entry
void remove_fd(int64_t fd);
void update_cwd(std::string_view cwd);
void set_args(const char* args, size_t len);
void set_args(std::vector<std::string> args);
void set_env(const char* env, size_t len);
void set_cgroups(const char* cgroups, size_t len);
void set_cgroups(std::vector<std::string> cgroups);
bool is_lastevent_data_valid() const;
inline void set_lastevent_data_validity(bool isvalid)
{
Expand Down
18 changes: 18 additions & 0 deletions userspace/libsinsp/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1385,6 +1385,24 @@ std::vector<std::string> sinsp_split(const std::string &s, char delim)
return res;
}

std::vector<std::string> sinsp_split(const char *buf, size_t len, char delim)
{
if(len == 0)
{
return {};
}

if(buf[len - 1] != '\0')
{
throw sinsp_exception("expected a NUL-terminated buffer of size " +
std::to_string(len) + ", which instead ends with " +
std::to_string(buf[len - 1]));
}

std::string s {buf, len - 1};
return sinsp_split(s, delim);
}

//
// trim from start
//
Expand Down
2 changes: 2 additions & 0 deletions userspace/libsinsp/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ limitations under the License.

#include <algorithm>
#include <cctype>
#include <cstddef>
#include <cstring>
#include <list>
#include <locale>
Expand Down Expand Up @@ -257,6 +258,7 @@ const char* print_format_to_string(ppm_print_format fmt);
// String helpers
///////////////////////////////////////////////////////////////////////////////
std::vector<std::string> sinsp_split(const std::string& s, char delim);
std::vector<std::string> sinsp_split(const char *buf, size_t len, char delim);

template<typename It>
std::string sinsp_join(It begin, It end, char delim)
Expand Down

0 comments on commit 739d3b9

Please sign in to comment.