Refactor - Assign SBPF versions to SIMDs #602

Lichtso · 2024-10-02T17:31:02Z

No description provided.

LucasSte · 2024-11-22T19:54:34Z

src/verifier.rs

                ebpf::EXIT       if !sbpf_version.static_syscalls()   => {},
                ebpf::RETURN     if sbpf_version.static_syscalls()    => {},
-                ebpf::SYSCALL    if sbpf_version.static_syscalls()    => {
+                ebpf::SYSCALL    if sbpf_version.stricter_controlflow()    => {


This one is still static syscall isn't it?

I believe that check here should still be sbpf_version.static_syscalls(), as this path will be available with static syscalls.

LucasSte · 2024-11-22T20:01:38Z

src/elf.rs

            ElfExecutable::parse_ro_sections(
                &config,
-                &SBPFVersion::V1, // v2 requires optimize_rodata=true
+                &SBPFVersion::V0, // v2 requires optimize_rodata=true


Should V3 in the comment now?

LucasSte · 2024-11-22T20:06:21Z

doc/bytecode.md

- `add64 reg, imm` can use `r11` as destination register
+
+### until v3
+- The targets of `call` instructions (which includes `syscall` instructions) is checked at runtime not verification time


Suggested change

- The targets of `call` instructions (which includes `syscall` instructions) is checked at runtime not verification time

- The targets of `call` instructions (which includes `syscall` instructions) are checked at runtime not verification time

doc/bytecode.md

LucasSte · 2024-11-25T19:38:57Z

src/elf.rs

+                SBPFVersion::Reserved
            } else {
-                SBPFVersion::V1
+                SBPFVersion::V0


If we get a 3 in the e_flags, the version will be V0 here.

Yep exactly, that is the bug we are feature gating out here.

LucasSte · 2024-11-25T19:42:30Z

src/verifier.rs

                ebpf::EXIT       if !sbpf_version.static_syscalls()   => {},
                ebpf::RETURN     if sbpf_version.static_syscalls()    => {},
-                ebpf::SYSCALL    if sbpf_version.static_syscalls()    => {
+                ebpf::SYSCALL    if sbpf_version.stricter_controlflow()    => {


I believe that check here should still be sbpf_version.static_syscalls(), as this path will be available with static syscalls.

LucasSte · 2024-11-25T21:00:42Z

src/verifier.rs

                ebpf::JSLT_REG   => { check_jmp_offset(prog, insn_ptr, &function_range)?; },
                ebpf::JSLE_IMM   => { check_jmp_offset(prog, insn_ptr, &function_range)?; },
                ebpf::JSLE_REG   => { check_jmp_offset(prog, insn_ptr, &function_range)?; },
                ebpf::CALL_IMM   if sbpf_version.static_syscalls() => {


This one doesn't need to be gated behind static_syscalls does it?

LucasSte · 2024-11-25T21:01:23Z

src/verifier.rs

                ebpf::SYSCALL    if sbpf_version.static_syscalls()    => {
-                    check_call_target(
-                        insn.imm as u32,
-                        syscall_registry,
-                        VerifierError::InvalidSyscall(insn.imm as u32))?;
+                    if sbpf_version.stricter_controlflow() {


Does this one need two gates?

One to make it a valid opcode and one for the verification rule. These are two different SIMDs but same SBPF version.

The verification rule for static syscalls is in the static syscalls SIMD.

LucasSte · 2024-11-25T21:03:29Z

doc/bytecode.md

 - Opcodes from the product / quotient / remainder instruction class are forbiden
+- `neg32` and `neg64` are allowed
 - `le` is allowed
+- `lddw` (opcodes `0x18` and `0x00`) are allowed


Suggested change

- `lddw` (opcodes `0x18` and `0x00`) are allowed

- `lddw` (opcodes `0x18` and `0x00`) is allowed

LucasSte · 2024-11-25T21:03:55Z

doc/bytecode.md

 - Opcodes from the product / quotient / remainder instruction class are allowed
+- `neg32` and `neg64` are forbidden
 - `le` is forbidden
+- `lddw` (opcodes `0x18` and `0x00`) are forbidden


Suggested change

- `lddw` (opcodes `0x18` and `0x00`) are forbidden

- `lddw` (opcodes `0x18` and `0x00`) is forbidden

Lichtso force-pushed the refactor/assign_sbpf_versions_to_simds branch from 88e0c85 to b6d8898 Compare October 3, 2024 16:37

Lichtso force-pushed the refactor/assign_sbpf_versions_to_simds branch from b6d8898 to 4b2c3df Compare October 11, 2024 16:42

Lichtso mentioned this pull request Oct 28, 2024

Feature - Strict ELF parser #624

Merged

Lichtso force-pushed the refactor/assign_sbpf_versions_to_simds branch 3 times, most recently from dcf9245 to 785454e Compare November 22, 2024 12:08

Lichtso marked this pull request as ready for review November 22, 2024 12:10

Inverts some SBPFVersion feature flags for consistency.

a6d4529

Lichtso force-pushed the refactor/assign_sbpf_versions_to_simds branch from 6f0ba50 to 08bcb37 Compare November 22, 2024 14:33

Lichtso requested a review from LucasSte November 22, 2024 14:33

Lichtso force-pushed the refactor/assign_sbpf_versions_to_simds branch 2 times, most recently from 60a4eed to dbd0844 Compare November 22, 2024 15:06

LucasSte reviewed Nov 22, 2024

View reviewed changes

LucasSte reviewed Nov 25, 2024

View reviewed changes

Lichtso force-pushed the refactor/assign_sbpf_versions_to_simds branch from dbd0844 to 80aa8e1 Compare November 25, 2024 20:11

LucasSte reviewed Nov 25, 2024

View reviewed changes

Lichtso force-pushed the refactor/assign_sbpf_versions_to_simds branch from 80aa8e1 to 0442846 Compare November 26, 2024 15:19

Lichtso added 3 commits November 26, 2024 16:41

Groups the SBPFVersion feature flags by their SIMDs.

8f3e61b

Adjusts SBPFVersion.

00f08ab

Updates ISA spec accordingly.

3630a01

Lichtso force-pushed the refactor/assign_sbpf_versions_to_simds branch from 0442846 to 3630a01 Compare November 26, 2024 15:45

LucasSte approved these changes Nov 26, 2024

View reviewed changes

Lichtso merged commit a8247dd into main Nov 26, 2024
12 checks passed

Lichtso deleted the refactor/assign_sbpf_versions_to_simds branch November 26, 2024 20:04

	- The targets of `call` instructions (which includes `syscall` instructions) is checked at runtime not verification time
	- The targets of `call` instructions (which includes `syscall` instructions) are checked at runtime not verification time

	- `lddw` (opcodes `0x18` and `0x00`) are allowed
	- `lddw` (opcodes `0x18` and `0x00`) is allowed

Refactor - Assign SBPF versions to SIMDs #602

Refactor - Assign SBPF versions to SIMDs #602

Uh oh!

Conversation

Lichtso commented Oct 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants