Skip to content

Commit

Permalink
bpf_helper: Do not crash the process when too many labels
Browse files Browse the repository at this point in the history 
Add tests for bpf_helper

Drive-by:
  Reformat code + code style fixes
  Fix potential OOB read in resolve_jumps
PiperOrigin-RevId: 621191165
Change-Id: I8c2909564dc626ed8488536f50267e916dc6819f
  • Loading branch information
@happyCoder92 @copybara-github
happyCoder92 authored and copybara-github committed Apr 2, 2024
1 parent fde80c1 commit c4cbf83
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 71 deletions.
11 changes: 11 additions & 0 deletions sandboxed_api/sandbox2/util/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ cc_library(
visibility = ["//visibility:public"],
)

cc_test(
name = "bpf_helper_test",
srcs = ["bpf_helper_test.cc"],
copts = sapi_platform_copts(),
deps = [
":bpf_helper",
"@com_google_absl//absl/strings",
"@com_google_googletest//:gtest_main",
],
)

cc_library(
name = "minielf",
srcs = ["minielf.cc"],
Expand Down
14 changes: 14 additions & 0 deletions sandboxed_api/sandbox2/util/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,18 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
sapi::test_main
)
gtest_discover_tests_xcompile(sandbox2_maps_parser_test)

# sandboxed_api/sandbox2/util:bpf_helper_test
add_executable(sandbox2_bpf_helper_test
bpf_helper_test.cc
)
set_target_properties(sandbox2_bpf_helper_test PROPERTIES
OUTPUT_NAME bpf_helper_test
)
target_link_libraries(sandbox2_bpf_helper_test PRIVATE
absl::strings
sandbox2::bpf_helper
sapi::test_main
)
gtest_discover_tests_xcompile(sandbox2_bpf_helper_test)
endif()
141 changes: 70 additions & 71 deletions sandboxed_api/sandbox2/util/bpf_helper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,81 +28,80 @@
#include <stdlib.h>
#include <string.h>

int bpf_resolve_jumps(struct bpf_labels *labels,
struct sock_filter *filter, size_t count)
{
size_t i;

if (count < 1 || count > BPF_MAXINSNS)
return -1;
/*
* Walk it once, backwards, to build the label table and do fixups.
* Since backward jumps are disallowed by BPF, this is easy.
*/
for (i = 0; i < count; ++i) {
size_t offset = count - i - 1;
struct sock_filter *instr = &filter[offset];
if (instr->code != (BPF_JMP+BPF_JA))
continue;
switch ((instr->jt<<8)|instr->jf) {
case (JUMP_JT<<8)|JUMP_JF:
if (labels->labels[instr->k].location == 0xffffffff) {
fprintf(stderr, "Unresolved label: '%s'\n",
labels->labels[instr->k].label);
return 1;
}
instr->k = labels->labels[instr->k].location -
(offset + 1);
instr->jt = 0;
instr->jf = 0;
continue;
case (LABEL_JT<<8)|LABEL_JF:
if (labels->labels[instr->k].location != 0xffffffff) {
fprintf(stderr, "Duplicate label use: '%s'\n",
labels->labels[instr->k].label);
return 1;
}
labels->labels[instr->k].location = offset;
instr->k = 0; /* fall through */
instr->jt = 0;
instr->jf = 0;
continue;
}
}
return 0;
int bpf_resolve_jumps(struct bpf_labels *labels, struct sock_filter *filter,
size_t count) {
if (count < 1 || count > BPF_MAXINSNS) {
return -1;
}
if (labels->count > BPF_LABELS_MAX) {
return -1;
}
/*
* Walk it once, backwards, to build the label table and do fixups.
* Since backward jumps are disallowed by BPF, this is easy.
*/
for (size_t i = 0; i < count; ++i) {
size_t offset = count - i - 1;
struct sock_filter *instr = &filter[offset];
if (instr->code != (BPF_JMP + BPF_JA)) {
continue;
}
if (instr->k >= labels->count) {
fprintf(stderr, "Label index out of bounds: '%d'\n", instr->k);
return 1;
}
switch ((instr->jt << 8) | instr->jf) {
case (JUMP_JT << 8) | JUMP_JF:
if (labels->labels[instr->k].location == 0xffffffff) {
fprintf(stderr, "Unresolved label: '%s'\n",
labels->labels[instr->k].label);
return 1;
}
instr->k = labels->labels[instr->k].location - (offset + 1);
instr->jt = 0;
instr->jf = 0;
continue;
case (LABEL_JT << 8) | LABEL_JF:
if (labels->labels[instr->k].location != 0xffffffff) {
fprintf(stderr, "Duplicate label use: '%s'\n",
labels->labels[instr->k].label);
return 1;
}
labels->labels[instr->k].location = offset;
instr->k = 0; /* fall through */
instr->jt = 0;
instr->jf = 0;
continue;
}
}
return 0;
}

/* Simple lookup table for labels. */
__u32 seccomp_bpf_label(struct bpf_labels *labels, const char *label)
{
struct __bpf_label *begin = labels->labels, *end;
int id;
__u32 seccomp_bpf_label(struct bpf_labels *labels, const char *label) {
struct __bpf_label *begin = labels->labels;
struct __bpf_label *end = begin + labels->count;

if (labels->count == BPF_LABELS_MAX) {
fprintf(stderr, "Too many labels\n");
exit(1);
}
if (labels->count == 0) {
begin->label = label;
begin->location = 0xffffffff;
labels->count++;
return 0;
}
end = begin + labels->count;
for (id = 0; begin < end; ++begin, ++id) {
if (!strcmp(label, begin->label))
return id;
}
begin->label = label;
begin->location = 0xffffffff;
labels->count++;
return id;
if (labels->count > BPF_LABELS_MAX) {
return labels->count;
}
for (int id = 0; begin < end; ++begin, ++id) {
if (!strcmp(label, begin->label)) {
return id;
}
}
if (labels->count == BPF_LABELS_MAX) {
fprintf(stderr, "Too many labels\n");
return labels->count++;
}
begin->label = label;
begin->location = 0xffffffff;
return labels->count++;
}

void seccomp_bpf_print(struct sock_filter *filter, size_t count)
{
struct sock_filter *end = filter + count;
for ( ; filter < end; ++filter)
printf("{ code=%u,jt=%u,jf=%u,k=%u },\n",
filter->code, filter->jt, filter->jf, filter->k);
void seccomp_bpf_print(struct sock_filter *filter, size_t count) {
struct sock_filter *end = filter + count;
for (; filter < end; ++filter)
printf("{ code=%u,jt=%u,jf=%u,k=%u },\n", filter->code, filter->jt,
filter->jf, filter->k);
}
89 changes: 89 additions & 0 deletions sandboxed_api/sandbox2/util/bpf_helper_test.cc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
#include "sandboxed_api/sandbox2/util/bpf_helper.h"

#include <linux/filter.h>

#include <string>
#include <vector>

#include "gtest/gtest.h"
#include "absl/strings/str_cat.h"

namespace {

void AddMaxLabels(bpf_labels& labels) {
static std::vector<std::string>* label_strs = []() {
auto* label_strs = new std::vector<std::string>();
for (int i = 0; i < BPF_LABELS_MAX; ++i) {
label_strs->push_back(absl::StrCat("lbl", i));
}
return label_strs;
}();
for (int i = 0; i < BPF_LABELS_MAX; ++i) {
seccomp_bpf_label(&labels, (*label_strs)[i].c_str());
}
}

TEST(BpfHelperTest, MaxLabels) {
bpf_labels labels = {};
AddMaxLabels(labels);
std::vector<struct sock_filter> filter = {
ALLOW,
};
EXPECT_EQ(bpf_resolve_jumps(&labels, filter.data(), filter.size()), 0);
}

TEST(BpfHelperTest, LabelOverflow) {
bpf_labels labels = {};
AddMaxLabels(labels);
std::vector<struct sock_filter> filter = {
JUMP(&labels, overflow),
LABEL(&labels, overflow),
ALLOW,
};
filter.push_back(ALLOW);
EXPECT_NE(bpf_resolve_jumps(&labels, filter.data(), filter.size()), 0);
}

TEST(BpfHelperTest, UnresolvedLabel) {
bpf_labels labels = {};
std::vector<struct sock_filter> filter = {
JUMP(&labels, unresolved),
LABEL(&labels, unused),
};
filter.push_back(ALLOW);
EXPECT_NE(bpf_resolve_jumps(&labels, filter.data(), filter.size()), 0);
}

TEST(BpfHelperTest, BackwardJump) {
bpf_labels labels = {};
std::vector<struct sock_filter> filter = {
LABEL(&labels, backward),
JUMP(&labels, backward),
};
filter.push_back(ALLOW);
EXPECT_NE(bpf_resolve_jumps(&labels, filter.data(), filter.size()), 0);
}

TEST(BpfHelperTest, Duplicate) {
bpf_labels labels = {};
std::vector<struct sock_filter> filter = {
JUMP(&labels, dup),
LABEL(&labels, dup),
LABEL(&labels, dup),
};
filter.push_back(ALLOW);
EXPECT_NE(bpf_resolve_jumps(&labels, filter.data(), filter.size()), 0);
}

TEST(BpfHelperTest, OutOfBoundsLabel) {
bpf_labels labels = {};
std::vector<struct sock_filter> filter = {
JUMP(&labels, normal),
LABEL(&labels, normal),
BPF_JUMP(BPF_JMP + BPF_JA, 1, JUMP_JT, JUMP_JF),
};
filter.push_back(ALLOW);
EXPECT_NE(bpf_resolve_jumps(&labels, filter.data(), filter.size()), 0);
}

} // namespace

0 comments on commit c4cbf83

Please sign in to comment.