Skip to content

Commit

Permalink
fix(libscap/engines): move some checks in start_capture
Browse files Browse the repository at this point in the history
Doing these checks in the init method could cause some issues.
In the kernel module engine, an old driver with different IOCTL codes
could be loaded and if we use IOCTL codes before checking the
compatibility we could face a not user friendly error. For this reason,
we move this `mark_syscall` check when the `check_api_compatibility`
method is already called by scap.

Signed-off-by: Andrea Terzolo <[email protected]>
  • Loading branch information
Andreagit97 authored and poiana committed Oct 17, 2023
1 parent 0bba33c commit e6da2ab
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 35 deletions.
29 changes: 16 additions & 13 deletions userspace/libscap/engine/bpf/scap_bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,22 @@ static int enforce_sc_set(struct bpf_engine* handle)
int32_t scap_bpf_start_capture(struct scap_engine_handle engine)
{
struct bpf_engine* handle = engine.m_handle;
int32_t rc = 0;
/* Here we are covering the case in which some syscalls don't have an associated ppm_sc
* and so we cannot set them as (un)interesting. For this reason, we default them to 0.
* Please note this is an extra check since our ppm_sc should already cover all possible syscalls.
* Ideally we should do this only once, but right now in our code we don't have a "right" place to do it.
* We need to move it, if `scap_start_capture` will be called frequently in our flow, right now in live mode, it
* should be called only once...
*/
for(int i = 0; i < SYSCALL_TABLE_SIZE; i++)
{
rc = set_single_syscall_of_interest(handle, i, false);
if(rc != SCAP_SUCCESS)
{
return rc;
}
}
handle->capturing = true;
return enforce_sc_set(handle);
}
Expand Down Expand Up @@ -2010,19 +2026,6 @@ static int32_t init(scap_t* handle, scap_open_args *oargs)
return rc;
}

/* Here we are covering the case in which some syscalls don't have an associated ppm_sc
* and so we cannot set them as (un)interesting. For this reason, we default them to 0.
* Please note this is an extra check since our ppm_sc should already cover all possible syscalls.
*/
for(int i = 0; i < SYSCALL_TABLE_SIZE; i++)
{
rc = set_single_syscall_of_interest(engine.m_handle, i, false);
if(rc != SCAP_SUCCESS)
{
return rc;
}
}

/* Store interesting sc codes */
memcpy(&engine.m_handle->curr_sc_set, &oargs->ppm_sc_of_interest, sizeof(interesting_ppm_sc_set));

Expand Down
29 changes: 16 additions & 13 deletions userspace/libscap/engine/kmod/scap_kmod.c
Original file line number Diff line number Diff line change
Expand Up @@ -513,19 +513,6 @@ int32_t scap_kmod_init(scap_t *handle, scap_open_args *oargs)
return scap_errprintf(handle->m_lasterr, 0, "mismatch, processors online after the 'for' loop: %d, '_SC_NPROCESSORS_ONLN' after the 'for' loop: %d", online_idx, final_ndevs);
}

/* Here we are covering the case in which some syscalls don't have an associated ppm_sc
* and so we cannot set them as (un)interesting. For this reason, we default them to 0.
* Please note this is an extra check since our ppm_sc should already cover all possible syscalls.
*/
for(int i = 0; i < SYSCALL_TABLE_SIZE; i++)
{
rc = mark_syscall(engine.m_handle, PPM_IOCTL_DISABLE_SYSCALL, i);
if(rc != SCAP_SUCCESS)
{
return rc;
}
}

/* Store interesting sc codes */
memcpy(&engine.m_handle->curr_sc_set, &oargs->ppm_sc_of_interest, sizeof(interesting_ppm_sc_set));

Expand Down Expand Up @@ -672,6 +659,22 @@ int32_t scap_kmod_stop_capture(struct scap_engine_handle engine)
int32_t scap_kmod_start_capture(struct scap_engine_handle engine)
{
struct kmod_engine* handle = engine.m_handle;
int32_t rc = 0;
/* Here we are covering the case in which some syscalls don't have an associated ppm_sc
* and so we cannot set them as (un)interesting. For this reason, we default them to 0.
* Please note this is an extra check since our ppm_sc should already cover all possible syscalls.
* Ideally we should do this only once, but right now in our code we don't have a "right" place to do it.
* We need to move it, if `scap_start_capture` will be called frequently in our flow, right now in live mode, it
* should be called only once...
*/
for(int i = 0; i < SYSCALL_TABLE_SIZE; i++)
{
rc = mark_syscall(handle, PPM_IOCTL_DISABLE_SYSCALL, i);
if(rc != SCAP_SUCCESS)
{
return rc;
}
}
handle->capturing = true;
return enforce_sc_set(handle);
}
Expand Down
20 changes: 11 additions & 9 deletions userspace/libscap/engine/modern_bpf/scap_modern_bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,17 @@ static int32_t scap_modern_bpf__configure(struct scap_engine_handle engine, enum
int32_t scap_modern_bpf__start_capture(struct scap_engine_handle engine)
{
struct modern_bpf_engine* handle = engine.m_handle;
/* Here we are covering the case in which some syscalls don't have an associated ppm_sc
* and so we cannot set them as (un)interesting. For this reason, we default them to 0.
* Please note this is an extra check since our ppm_sc should already cover all possible syscalls.
* Ideally we should do this only once, but right now in our code we don't have a "right" place to do it.
* We need to move it, if `scap_start_capture` will be called frequently in our flow, right now in live mode, it
* should be called only once...
*/
for(int i = 0; i < SYSCALL_TABLE_SIZE; i++)
{
pman_mark_single_64bit_syscall(i, false);
}
handle->capturing = true;
return pman_enforce_sc_set(handle->curr_sc_set.ppm_sc);
}
Expand Down Expand Up @@ -203,15 +214,6 @@ int32_t scap_modern_bpf__init(scap_t* handle, scap_open_args* oargs)
return ret;
}

/* Here we are covering the case in which some syscalls don't have an associated ppm_sc
* and so we cannot set them as (un)interesting. For this reason, we default them to 0.
* Please note this is an extra check since our ppm_sc should already cover all possible syscalls.
*/
for(int i = 0; i < SYSCALL_TABLE_SIZE; i++)
{
pman_mark_single_64bit_syscall(i, false);
}

/* Store interesting sc codes */
memcpy(&engine.m_handle->curr_sc_set, &oargs->ppm_sc_of_interest, sizeof(interesting_ppm_sc_set));

Expand Down

0 comments on commit e6da2ab

Please sign in to comment.