Skip to content

Commit 73ff0fa

Browse files
CopilotAlan-Jowett
andcommitted
Address code review feedback: fix documentation and add overflow checks
Co-authored-by: Alan-Jowett <20480683+Alan-Jowett@users.noreply.github.com>
1 parent f490854 commit 73ff0fa

File tree

4 files changed

+24
-14
lines changed

4 files changed

+24
-14
lines changed
Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
# Large Program Test
22

3-
This test validates that programs with more than 65,536 instructions can be loaded and executed correctly when `ubpf_set_max_instructions()` is used to increase the limit.
3+
This test validates that programs with more than 65,536 instructions can be loaded correctly when `ubpf_set_max_instructions()` is used to increase the limit.
44

55
The test performs the following:
66
1. Creates a VM and sets max_instructions to 100,000
7-
2. Generates a program with 70,000 instructions (NOP-like JA instructions with offset 0) plus an EXIT
7+
2. Generates a program with 66,000 instructions (NOP-like JA instructions with offset 0) plus an EXIT
88
3. Loads the program into the VM
9-
4. Executes the program via interpreter
10-
5. JIT compiles and executes the program
11-
6. Verifies both return the same result
129

1310
This validates that:
1411
- The type change from uint16_t to uint32_t for num_insts works correctly
1512
- Programs beyond the old 65,536 limit can be loaded
16-
- The validation and execution paths handle large instruction counts
17-
- The JIT compiler can handle large programs
13+
- The validation path handles large instruction counts correctly
14+
15+
Note: The test skips interpreter and JIT execution to keep test runtime reasonable.
16+
Large programs with many sequential NOPs would take prohibitively long to execute.

custom_tests/descrs/ubpf_test_max_insts_api.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22

33
This test validates the boundary conditions and API for configurable instruction limits:
44

5-
1. Test that exactly 65,536 instructions work with default settings
6-
2. Test that 65,537 instructions fail with default settings
5+
1. Test that 65,535 instructions (just under the default limit) load successfully
6+
2. Test that 65,536 instructions (at the default limit) fail to load
77
3. Test that ubpf_set_max_instructions() works correctly
88
4. Test that ubpf_set_max_instructions() fails if code is already loaded
99
5. Test setting a lower limit than default
1010

1111
This ensures:
12-
- The old default limit of 65,536 is preserved for backward compatibility
12+
- The default limit of 65,536 is preserved for backward compatibility
1313
- The API correctly validates and enforces limits
1414
- Programs at the boundary behave as expected

custom_tests/srcs/ubpf_test_max_insts_api.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ main(int argc, char** argv)
4343
(void)argc;
4444
(void)argv;
4545

46-
// Test 1: Exactly 65,536 instructions should work with default settings (just under the limit)
46+
// Test 1: 65,535 instructions (just under default limit) should work
4747
{
48-
std::cout << "Test 1: Loading 65,535 instructions with default limit..." << std::endl;
48+
std::cout << "Test 1: Loading 65,535 instructions (just under default limit)..." << std::endl;
4949
std::unique_ptr<ubpf_vm, decltype(&ubpf_destroy)> vm(ubpf_create(), ubpf_destroy);
5050
auto program = generate_program(65535);
5151

@@ -60,9 +60,9 @@ main(int argc, char** argv)
6060
std::cout << "Test 1 PASSED" << std::endl;
6161
}
6262

63-
// Test 2: 65,536 or more instructions should fail with default settings
63+
// Test 2: 65,536 instructions (at default limit) should fail
6464
{
65-
std::cout << "Test 2: Loading 65,536 instructions with default limit (should fail)..." << std::endl;
65+
std::cout << "Test 2: Loading 65,536 instructions (at default limit - should fail)..." << std::endl;
6666
std::unique_ptr<ubpf_vm, decltype(&ubpf_destroy)> vm(ubpf_create(), ubpf_destroy);
6767
auto program = generate_program(65536);
6868

vm/ubpf_jit_support.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,19 @@ initialize_jit_state_result(
8787
// - Dispatcher loads
8888
// - Other JIT-generated relocations
8989
// Conservative estimate: 2x the instruction count + some fixed overhead
90+
// Check for overflow: ensure vm->num_insts <= (UINT32_MAX - 64) / 2
91+
if (vm->num_insts > (UINT32_MAX - 64) / 2) {
92+
*errmsg = ubpf_error("Program too large for JIT compilation");
93+
return -1;
94+
}
9095
uint32_t array_size = vm->num_insts * 2 + 64;
9196

97+
// Check for overflow in pc_locs allocation (vm->num_insts + 1)
98+
if (vm->num_insts == UINT32_MAX) {
99+
*errmsg = ubpf_error("Program too large for JIT compilation");
100+
return -1;
101+
}
102+
92103
state->max_insts = array_size;
93104
state->pc_locs = calloc(vm->num_insts + 1, sizeof(state->pc_locs[0]));
94105
state->jumps = calloc(array_size, sizeof(state->jumps[0]));

0 commit comments

Comments
 (0)