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.

Sign up for GitHub

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

fix(bpf): fixed a couple of clang15 verifier issues. #858

Merged
merged 1 commit into from
Feb 7, 2023
Merged

fix(bpf): fixed a couple of clang15 verifier issues. #858

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions driver/bpf/fillers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ static __always_inline int __bpf_##x(struct filler_data *data); \
__bpf_section(TP_NAME "filler/" #x) \
static __always_inline int bpf_##x(void *ctx) \
{ \
struct filler_data data; \
struct filler_data data = {0}; \
int res; \
\
res = init_filler_data(ctx, &data, is_syscall); \
Expand Down Expand Up @@ -289,7 +289,7 @@ FILLER_RAW(terminate_filler)
if (state->n_drops_scratch_map != ULLONG_MAX) {
++state->n_drops_scratch_map;
}
break;
break;
default:
bpf_printk("Unknown filler res=%d event=%d curarg=%d\n",
state->tail_ctx.prev_res,
Expand Down Expand Up @@ -2302,7 +2302,7 @@ FILLER(proc_startupdate, true)
arg_start = _READ(mm->arg_start);
args_len = arg_end - arg_start;

if (args_len) {
if (args_len > 0) {
if (args_len > ARGS_ENV_SIZE_MAX)
args_len = ARGS_ENV_SIZE_MAX;

Expand All @@ -2329,7 +2329,7 @@ FILLER(proc_startupdate, true)
case PPME_SYSCALL_EXECVE_19_X:
val = bpf_syscall_get_argument(data, 1);
break;

case PPME_SYSCALL_EXECVEAT_X:
val = bpf_syscall_get_argument(data, 2);
break;
Expand All @@ -2347,14 +2347,14 @@ FILLER(proc_startupdate, true)
args_len = 0;
}

if (args_len) {
if (args_len > 0) {
int exe_len;

exe_len = bpf_probe_read_kernel_str(&data->buf[data->state->tail_ctx.curoff & SCRATCH_SIZE_HALF],
SCRATCH_SIZE_HALF,
&data->buf[data->state->tail_ctx.curoff & SCRATCH_SIZE_HALF]);

if (exe_len == -EFAULT)
if (exe_len < 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers:

 * long bpf_probe_read_str(void *dst, u32 size, const void *unsafe_ptr)
 * 	Description
 * 		Copy a NUL terminated string from an unsafe kernel address
 * 		*unsafe_ptr* to *dst*. See **bpf_probe_read_kernel_str**\ () for
 * 		more details.
 *
 * 		Generally, use **bpf_probe_read_user_str**\ () or
 * 		**bpf_probe_read_kernel_str**\ () instead.
 * 	Return
 * 		On success, the strictly positive length of the string,
 * 		including the trailing NUL character. On error, a negative
 * 		value.

Source: linux include/uapi/linux/bpf.h

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we change https://github.com/falcosecurity/libs/blob/fix/clang15_bpf/driver/bpf/fillers.h#L1862,L1866 too?

Copy link
Contributor Author

@FedeDP FedeDP Feb 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so! You just earned a coauthored commit :P

return PPM_FAILURE_INVALID_USER_MEMORY;

/*
Expand All @@ -2365,11 +2365,15 @@ FILLER(proc_startupdate, true)
if (res != PPM_SUCCESS)
return res;

args_len -= exe_len;
if (args_len < 0)
return PPM_FAILURE_INVALID_USER_MEMORY;

/*
* Args
*/
data->curarg_already_on_frame = true;
res = __bpf_val_to_ring(data, 0, args_len - exe_len, PT_BYTEBUF, -1, false, KERNEL);
res = __bpf_val_to_ring(data, 0, args_len, PT_BYTEBUF, -1, false, KERNEL);
if (res != PPM_SUCCESS)
return res;
} else {
Expand Down