Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

Refactor - Moves config flags into executable capabilities #473

Merged
merged 9 commits into from
Jun 27, 2023

Conversation

Lichtso
Copy link

@Lichtso Lichtso commented Jun 16, 2023

Makes the SBPF version selectable per executable, instead of per loader.

Replaces:

  • Config::dynamic_stack_frames
  • Config::enable_sdiv
  • Config::static_syscalls
  • Config::enable_elf_vaddr
  • Config::reject_rodata_stack_overlap

With Config::enable_sbpf_v1 and Config::enable_sbpf_v2.

@Lichtso Lichtso marked this pull request as draft June 16, 2023 13:09
@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2023

Codecov Report

Merging #473 (e96e757) into main (f91d598) will increase coverage by 0.15%.
The diff coverage is 95.53%.

@@            Coverage Diff             @@
##             main     #473      +/-   ##
==========================================
+ Coverage   89.65%   89.81%   +0.15%     
==========================================
  Files          23       23              
  Lines        9989    10195     +206     
==========================================
+ Hits         8956     9157     +201     
- Misses       1033     1038       +5     
Impacted Files Coverage Δ
src/ebpf.rs 79.59% <ø> (ø)
src/syscalls.rs 98.49% <ø> (ø)
test_utils/src/lib.rs 96.15% <75.00%> (+0.15%) ⬆️
src/elf.rs 89.25% <92.22%> (+0.27%) ⬆️
src/assembler.rs 99.61% <100.00%> (+0.01%) ⬆️
src/disassembler.rs 93.36% <100.00%> (+0.03%) ⬆️
src/interpreter.rs 97.89% <100.00%> (+0.30%) ⬆️
src/jit.rs 92.78% <100.00%> (+0.08%) ⬆️
src/memory_region.rs 94.01% <100.00%> (+0.38%) ⬆️
src/static_analysis.rs 66.27% <100.00%> (+0.03%) ⬆️
... and 2 more

@Lichtso Lichtso force-pushed the refactor/executable_capabilities branch 8 times, most recently from ebb0385 to 42e217f Compare June 21, 2023 18:35
@Lichtso Lichtso marked this pull request as ready for review June 21, 2023 19:06
@Lichtso Lichtso requested a review from alessandrod June 21, 2023 19:08
src/elf.rs Outdated
@@ -260,13 +261,26 @@ pub(crate) enum Section {
Borrowed(usize, Range<usize>),
}

/// Defines a set of capabilities of an executable
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum ExecutableCapabilities {

Choose a reason for hiding this comment

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

Wouldn't it be better if this was

#[deive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
enum SbpfVersion {
	V1 = 1,
	V2 = 2,
	V3 = 3,
}

impl SbpfVersion {
	fn has_static_syscalls(&self) -> bool {
		self >= SbpfVersion::V2
	}

	fn allows_elf_virtual_addresses(&self) -> bool { ... }

	...
}

Then in the call sites it would be obvious from the has_* name what's being
checked for, and we wouldn't have to go and edit call sites if we flip things on
and off across versions.

@Lichtso Lichtso force-pushed the refactor/executable_capabilities branch from 42e217f to 11ad14d Compare June 26, 2023 14:22
@Lichtso Lichtso merged commit 0881f8a into main Jun 27, 2023
@Lichtso Lichtso deleted the refactor/executable_capabilities branch June 27, 2023 10:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants