Skip to content

Commit

Permalink
sbpf: undo rodata footprint optimization
Browse files Browse the repository at this point in the history
This commit increases the ELF loader's rodata_footprint to be equal
to the ELF file size.  This increases memory usage by 5-10% for most
contracts.

The ELF loader made an incorrect assumption that relocations outside
of the rodata segment are invisible to the virtual machine.  The
loader therefore reduced the rodata segment size and skipped those
relocations.

This is problematic for two reasons:

  1) R_BPF_64_32 can read ~15 bytes past the end of the rodata region
     and conditionally fails loading depending on the content that was
     read.
  2) Some relocations move information from high bits to low bits
     within a 64-bit value. Chaining those relocations thus allows
     moving information from outside the rodata segment into the
     rodata segment.

fd_sbpf_loader would have the wrong execution result in both cases.
  • Loading branch information
riptl authored and ripatel-fd committed Dec 12, 2023
1 parent 225a251 commit 2e27ed1
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 30 deletions.
30 changes: 3 additions & 27 deletions src/ballet/sbpf/fd_sbpf_loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,6 @@ typedef union fd_sbpf_elf fd_sbpf_elf_t;
#define FD_SBPF_MM_PROGRAM_ADDR (0x100000000UL) /* readonly program data */
#define FD_SBPF_MM_STACK_ADDR (0x200000000UL) /* stack (with gaps) */

/* FD_SBPF_RODATA_GUARD is the size of the guard area after the rodata
segment. This is required because relocations can be partially
out-of-bounds. The largest relocation type is 16 bytes of read/write,
so there should be 16 minus 1 bytes of guard area past the end.
Guard area at the beginning is not required, as the rodata segment
always starts at file offset zero.
Example:
[ RODATA segment ....... ] [ Guard area .................... ]
[ Relocation .................... ] */

#define FD_SBPF_RODATA_GUARD (15UL)

/* _fd_int_store_if_negative stores x to *p if *p is negative (branchless) */

static inline int
Expand Down Expand Up @@ -441,7 +427,7 @@ fd_sbpf_load_shdrs( fd_sbpf_elf_info_t * info,
}

info->rodata_sz = (uint)segment_end;
info->rodata_footprint = (uint)( segment_end+FD_SBPF_RODATA_GUARD );
info->rodata_footprint = (uint)elf_sz;

return 0;
}
Expand Down Expand Up @@ -801,6 +787,8 @@ fd_sbpf_r_bpf_64_64( fd_sbpf_loader_t const * loader,
fd_sbpf_elf_info_t const * info,
fd_elf64_rel const * rel ) {

(void)info;

uint r_sym = FD_ELF64_R_SYM( rel->r_info );
ulong r_offset = rel->r_offset;

Expand All @@ -822,9 +810,6 @@ fd_sbpf_r_bpf_64_64( fd_sbpf_loader_t const * loader,
fd_elf64_sym const * sym = &dynsyms[ r_sym ];
ulong S = sym->st_value;

/* Skip if side effects not visible in VM */
if( FD_UNLIKELY( A_off_lo > info->rodata_sz ) ) return 0;

/* Relocate */
ulong A = FD_LOAD( uint, &rodata[ A_off_lo ] );
ulong V = fd_ulong_sat_add( S, A );
Expand Down Expand Up @@ -876,9 +861,6 @@ fd_sbpf_r_bpf_64_relative( fd_sbpf_elf_t const * elf,
REQUIRE( va!=0UL );
va = va<FD_SBPF_MM_PROGRAM_ADDR ? va+FD_SBPF_MM_PROGRAM_ADDR : va;

/* Skip if side effects not visible in VM */
if( FD_UNLIKELY( imm_lo_off > info->rodata_sz ) ) return 0;

/* Write back
Skip bounds check as .text is guaranteed to be writable */
FD_STORE( uint, rodata+imm_lo_off, (uint)( va ) );
Expand All @@ -889,9 +871,6 @@ fd_sbpf_r_bpf_64_relative( fd_sbpf_elf_t const * elf,
/* Bounds checks */
REQUIRE( (r_offset+8UL>r_offset) & (r_offset+8UL<=elf_sz) );

/* Skip if side effects not visible in VM */
if( FD_UNLIKELY( r_offset > info->rodata_sz ) ) return 0;

/* Read implicit addend */
ulong va = FD_LOAD( uint, rodata+r_offset+4UL );

Expand Down Expand Up @@ -980,9 +959,6 @@ fd_sbpf_r_bpf_64_32( fd_sbpf_loader_t const * loader,
REQUIRE( (r_offset+8UL>r_offset) & (r_offset+8UL<=elf_sz) );
ulong A_off = r_offset+4UL;

/* Skip if side effects not visible in VM */
if( FD_UNLIKELY( A_off > info->rodata_sz ) ) return 0;

/* Apply relocation */
FD_STORE( uint, rodata+A_off, V );

Expand Down
6 changes: 3 additions & 3 deletions src/ballet/sbpf/fd_sbpf_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ struct fd_sbpf_elf_info {
uint dynstr_sz; /* Dynstr char count */

uint rodata_sz; /* size of rodata segment */
uint rodata_footprint; /* rodata_sz + FD_SBPF_RODATA_GUARD */
uint rodata_footprint; /* size of ELF binary */

/* Known section indices
In [-1,USHORT_MAX) where -1 means "not found" */
Expand All @@ -77,8 +77,8 @@ typedef struct fd_sbpf_elf_info fd_sbpf_elf_info_t;
[rodata,rodata+rodata_sz) is an externally allocated buffer holding
the read-only segment to be loaded into the VM. WARNING: The rodata
area required doing load (rodata_footprint) is slightly larger than
the area mapped into the VM (rodata_sz). See FD_SBPF_RODATA_GUARD.
area required doing load (rodata_footprint) is larger than the area
mapped into the VM (rodata_sz).
[text,text+8*text_cnt) is a sub-region of the read-only segment
containing executable code. */
Expand Down

0 comments on commit 2e27ed1

Please sign in to comment.