Fixes for multiple bpf2bpf stack levels (#801)

Updates for multiple stack levels
Make read_elf() with a desired_section work correctly
Fixes #792

Signed-off-by: Dave Thaler <[email protected]>

src/asm_files.cpp

@@ -205,30 +205,28 @@ struct function_relocation {
     string target_function_name;
 };
 
-static vector<ebpf_inst> read_subprogram(const ELFIO::section& subprogram_section,
-                                         const ELFIO::const_symbol_section_accessor& symbols,
-                                         const string& symbol_name) {
+static raw_program* find_subprogram(vector<raw_program>& programs,
+                                    const ELFIO::section& subprogram_section,
+                                    const std::string& symbol_name) {
     // Find subprogram by name.
-    for (ELFIO::Elf_Xword subprogram_offset = 0; subprogram_offset < subprogram_section.get_size();) {
-        auto [subprogram_name, subprogram_size] =
-            get_program_name_and_size(subprogram_section, subprogram_offset, symbols);
-        if (subprogram_size == 0) {
-            throw UnmarshalError("Zero-size subprogram '" + subprogram_name + "' in section '" +
-                                 subprogram_section.get_name() + "'");
+    for (auto& subprog : programs) {
+        if ((subprog.section_name == subprogram_section.get_name()) && (subprog.function_name == symbol_name)) {
+            return &subprog;
         }
-        if (subprogram_name == symbol_name) {
-            // Append subprogram instructions to the main program.
-            return vector_of<ebpf_inst>(subprogram_section.get_data() + subprogram_offset, subprogram_size);
-        }
-        subprogram_offset += subprogram_size;
     }
-    throw UnmarshalError("Subprogram '" + symbol_name + "' not found in section '" + subprogram_section.get_name() +
-                         "'");
+    return nullptr;
 }
 
-static void append_subprograms(raw_program& prog, const vector<raw_program>& programs,
-                               const vector<function_relocation>& function_relocations, const ELFIO::elfio& reader,
-                               const ELFIO::const_symbol_section_accessor& symbols) {
+// Returns an error message, or empty string on success.
+static std::string append_subprograms(raw_program& prog, vector<raw_program>& programs,
+                                      const vector<function_relocation>& function_relocations, const ELFIO::elfio& reader,
+                                      const ELFIO::const_symbol_section_accessor& symbols) {
+    if (prog.resolved_subprograms) {
+        // We've already appended any relevant subprograms.
+        return {};
+    }
+    prog.resolved_subprograms = true;
+
     // Perform function relocations and fill in the inst.imm values of CallLocal instructions.
     std::map<string, ELFIO::Elf_Xword> subprogram_offsets;
     for (const auto& reloc : function_relocations) {
@@ -250,8 +248,25 @@ static void append_subprograms(raw_program& prog, const vector<raw_program>& pro
                                      std::to_string(reloc.source_offset));
             }
             ELFIO::section& subprogram_section = *reader.sections[section_index];
-            auto subprogram = read_subprogram(subprogram_section, symbols, symbol_name);
-            prog.prog.insert(prog.prog.end(), subprogram.begin(), subprogram.end());
+
+            if (auto subprogram = find_subprogram(programs, subprogram_section, symbol_name)) {
+                // Make sure subprogram has already had any subprograms of its own appended.
+                std::string error = append_subprograms(*subprogram, programs, function_relocations, reader, symbols);
+                if (!error.empty()) {
+                    return error;
+                }
+
+                // Append subprogram to program.
+                prog.prog.insert(prog.prog.end(), subprogram->prog.begin(), subprogram->prog.end());
+                for (int i = 0; i < subprogram->info.line_info.size(); i++) {
+                    prog.info.line_info[prog.info.line_info.size()] = subprogram->info.line_info[i];
+                }
+            } else {
+                // The program will be invalid, but continue rather than throwing an exception
+                // since we might be verifying a different program in the file.
+                return std::string("Subprogram '" + symbol_name + "' not found in section '" +
+                                   subprogram_section.get_name() + "'");
+            }
         }
 
         // Fill in the PC offset into the imm field of the CallLocal instruction.
@@ -263,6 +278,7 @@ static void append_subprograms(raw_program& prog, const vector<raw_program>& pro
         }
         prog.prog[reloc.source_offset].imm = gsl::narrow_cast<int32_t>(offset_diff);
     }
+    return {};
 }
 
 std::map<std::string, size_t> parse_map_section(const libbtf::btf_type_data& btf_data,
@@ -358,14 +374,8 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
     vector<string> unresolved_symbols_errors;
     vector<function_relocation> function_relocations;
     for (const auto& section : reader.sections) {
-        const string name = section->get_name();
-        if (!desired_section.empty() && name != desired_section) {
-            continue;
-        }
-        if (name == "license" || name == "version" || is_map_section(name)) {
-            continue;
-        }
-        if (name != ".text" && name.find('.') == 0) {
+        if (!(section->get_flags() & ELFIO::SHF_EXECINSTR)) {
+            // Section does not contain executable instructions.
             continue;
         }
         const auto section_size = section->get_size();
@@ -376,6 +386,7 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
         if (section_data == nullptr) {
             continue;
         }
+        const string name = section->get_name();
         info.type = platform->get_program_type(name, path);
 
         for (ELFIO::Elf_Xword program_offset = 0; program_offset < section_size;) {
@@ -446,14 +457,6 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
         }
     }
 
-    // Now that we have all programs in the list, we can recursively append any subprograms
-    // to the calling programs.  We have to keep them as programs themselves in case the caller
-    // wants to verify them separately, but we also have to append them if used as subprograms to
-    // allow the caller to be fully verified since inst.imm can only point into the same program.
-    for (auto& prog : res) {
-        append_subprograms(prog, res, function_relocations, reader, symbols);
-    }
-
     // Below, only relocations of symbols located in the map sections are allowed,
     // so if there are relocations there needs to be a maps section.
     if (!unresolved_symbols_errors.empty()) {
@@ -477,7 +480,6 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
                     }
                     program.info.line_info.insert_or_assign(
                         inst_index, btf_line_info_t{file_name, source, line_number, column_number});
-                    return;
                 }
             }
         };
@@ -486,7 +488,7 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
 
         // BTF doesn't include line info for every instruction, only on the first instruction per source line.
         for (auto& program : res) {
-            for (size_t i = 1; i < program.info.line_info.size(); i++) {
+            for (size_t i = 1; i < program.prog.size(); i++) {
                 // If the previous PC has line info, copy it.
                 if (program.info.line_info[i].line_number == 0 && program.info.line_info[i - 1].line_number != 0) {
                     program.info.line_info[i] = program.info.line_info[i - 1];
@@ -495,6 +497,29 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
         }
     }
 
+    // Now that we have all programs in the list, we can recursively append any subprograms
+    // to the calling programs.  We have to keep them as programs themselves in case the caller
+    // wants to verify them separately, but we also have to append them if used as subprograms to
+    // allow the caller to be fully verified since inst.imm can only point into the same program.
+    for (auto& prog : res) {
+        std::string error = append_subprograms(prog, res, function_relocations, reader, symbols);
+        if (!error.empty()) {
+            if (prog.section_name == desired_section) {
+                throw UnmarshalError(error);
+            }
+        }
+    }
+
+    // Now that we've incorporated any subprograms from other sections, we can narrow the list
+    // to return to just those programs in the desired section, if any.
+    if (!desired_section.empty() && !res.empty()) {
+        for (int index = res.size() - 1; index >= 0; index--) {
+            if (res[index].section_name != desired_section) {
+                res.erase(res.begin() + index);
+            }
+        }
+    }
+
     if (res.empty()) {
         if (desired_section.empty()) {
             throw UnmarshalError("Can't find any non-empty TEXT sections in file " + path);

src/spec_type_descriptors.hpp

@@ -70,6 +70,10 @@ struct raw_program {
     std::string function_name{};
     std::vector<ebpf_inst> prog{};
     program_info info{};
+
+    // Track whether subprograms for this raw_program have been resolved,
+    // preventing duplicate resolution when multiple programs exist in a section.
+    bool resolved_subprograms{};
 };
 
 void print_map_descriptors(const std::vector<EbpfMapDescriptor>& descriptors, std::ostream& o);

src/test/test_verify.cpp

@@ -508,8 +508,9 @@ TEST_SECTION("raw_tracepoint/filler/proc_startupdate_2")
 TEST_SECTION("raw_tracepoint/filler/sys_recvfrom_x")
 */
 
-TEST_PROGRAM_REJECT_FAIL("build", "bpf2bpf.o", ".text", "plus1", 1); // Subprogram should fail verification.
-TEST_PROGRAM("build", "bpf2bpf.o", "test", "func", 1);               // Subprogram can be called from main program.
+TEST_PROGRAM("build", "bpf2bpf.o", ".text", "add1", 2);
+TEST_PROGRAM("build", "bpf2bpf.o", ".text", "add2", 2);
+TEST_PROGRAM("build", "bpf2bpf.o", "test", "func", 1);
 
 TEST_SECTION("build", "byteswap.o", ".text")
 TEST_SECTION("build", "stackok.o", ".text")