From ad7725a43891a392dd24c0afa28f784186ec1fc9 Mon Sep 17 00:00:00 2001 From: Richard Patel Date: Tue, 12 Dec 2023 22:01:41 +0000 Subject: [PATCH] sbpf: undo rodata footprint optimization 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. --- src/ballet/sbpf/fd_sbpf_loader.c | 30 +++--------------------------- src/ballet/sbpf/fd_sbpf_loader.h | 6 +++--- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/src/ballet/sbpf/fd_sbpf_loader.c b/src/ballet/sbpf/fd_sbpf_loader.c index 99ae395f7d..65ca6cd056 100644 --- a/src/ballet/sbpf/fd_sbpf_loader.c +++ b/src/ballet/sbpf/fd_sbpf_loader.c @@ -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 @@ -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; } @@ -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; @@ -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 ); @@ -876,9 +861,6 @@ fd_sbpf_r_bpf_64_relative( fd_sbpf_elf_t const * elf, REQUIRE( va!=0UL ); va = va 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 ) ); @@ -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 ); @@ -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 ); diff --git a/src/ballet/sbpf/fd_sbpf_loader.h b/src/ballet/sbpf/fd_sbpf_loader.h index 58c7829f0a..d7b90974eb 100644 --- a/src/ballet/sbpf/fd_sbpf_loader.h +++ b/src/ballet/sbpf/fd_sbpf_loader.h @@ -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" */ @@ -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. */