From 355b8df7f57bdfa6afe474a93b14c31e8c19d7b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Fri, 2 Jul 2021 08:01:05 +0000 Subject: [PATCH] bpf loader improvements for better long term compatibility with mainline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is bpfLoader v0.1, previously we had no version number, the version immediately preceding this commit we'll call v0.0. Versions older than that are either pre-S and don't matter, since loading mainline eBpf code was only added in S, or are from early during the S development cycle (ie. pre-March 5th 2021 or earlier) and simply no longer supported (no need to maintain compatibility). Bug: 190519702 Test: atest, TreeHugger - existing bpf programs load examination of bpfloader logs Signed-off-by: Maciej Żenczykowski Original-Change: https://android-review.googlesource.com/1754794 Merged-In: I36fa5b917540be7ea3ecfddc5fe7834e9eb18d88 Change-Id: I36fa5b917540be7ea3ecfddc5fe7834e9eb18d88 --- libbpf_android/Loader.cpp | 158 +++++++++++++++++++++++++++++++++--- progs/include/bpf_helpers.h | 41 +++++++++- progs/include/bpf_map_def.h | 40 +++++++++ 3 files changed, 225 insertions(+), 14 deletions(-) diff --git a/libbpf_android/Loader.cpp b/libbpf_android/Loader.cpp index dbd198d..c10988d 100644 --- a/libbpf_android/Loader.cpp +++ b/libbpf_android/Loader.cpp @@ -28,6 +28,12 @@ #include #include +// This is BpfLoader 0.1, we need to define this prior to including bpf_map_def.h +// to get the 0.1 struct definitions +#define BPFLOADER_VERSION_MAJOR 0u +#define BPFLOADER_VERSION_MINOR 1u +#define BPFLOADER_VERSION ((BPFLOADER_VERSION_MAJOR << 16) | BPFLOADER_VERSION_MINOR) + #include "../progs/include/bpf_map_def.h" #include "bpf/BpfUtils.h" #include "include/libbpf_android.h" @@ -205,6 +211,29 @@ static int readSectionByName(const char* name, ifstream& elfFile, vector& return -2; } +static unsigned int readSectionUint(const char* name, ifstream& elfFile, unsigned int defVal) { + vector theBytes; + int ret = readSectionByName(name, elfFile, theBytes); + if (ret) { + ALOGD("Couldn't find section %s (defaulting to %u [0x%x]).\n", name, defVal, defVal); + return defVal; + } else if (theBytes.size() < sizeof(unsigned int)) { + ALOGE("Section %s too short (defaulting to %u [0x%x]).\n", name, defVal, defVal); + return defVal; + } else { + // decode first 4 bytes as LE32 uint, there will likely be more bytes due to alignment. + unsigned int value = static_cast(theBytes[3]); + value <<= 8; + value += static_cast(theBytes[2]); + value <<= 8; + value += static_cast(theBytes[1]); + value <<= 8; + value += static_cast(theBytes[0]); + ALOGI("Section %s value is %u [0x%x]\n", name, value, value); + return value; + } +} + static int readSectionByType(ifstream& elfFile, int type, vector& data) { int ret; vector shTable; @@ -281,14 +310,36 @@ static bool isRelSection(codeSection& cs, string& name) { return false; } -static int readProgDefs(ifstream& elfFile, vector& pd) { +static int readProgDefs(ifstream& elfFile, vector& pd, + size_t sizeOfBpfProgDef) { vector pdData; int ret = readSectionByName("progs", elfFile, pdData); + // Older file formats do not require a 'progs' section at all. + // (We should probably figure out whether this is behaviour which is safe to remove now.) if (ret == -2) return 0; if (ret) return ret; - pd.resize(pdData.size() / sizeof(struct bpf_prog_def)); - memcpy(pd.data(), pdData.data(), pdData.size()); + if (pdData.size() % sizeOfBpfProgDef) { + ALOGE("readProgDefs failed due to improper sized progs section, %zu %% %zu != 0\n", + pdData.size(), sizeOfBpfProgDef); + return -1; + }; + + int progCount = pdData.size() / sizeOfBpfProgDef; + pd.resize(progCount); + size_t trimmedSize = std::min(sizeOfBpfProgDef, sizeof(struct bpf_prog_def)); + + const char* dataPtr = pdData.data(); + for (auto& p : pd) { + // First we zero initialize + memset(&p, 0, sizeof(p)); + // Then we set non-zero defaults + p.bpfloader_max_ver = DEFAULT_BPFLOADER_MAX_VER; // v1.0 + // Then we copy over the structure prefix from the ELF file. + memcpy(&p, dataPtr, trimmedSize); + // Move to next struct in the ELF file + dataPtr += sizeOfBpfProgDef; + } return 0; } @@ -335,7 +386,7 @@ static int getSectionSymNames(ifstream& elfFile, const string& sectionName, vect } /* Read a section by its index - for ex to get sec hdr strtab blob */ -static int readCodeSections(ifstream& elfFile, vector& cs) { +static int readCodeSections(ifstream& elfFile, vector& cs, size_t sizeOfBpfProgDef) { vector shTable; int entries, ret = 0; @@ -344,7 +395,7 @@ static int readCodeSections(ifstream& elfFile, vector& cs) { entries = shTable.size(); vector pd; - ret = readProgDefs(elfFile, pd); + ret = readProgDefs(elfFile, pd, sizeOfBpfProgDef); if (ret) return ret; vector progDefNames; ret = getSectionSymNames(elfFile, "progs", progDefNames); @@ -416,7 +467,7 @@ static int getSymNameByIdx(ifstream& elfFile, int index, string& name) { } static int createMaps(const char* elfPath, ifstream& elfFile, vector& mapFds, - const char* prefix) { + const char* prefix, size_t sizeOfBpfMapDef) { int ret; vector mdData; vector md; @@ -426,8 +477,28 @@ static int createMaps(const char* elfPath, ifstream& elfFile, vector& ret = readSectionByName("maps", elfFile, mdData); if (ret == -2) return 0; // no maps to read if (ret) return ret; - md.resize(mdData.size() / sizeof(struct bpf_map_def)); - memcpy(md.data(), mdData.data(), mdData.size()); + + if (mdData.size() % sizeOfBpfMapDef) { + ALOGE("createMaps failed due to improper sized maps section, %zu %% %zu != 0\n", + mdData.size(), sizeOfBpfMapDef); + return -1; + }; + + int mapCount = mdData.size() / sizeOfBpfMapDef; + md.resize(mapCount); + size_t trimmedSize = std::min(sizeOfBpfMapDef, sizeof(struct bpf_map_def)); + + const char* dataPtr = mdData.data(); + for (auto& m : md) { + // First we zero initialize + memset(&m, 0, sizeof(m)); + // Then we set non-zero defaults + m.bpfloader_max_ver = DEFAULT_BPFLOADER_MAX_VER; // v1.0 + // Then we copy over the structure prefix from the ELF file. + memcpy(&m, dataPtr, trimmedSize); + // Move to next struct in the ELF file + dataPtr += sizeOfBpfMapDef; + } ret = getSectionSymNames(elfFile, "maps", mapNames); if (ret) return ret; @@ -436,10 +507,24 @@ static int createMaps(const char* elfPath, ifstream& elfFile, vector& unique_fd fd; int saved_errno; // Format of pin location is /sys/fs/bpf/map__ - string mapPinLoc; + string mapPinLoc = + string(BPF_FS_PATH) + prefix + "map_" + fname + "_" + string(mapNames[i]); bool reuse = false; - mapPinLoc = string(BPF_FS_PATH) + prefix + "map_" + fname + "_" + string(mapNames[i]); + if (BPFLOADER_VERSION < md[i].bpfloader_min_ver) { + ALOGI("skipping map %s which requires bpfloader min ver 0x%05x\n", mapNames[i].c_str(), + md[i].bpfloader_min_ver); + mapFds.push_back(std::move(fd)); // -1 + continue; + } + + if (BPFLOADER_VERSION >= md[i].bpfloader_max_ver) { + ALOGI("skipping map %s which requires bpfloader max ver 0x%05x\n", mapNames[i].c_str(), + md[i].bpfloader_max_ver); + mapFds.push_back(std::move(fd)); // -1 + continue; + } + if (access(mapPinLoc.c_str(), F_OK) == 0) { fd.reset(bpf_obj_get(mapPinLoc.c_str())); saved_errno = errno; @@ -574,6 +659,8 @@ static int loadCodeSections(const char* elfPath, vector& cs, const for (int i = 0; i < (int)cs.size(); i++) { string name = cs[i].name; + unsigned bpfMinVer = DEFAULT_BPFLOADER_MIN_VER; // v0.0 + unsigned bpfMaxVer = DEFAULT_BPFLOADER_MAX_VER; // v1.0 if (cs[i].prog_def.has_value()) { unsigned min_kver = cs[i].prog_def->min_kver; @@ -582,8 +669,16 @@ static int loadCodeSections(const char* elfPath, vector& cs, const max_kver, kvers); if (kvers < min_kver) continue; if (kvers >= max_kver) continue; + + bpfMinVer = cs[i].prog_def->bpfloader_min_ver; + bpfMaxVer = cs[i].prog_def->bpfloader_max_ver; } + ALOGD("cs[%d].name:%s requires bpfloader version [0x%05x,0x%05x)\n", i, name.c_str(), + bpfMinVer, bpfMaxVer); + if (BPFLOADER_VERSION < bpfMinVer) continue; + if (BPFLOADER_VERSION >= bpfMaxVer) continue; + // strip any potential $foo suffix // this can be used to provide duplicate programs // conditionally loaded based on running kernel version @@ -674,7 +769,46 @@ int loadProg(const char* elfPath, bool* isCritical, const char* prefix) { elfPath, (char*)license.data()); } - ret = readCodeSections(elfFile, cs); + // the following default values are for bpfloader V0.0 format which does not include them + unsigned int bpfLoaderMinVer = + readSectionUint("bpfloader_min_ver", elfFile, DEFAULT_BPFLOADER_MIN_VER); + unsigned int bpfLoaderMaxVer = + readSectionUint("bpfloader_max_ver", elfFile, DEFAULT_BPFLOADER_MAX_VER); + size_t sizeOfBpfMapDef = + readSectionUint("size_of_bpf_map_def", elfFile, DEFAULT_SIZEOF_BPF_MAP_DEF); + size_t sizeOfBpfProgDef = + readSectionUint("size_of_bpf_prog_def", elfFile, DEFAULT_SIZEOF_BPF_PROG_DEF); + + // inclusive lower bound check + if (BPFLOADER_VERSION < bpfLoaderMinVer) { + ALOGI("BpfLoader version 0x%05x ignoring ELF object %s with min ver 0x%05x\n", + BPFLOADER_VERSION, elfPath, bpfLoaderMinVer); + return 0; + } + + // exclusive upper bound check + if (BPFLOADER_VERSION >= bpfLoaderMaxVer) { + ALOGI("BpfLoader version 0x%05x ignoring ELF object %s with max ver 0x%05x\n", + BPFLOADER_VERSION, elfPath, bpfLoaderMaxVer); + return 0; + } + + ALOGI("BpfLoader version 0x%05x processing ELF object %s with ver [0x%05x,0x%05x)\n", + BPFLOADER_VERSION, elfPath, bpfLoaderMinVer, bpfLoaderMaxVer); + + if (sizeOfBpfMapDef < DEFAULT_SIZEOF_BPF_MAP_DEF) { + ALOGE("sizeof(bpf_map_def) of %zu is too small (< %d)\n", sizeOfBpfMapDef, + DEFAULT_SIZEOF_BPF_MAP_DEF); + return -1; + } + + if (sizeOfBpfProgDef < DEFAULT_SIZEOF_BPF_PROG_DEF) { + ALOGE("sizeof(bpf_prog_def) of %zu is too small (< %d)\n", sizeOfBpfProgDef, + DEFAULT_SIZEOF_BPF_PROG_DEF); + return -1; + } + + ret = readCodeSections(elfFile, cs, sizeOfBpfProgDef); if (ret) { ALOGE("Couldn't read all code sections in %s\n", elfPath); return ret; @@ -683,7 +817,7 @@ int loadProg(const char* elfPath, bool* isCritical, const char* prefix) { /* Just for future debugging */ if (0) dumpAllCs(cs); - ret = createMaps(elfPath, elfFile, mapFds, prefix); + ret = createMaps(elfPath, elfFile, mapFds, prefix, sizeOfBpfMapDef); if (ret) { ALOGE("Failed to create maps: (ret=%d) in %s\n", ret, elfPath); return ret; diff --git a/progs/include/bpf_helpers.h b/progs/include/bpf_helpers.h index 075ea94..82ed72e 100644 --- a/progs/include/bpf_helpers.h +++ b/progs/include/bpf_helpers.h @@ -6,11 +6,47 @@ #include "bpf_map_def.h" +/****************************************************************************** + * WARNING: CHANGES TO THIS FILE OUTSIDE OF AOSP/MASTER ARE LIKELY TO BREAK * + * DEVICE COMPATIBILITY WITH MAINLINE MODULES SHIPPING EBPF CODE. * + * * + * THIS WILL LIKELY RESULT IN BRICKED DEVICES AT SOME ARBITRARY FUTURE TIME * + * * + * THAT GOES ESPECIALLY FOR THE 'SEC' 'LICENSE' AND 'CRITICAL' MACRO DEFINES * + * * + * We strongly suggest that if you need changes to bpfloader functionality * + * you get your changes reviewed and accepted into aosp/master. * + * * + ******************************************************************************/ + /* place things in different elf sections */ #define SEC(NAME) __attribute__((section(NAME), used)) -/* Example use: LICENSE("GPL"); or LICENSE("Apache 2.0"); */ -#define LICENSE(NAME) char _license[] SEC("license") = (NAME) +/* Must be present in every program, example usage: + * LICENSE("GPL"); or LICENSE("Apache 2.0"); + * + * We also take this opportunity to embed a bunch of other useful values in + * the resulting .o (This is to enable some limited forward compatibility + * with mainline module shipped ebpf programs) + * + * The bpfloader_{min/max}_ver defines the [min, max) range of bpfloader + * versions that should load this .o file (bpfloaders outside of this range + * will simply ignore/skip this *entire* .o) + * The [inclusive,exclusive) matches what we do for kernel ver dependencies. + * + * The size_of_bpf_{map,prog}_def allow the bpfloader to load programs where + * these structures have been extended with additional fields (they will of + * course simply be ignored then). + * + * If missing, bpfloader_{min/max}_ver default to 0/0x10000 ie. [v0.0, v1.0), + * while size_of_bpf_{map/prog}_def default to 32/20 which are the v0.0 sizes. + */ +#define LICENSE(NAME) \ + unsigned int _bpfloader_min_ver SEC("bpfloader_min_ver") = DEFAULT_BPFLOADER_MIN_VER; \ + unsigned int _bpfloader_max_ver SEC("bpfloader_max_ver") = DEFAULT_BPFLOADER_MAX_VER; \ + size_t _size_of_bpf_map_def SEC("size_of_bpf_map_def") = sizeof(struct bpf_map_def); \ + size_t _size_of_bpf_prog_def SEC("size_of_bpf_prog_def") = sizeof(struct bpf_prog_def); \ + char _license[] SEC("license") = (NAME) /* flag the resulting bpf .o file as critical to system functionality, * loading all kernel version appropriate programs in it must succeed @@ -68,6 +104,7 @@ static int (*bpf_map_delete_elem_unsafe)(const struct bpf_map_def* map, .key_size = sizeof(TypeOfKey), \ .value_size = sizeof(TypeOfValue), \ .max_entries = (num_entries), \ + .map_flags = 0, \ .uid = (usr), \ .gid = (grp), \ .mode = (md), \ diff --git a/progs/include/bpf_map_def.h b/progs/include/bpf_map_def.h index 50a822c..d80bfe7 100644 --- a/progs/include/bpf_map_def.h +++ b/progs/include/bpf_map_def.h @@ -25,6 +25,34 @@ // Pull in AID_* constants from //system/core/libcutils/include/private/android_filesystem_config.h #include +/****************************************************************************** + * * + * ! ! ! W A R N I N G ! ! ! * + * * + * CHANGES TO THESE STRUCTURE DEFINITIONS OUTSIDE OF AOSP/MASTER *WILL* BREAK * + * MAINLINE MODULE COMPATIBILITY * + * * + * AND THUS MAY RESULT IN YOUR DEVICE BRICKING AT SOME ARBITRARY POINT IN * + * THE FUTURE * + * * + * (and even in aosp/master you may only append new fields at the very end, * + * you may *never* delete fields, change their types, ordering, insert in * + * the middle, etc. If a mainline module using the old definition has * + * already shipped (which happens roughly monthly), then it's set in stone) * + * * + ******************************************************************************/ + +// For now we default to v0.0 format +#ifndef BPFLOADER_VERSION +#define BPFLOADER_VERSION 0u +#endif + +// These are the values used if these fields are missing +#define DEFAULT_BPFLOADER_MIN_VER 0u // v0.0 +#define DEFAULT_BPFLOADER_MAX_VER 0x10000u // v1.0 +#define DEFAULT_SIZEOF_BPF_MAP_DEF 32 // v0.0 struct: enum + alignment padding + 7 uint +#define DEFAULT_SIZEOF_BPF_PROG_DEF 20 // v0.0 struct: 4 uint + bool + alignment padding + /* * Map structure to be used by Android eBPF C programs. The Android eBPF loader * uses this structure from eBPF object to create maps at boot time. @@ -51,6 +79,12 @@ struct bpf_map_def { unsigned int uid; // uid_t unsigned int gid; // gid_t unsigned int mode; // mode_t + +#if BPFLOADER_VERSION >= 1u + // The following fields were added in version 0.1 + unsigned int bpfloader_min_ver; // if missing, defaults to 0, ie. v0.0 + unsigned int bpfloader_max_ver; // if missing, defaults to 0x10000, ie. v1.0 +#endif }; struct bpf_prog_def { @@ -62,4 +96,10 @@ struct bpf_prog_def { unsigned int max_kver; bool optional; // program section (ie. function) may fail to load, continue onto next func. + +#if BPFLOADER_VERSION >= 1u + // The following fields were added in version 0.1 + unsigned int bpfloader_min_ver; // if missing, defaults to 0, ie. v0.0 + unsigned int bpfloader_max_ver; // if missing, defaults to 0x10000, ie. v1.0 +#endif };