Skip to content

Commit

Permalink
fix(userspace/libsinsp): fix a possible source of use-after-free.
Browse files Browse the repository at this point in the history
Basically, `scap_get_fdlist()` called in the sinsp parsers loop to manage SCM_RIGHTS flag for recvmsg,
updates the pointers in the thread fdtable.
After #1637, doing so invalidates `event` m_fdinfo field, that is a pointer to a now freed data.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
  • Loading branch information
FedeDP authored and poiana committed Mar 22, 2024
1 parent 7de3404 commit 94ee389
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 1 deletion.
2 changes: 2 additions & 0 deletions userspace/libsinsp/fdinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ sinsp_fdinfo* sinsp_fdtable::find(int64_t fd)

sinsp_fdinfo* sinsp_fdtable::add(int64_t fd, std::unique_ptr<sinsp_fdinfo> fdinfo)
{
fdinfo->m_fd = fd;

//
// Look for the FD in the table
//
Expand Down
1 change: 1 addition & 0 deletions userspace/libsinsp/fdinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ class SINSP_PUBLIC sinsp_fdinfo
uint32_t m_mount_id = 0;
uint64_t m_ino = 0;
int64_t m_pid = 0; // only if fd is a pidfd
int64_t m_fd = -1;
};

/*@}*/
Expand Down
13 changes: 13 additions & 0 deletions userspace/libsinsp/parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4172,13 +4172,26 @@ void sinsp_parser::parse_rw_exit(sinsp_evt *evt)

m_inspector->m_thread_manager->thread_to_scap(*evt->get_tinfo(), &scap_tinfo);

// Store current fd; it might get changed by scap_get_fdlist below.
int64_t fd = -1;
if (evt->get_fd_info())
{
fd = evt->get_fd_info()->m_fd;
}

// Get the new fds. The callbacks we have registered populate the fd table
// with the new file descriptors.
if (scap_get_fdlist(m_inspector->get_scap_platform(), &scap_tinfo, error) != SCAP_SUCCESS)
{
libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, "scap_get_fdlist failed: %s, proc table will not be updated with new fds.",
error);
}

// Force refresh event fdinfo
if (fd != -1)
{
evt->set_fd_info(evt->get_tinfo()->get_fd(fd));
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion userspace/libsinsp/threadinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ void sinsp_threadinfo::add_fd_from_scap(scap_fdinfo *fdi)
newfdi->m_type = fdi->type;
newfdi->m_flags = sinsp_fdinfo::FLAGS_FROM_PROC;
newfdi->m_ino = fdi->ino;
newfdi->m_fd = fdi->fd;

switch(newfdi->m_type)
{
Expand Down Expand Up @@ -1316,6 +1317,7 @@ static void fd_to_scap(scap_fdinfo *dst, sinsp_fdinfo* src)
{
dst->type = src->m_type;
dst->ino = src->m_ino;
dst->fd = src->m_fd;

switch(dst->type)
{
Expand Down Expand Up @@ -1992,7 +1994,6 @@ void sinsp_thread_manager::dump_threads_to_file(scap_dumper_t* dumper)
//
// Populate the fd info
//
scfdinfo->fd = fd;
fd_to_scap(scfdinfo, &info);

//
Expand Down

0 comments on commit 94ee389

Please sign in to comment.