Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup(scap,sinsp): Clean up proc callback handling code #1471

Merged
merged 24 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
94ac9db
doc(scap): remove misleading comment
gnosek Nov 4, 2023
b8cac94
cleanup(scap): avoid allocations in scap_proc_scan_vtable
gnosek Nov 4, 2023
0dc920a
cleanup(scap): handle allocations internally in scap_add_fd_to_proc_t…
gnosek Nov 4, 2023
ac864f0
cleanup(scap): inline scap_fd_scan_vtable
gnosek Nov 4, 2023
f93c240
cleanup(scap): delay allocations in scap_proc_add_from_proc
gnosek Nov 4, 2023
10fea48
cleanup(scap): only touch the internal fdlist when proc callback is n…
gnosek Nov 4, 2023
29b5367
cleanup(scap): remove fdinfo alloc/free functions
gnosek Nov 4, 2023
be1dd84
cleanup(scap): extend proc_callback type for future expansion
gnosek Nov 4, 2023
2aab28a
cleanup(scap): introduce default proc callback
gnosek Nov 4, 2023
56c8f74
cleanup(scap): introduce helper for initializing the proclist
gnosek Nov 4, 2023
06e6c3c
cleanup(scap): use default proc callback instead of NULL checks
gnosek Nov 4, 2023
1147ab9
cleanup(scap): inline scap_add_fd_to_proc_table
gnosek Nov 4, 2023
2b05f1f
cleanup(scap): remove some unused includes
gnosek Nov 4, 2023
b37a50a
cleanup(scap): flatten scap_proc_scan_vtable a bit
gnosek Nov 4, 2023
84c5ddb
cleanup(scap): remove procinfo from scap_proc_add_from_proc
gnosek Nov 4, 2023
17c3be9
cleanup(scap): remove proclist from scap_proc_read_thread
gnosek Nov 4, 2023
93ed318
cleanup(scap): remove proclist from vtable->get_proc
gnosek Nov 4, 2023
7aa434c
cleanup(scap): remove scap_t from scap_proc_free
gnosek Sep 10, 2023
a0dfaad
cleanup(scap): remove scap_t from scap_fd_add
gnosek Sep 10, 2023
f38a596
cleanup(scap): remove scap_proc_alloc
gnosek Sep 10, 2023
b5db5a0
cleanup(scap,sinp): do not allocate scap_threadinfo for scap_proc_get
gnosek Oct 17, 2023
db81cce
cleanup(scap): remove scap_proc_free
gnosek Oct 17, 2023
14ef2c4
cleanup(scap): remove unused parameter from scap_fd_add
gnosek Nov 7, 2023
d440f43
cleanup(scap): move scap_alloc_proclist_info to scap_procs.c
gnosek Oct 17, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions userspace/libscap/engine/gvisor/gvisor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,7 @@ scap_platform* scap_gvisor_alloc_platform(proc_entry_callback proc_callback, voi
(struct scap_gvisor_platform*)calloc(sizeof(*platform), 1);
platform->m_generic.m_vtable = &scap_gvisor_platform_vtable;

platform->m_generic.m_proclist.m_proc_callback = proc_callback;
platform->m_generic.m_proclist.m_proc_callback_context = proc_callback_context;
platform->m_generic.m_proclist.m_proclist = NULL;
init_proclist(&platform->m_generic.m_proclist, proc_callback, proc_callback_context);

return &platform->m_generic;
}
Expand Down
91 changes: 3 additions & 88 deletions userspace/libscap/engine/savefile/scap_savefile.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ static int32_t scap_read_proclist(scap_reader_t* r, uint32_t block_length, uint3
size_t padding_len;
uint16_t stlen;
uint32_t padding;
int32_t uth_status = SCAP_SUCCESS;
uint32_t toread;
int fseekres;

Expand Down Expand Up @@ -712,34 +711,7 @@ static int32_t scap_read_proclist(scap_reader_t* r, uint32_t block_length, uint3
//
// All parsed. Add the entry to the table, or fire the notification callback
//
if(proclist->m_proc_callback == NULL)
{
//
// All parsed. Allocate the new entry and copy the temp one into into it.
//
struct scap_threadinfo *ntinfo = (scap_threadinfo *)malloc(sizeof(scap_threadinfo));
if(ntinfo == NULL)
{
snprintf(error, SCAP_LASTERR_SIZE, "process table allocation error (fd1)");
return SCAP_FAILURE;
}

// Structure copy
*ntinfo = tinfo;

HASH_ADD_INT64(proclist->m_proclist, tid, ntinfo);
if(uth_status != SCAP_SUCCESS)
{
free(ntinfo);
snprintf(error, SCAP_LASTERR_SIZE, "process table allocation error (fd2)");
return SCAP_FAILURE;
}
}
else
{
proclist->m_proc_callback(
proclist->m_proc_callback_context, tinfo.tid, &tinfo, NULL);
}
proclist->m_proc_callback(proclist->m_proc_callback_context, error, tinfo.tid, &tinfo, NULL, NULL);

if(sub_len && subreadsize != sub_len)
{
Expand Down Expand Up @@ -1625,12 +1597,9 @@ static int32_t scap_read_fdlist(scap_reader_t* r, uint32_t block_length, uint32_
size_t readsize;
size_t totreadsize = 0;
size_t padding_len;
struct scap_threadinfo *tinfo;
scap_fdinfo fdi;
scap_fdinfo *nfdi;
// uint16_t stlen;
uint64_t tid;
int32_t uth_status = SCAP_SUCCESS;
uint32_t padding;

//
Expand All @@ -1640,69 +1609,17 @@ static int32_t scap_read_fdlist(scap_reader_t* r, uint32_t block_length, uint32_
CHECK_READ_SIZE_ERR(readsize, sizeof(tid), error);
totreadsize += readsize;

if(proclist->m_proc_callback == NULL)
{
//
// Identify the process descriptor
//
HASH_FIND_INT64(proclist->m_proclist, &tid, tinfo);
}
else
{
tinfo = NULL;
}

while(((int32_t)block_length - (int32_t)totreadsize) >= 4)
{
if(scap_fd_read_from_disk(&fdi, &readsize, block_type, r, error) != SCAP_SUCCESS)
{
return SCAP_FAILURE;
}
totreadsize += readsize;

//
// Add the entry to the table, or fire the notification callback
//
if(proclist->m_proc_callback == NULL)
{
if(tinfo == NULL)
{
//
// We have the fdinfo but no associated tid, skip it
//
continue;
}

//
// Parsed successfully. Allocate the new entry and copy the temp one into into it.
//
nfdi = (scap_fdinfo *)malloc(sizeof(scap_fdinfo));
if(nfdi == NULL)
{
snprintf(error, SCAP_LASTERR_SIZE, "process table allocation error (fd1)");
return SCAP_FAILURE;
}

// Structure copy
*nfdi = fdi;

ASSERT(tinfo != NULL);

HASH_ADD_INT64(tinfo->fdlist, fd, nfdi);
if(uth_status != SCAP_SUCCESS)
{
free(nfdi);
snprintf(error, SCAP_LASTERR_SIZE, "process table allocation error (fd2)");
return SCAP_FAILURE;
}
}
else
{
ASSERT(tinfo == NULL);

proclist->m_proc_callback(
proclist->m_proc_callback_context, tid, NULL, &fdi);
}
proclist->m_proc_callback(proclist->m_proc_callback_context, error, tid, NULL, &fdi, NULL);
}

//
Expand Down Expand Up @@ -2186,9 +2103,7 @@ struct scap_platform *scap_savefile_alloc_platform(proc_entry_callback proc_call
platform->m_generic.m_vtable = &scap_savefile_platform_vtable;
platform->m_generic.m_machine_info.num_cpus = (uint32_t)-1;

platform->m_generic.m_proclist.m_proc_callback = proc_callback;
platform->m_generic.m_proclist.m_proc_callback_context = proc_callback_context;
platform->m_generic.m_proclist.m_proclist = NULL;
init_proclist(&platform->m_generic.m_proclist, proc_callback, proc_callback_context);

return &platform->m_generic;
}
Expand Down
4 changes: 1 addition & 3 deletions userspace/libscap/engine/test_input/test_input_platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,7 @@ struct scap_platform* scap_test_input_alloc_platform(proc_entry_callback proc_ca
struct scap_platform* generic = &platform->m_generic;
generic->m_vtable = &scap_test_input_platform;

platform->m_generic.m_proclist.m_proc_callback = proc_callback;
platform->m_generic.m_proclist.m_proc_callback_context = proc_callback_context;
platform->m_generic.m_proclist.m_proclist = NULL;
init_proclist(&platform->m_generic.m_proclist, proc_callback, proc_callback_context);

return generic;
}
87 changes: 23 additions & 64 deletions userspace/libscap/linux/scap_fds.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ int32_t scap_fd_handle_pipe(struct scap_proclist* proclist, char *fname, scap_th
strlcpy(fdi->info.fname, link_name, sizeof(fdi->info.fname));

fdi->ino = ino;
return scap_add_fd_to_proc_table(proclist, tinfo, fdi, error);
proclist->m_proc_callback(proclist->m_proc_callback_context, error, tinfo->tid, tinfo, fdi, NULL);
return SCAP_SUCCESS;
}

static inline uint32_t open_flags_to_scap(unsigned long flags)
Expand Down Expand Up @@ -354,7 +355,8 @@ int32_t scap_fd_handle_regular_file(struct scap_proclist *proclist, char *fname,
strlcpy(fdi->info.fname, link_name, sizeof(fdi->info.fname));
}

return scap_add_fd_to_proc_table(proclist, tinfo, fdi, error);
proclist->m_proc_callback(proclist->m_proc_callback_context, error, tinfo->tid, tinfo, fdi, NULL);
return SCAP_SUCCESS;
}

int32_t scap_fd_handle_socket(struct scap_proclist *proclist, char *fname, scap_threadinfo *tinfo, scap_fdinfo *fdi, char* procdir, uint64_t net_ns, struct scap_ns_socket_list **sockets_by_ns, char *error)
Expand Down Expand Up @@ -417,7 +419,8 @@ int32_t scap_fd_handle_socket(struct scap_proclist *proclist, char *fname, scap_
{
// it's a kind of socket, but we don't support it right now
fdi->type = SCAP_FD_UNSUPPORTED;
return scap_add_fd_to_proc_table(proclist, tinfo, fdi, error);
proclist->m_proc_callback(proclist->m_proc_callback_context, error, tinfo->tid, tinfo, fdi, NULL);
return SCAP_SUCCESS;
}

//
Expand All @@ -429,12 +432,9 @@ int32_t scap_fd_handle_socket(struct scap_proclist *proclist, char *fname, scap_
memcpy(&(fdi->info), &(tfdi->info), sizeof(fdi->info));
fdi->ino = ino;
fdi->type = tfdi->type;
return scap_add_fd_to_proc_table(proclist, tinfo, fdi, error);
}
else
{
return SCAP_SUCCESS;
proclist->m_proc_callback(proclist->m_proc_callback_context, error, tinfo->tid, tinfo, fdi, NULL);
}
return SCAP_SUCCESS;
}

int32_t scap_fd_read_unix_sockets_from_proc_fs(const char* filename, scap_fdinfo **sockets, char *error)
Expand Down Expand Up @@ -1254,7 +1254,7 @@ int32_t scap_fd_scan_fd_dir(struct scap_linux_platform *linux_platform, struct s
char link_name[SCAP_MAX_PATH_SIZE];
struct stat sb;
uint64_t fd;
scap_fdinfo *fdi = NULL;
scap_fdinfo fdi = {};
uint64_t net_ns;
ssize_t r;
uint32_t fd_added = 0;
Expand Down Expand Up @@ -1293,13 +1293,13 @@ int32_t scap_fd_scan_fd_dir(struct scap_linux_platform *linux_platform, struct s
while((dir_entry_p = readdir(dir_p)) != NULL &&
(linux_platform->m_fd_lookup_limit == 0 || fd_added < linux_platform->m_fd_lookup_limit))
{
fdi = NULL;
snprintf(f_name, SCAP_MAX_PATH_SIZE, "%s/%s", fd_dir_name, dir_entry_p->d_name);

if(-1 == stat(f_name, &sb) || 1 != sscanf(dir_entry_p->d_name, "%"PRIu64, &fd))
{
continue;
}
fdi.fd = fd;

// In no driver mode to limit cpu usage we just parse sockets
// because we are interested only on them
Expand All @@ -1311,74 +1311,33 @@ int32_t scap_fd_scan_fd_dir(struct scap_linux_platform *linux_platform, struct s
switch(sb.st_mode & S_IFMT)
{
case S_IFIFO:
res = scap_fd_allocate_fdinfo(&fdi, fd, SCAP_FD_FIFO);
if(SCAP_FAILURE == res)
{
snprintf(error, SCAP_LASTERR_SIZE, "can't allocate scap fd handle for fifo fd %" PRIu64, fd);
break;
}
res = scap_fd_handle_pipe(proclist, f_name, tinfo, fdi, error);
fdi.type = SCAP_FD_FIFO;
res = scap_fd_handle_pipe(proclist, f_name, tinfo, &fdi, error);
break;
case S_IFREG:
case S_IFBLK:
case S_IFCHR:
case S_IFLNK:
res = scap_fd_allocate_fdinfo(&fdi, fd, SCAP_FD_FILE_V2);
if(SCAP_FAILURE == res)
{
snprintf(error, SCAP_LASTERR_SIZE, "can't allocate scap fd handle for file fd %" PRIu64, fd);
break;
}
fdi->ino = sb.st_ino;
res = scap_fd_handle_regular_file(proclist, f_name, tinfo, fdi, procdir, error);
fdi.type = SCAP_FD_FILE_V2;
fdi.ino = sb.st_ino;
res = scap_fd_handle_regular_file(proclist, f_name, tinfo, &fdi, procdir, error);
break;
case S_IFDIR:
res = scap_fd_allocate_fdinfo(&fdi, fd, SCAP_FD_DIRECTORY);
if(SCAP_FAILURE == res)
{
snprintf(error, SCAP_LASTERR_SIZE, "can't allocate scap fd handle for dir fd %" PRIu64, fd);
break;
}
fdi->ino = sb.st_ino;
res = scap_fd_handle_regular_file(proclist, f_name, tinfo, fdi, procdir, error);
fdi.type = SCAP_FD_DIRECTORY;
fdi.ino = sb.st_ino;
res = scap_fd_handle_regular_file(proclist, f_name, tinfo, &fdi, procdir, error);
break;
case S_IFSOCK:
res = scap_fd_allocate_fdinfo(&fdi, fd, SCAP_FD_UNKNOWN);
if(SCAP_FAILURE == res)
{
snprintf(error, SCAP_LASTERR_SIZE, "can't allocate scap fd handle for sock fd %" PRIu64, fd);
break;
}
res = scap_fd_handle_socket(proclist, f_name, tinfo, fdi, procdir, net_ns, sockets_by_ns, error);
if(proclist->m_proc_callback == NULL)
{
// we can land here if we've got a netlink socket
if(fdi->type == SCAP_FD_UNKNOWN)
{
scap_fd_free_fdinfo(&fdi);
}
}
fdi.type = SCAP_FD_UNKNOWN;
res = scap_fd_handle_socket(proclist, f_name, tinfo, &fdi, procdir, net_ns, sockets_by_ns, error);
break;
default:
res = scap_fd_allocate_fdinfo(&fdi, fd, SCAP_FD_UNSUPPORTED);
if(SCAP_FAILURE == res)
{
snprintf(error, SCAP_LASTERR_SIZE, "can't allocate scap fd handle for unsupported fd %" PRIu64, fd);
break;
}
fdi->ino = sb.st_ino;
res = scap_fd_handle_regular_file(proclist, f_name, tinfo, fdi, procdir, error);
fdi.type = SCAP_FD_UNSUPPORTED;
fdi.ino = sb.st_ino;
res = scap_fd_handle_regular_file(proclist, f_name, tinfo, &fdi, procdir, error);
break;
}

if(proclist->m_proc_callback != NULL)
{
if(fdi)
{
scap_fd_free_fdinfo(&fdi);
}
}

if(SCAP_SUCCESS != res)
{
break;
Expand Down
3 changes: 2 additions & 1 deletion userspace/libscap/linux/scap_linux_int.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ int32_t scap_linux_create_iflist(struct scap_platform* platform);
int32_t scap_linux_create_userlist(struct scap_platform* platform);

uint32_t scap_linux_get_device_by_mount_id(struct scap_platform* platform, const char *procdir, unsigned long requested_mount_id);
struct scap_threadinfo* scap_linux_proc_get(struct scap_platform* platform, struct scap_proclist* proclist, int64_t tid, bool scan_sockets);
int32_t scap_linux_proc_get(struct scap_platform* platform, int64_t tid,
struct scap_threadinfo* tinfo, bool scan_sockets);
int32_t scap_linux_refresh_proc_table(struct scap_platform* platform, struct scap_proclist* proclist);
bool scap_linux_is_thread_alive(struct scap_platform* platform, int64_t pid, int64_t tid, const char* comm);
int32_t scap_linux_getpid_global(struct scap_platform* platform, int64_t *pid, char* error);
Expand Down
4 changes: 1 addition & 3 deletions userspace/libscap/linux/scap_linux_platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ struct scap_platform* scap_linux_alloc_platform(proc_entry_callback proc_callbac
struct scap_platform* generic = &platform->m_generic;
generic->m_vtable = &scap_linux_platform_vtable;

generic->m_proclist.m_proc_callback = proc_callback;
generic->m_proclist.m_proc_callback_context = proc_callback_context;
generic->m_proclist.m_proclist = NULL;
init_proclist(&generic->m_proclist, proc_callback, proc_callback_context);

return generic;
}