diff --git a/loader/BpfLoadTest.cpp b/loader/BpfLoadTest.cpp index f3a8f85..8a2d072 100644 --- a/loader/BpfLoadTest.cpp +++ b/loader/BpfLoadTest.cpp @@ -58,7 +58,6 @@ class BpfLoadTest : public TestWithParam { Location loc = { .dir = "", .prefix = "", - .allowedDomainBitmask = 0, .allowedProgTypes = kAllowed, .allowedProgTypesLength = arraysize(kAllowed), }; diff --git a/loader/BpfLoader.cpp b/loader/BpfLoader.cpp index 7a68f99..c05d1f4 100644 --- a/loader/BpfLoader.cpp +++ b/loader/BpfLoader.cpp @@ -50,7 +50,6 @@ #include "bpf/BpfUtils.h" using android::base::EndsWith; -using android::bpf::domain; using std::string; bool exists(const char* const path) { @@ -93,7 +92,6 @@ const android::bpf::Location locations[] = { { .dir = "/system/etc/bpf/", .prefix = "", - .allowedDomainBitmask = domainToBitmask(domain::platform), .allowedProgTypes = kPlatformAllowedProgTypes, .allowedProgTypesLength = arraysize(kPlatformAllowedProgTypes), }, @@ -101,7 +99,6 @@ const android::bpf::Location locations[] = { { .dir = "/system/etc/bpf/uprobestats/", .prefix = "uprobestats/", - .allowedDomainBitmask = domainToBitmask(domain::platform), .allowedProgTypes = kUprobestatsAllowedProgTypes, .allowedProgTypesLength = arraysize(kUprobestatsAllowedProgTypes), }, @@ -109,7 +106,6 @@ const android::bpf::Location locations[] = { { .dir = "/vendor/etc/bpf/", .prefix = "vendor/", - .allowedDomainBitmask = domainToBitmask(domain::vendor), .allowedProgTypes = kVendorAllowedProgTypes, .allowedProgTypesLength = arraysize(kVendorAllowedProgTypes), }, @@ -166,17 +162,9 @@ int main(int argc, char** argv) { (void)argc; android::base::InitLogging(argv, &android::base::KernelLogger); - // Create all the pin subdirectories - // (this must be done first to allow selinux_context and pin_subdir functionality, - // which could otherwise fail with ENOENT during object pinning or renaming, - // due to ordering issues) - for (const auto& location : locations) { - if (createSysFsBpfSubDir(location.prefix)) return 1; - } - // Load all ELF objects, create programs and maps, and pin them for (const auto& location : locations) { - if (loadAllElfObjects(location) != 0) { + if (createSysFsBpfSubDir(location.prefix) || loadAllElfObjects(location)) { ALOGE("=== CRITICAL FAILURE LOADING BPF PROGRAMS FROM %s ===", location.dir); ALOGE("If this triggers reliably, you're probably missing kernel options or patches."); ALOGE("If this triggers randomly, you might be hitting some memory allocation " @@ -187,7 +175,7 @@ int main(int argc, char** argv) { } } - if (android::base::SetProperty("bpf.progs_loaded", "1") == false) { + if (!android::base::SetProperty("bpf.progs_loaded", "1")) { ALOGE("Failed to set bpf.progs_loaded property"); return 1; } diff --git a/loader/Loader.cpp b/loader/Loader.cpp index 305c60e..98ddd26 100644 --- a/loader/Loader.cpp +++ b/loader/Loader.cpp @@ -71,60 +71,6 @@ namespace bpf { static unsigned int page_size = static_cast(getpagesize()); -constexpr const char* lookupSelinuxContext(const domain d, const char* const unspecified = "") { - switch (d) { - case domain::unspecified: return unspecified; - case domain::platform: return "fs_bpf"; - case domain::vendor: return "fs_bpf_vendor"; - case domain::loader: return "fs_bpf_loader"; - default: return "(unrecognized)"; - } -} - -domain getDomainFromSelinuxContext(const char s[BPF_SELINUX_CONTEXT_CHAR_ARRAY_SIZE]) { - for (domain d : AllDomains) { - // Not sure how to enforce this at compile time, so abort() bpfloader at boot instead - if (strlen(lookupSelinuxContext(d)) >= BPF_SELINUX_CONTEXT_CHAR_ARRAY_SIZE) abort(); - if (!strncmp(s, lookupSelinuxContext(d), BPF_SELINUX_CONTEXT_CHAR_ARRAY_SIZE)) return d; - } - ALOGW("ignoring unrecognized selinux_context '%-32s'", s); - // We should return 'unrecognized' here, however: returning unspecified will - // result in the system simply using the default context, which in turn - // will allow future expansion by adding more restrictive selinux types. - // Older bpfloader will simply ignore that, and use the less restrictive default. - // This does mean you CANNOT later add a *less* restrictive type than the default. - // - // Note: we cannot just abort() here as this might be a mainline module shipped optional update - return domain::unspecified; -} - -constexpr const char* lookupPinSubdir(const domain d, const char* const unspecified = "") { - switch (d) { - case domain::unspecified: return unspecified; - case domain::platform: return "/"; - case domain::vendor: return "vendor/"; - case domain::loader: return "loader/"; - default: return "(unrecognized)"; - } -}; - -domain getDomainFromPinSubdir(const char s[BPF_PIN_SUBDIR_CHAR_ARRAY_SIZE]) { - for (domain d : AllDomains) { - // Not sure how to enforce this at compile time, so abort() bpfloader at boot instead - if (strlen(lookupPinSubdir(d)) >= BPF_PIN_SUBDIR_CHAR_ARRAY_SIZE) abort(); - if (!strncmp(s, lookupPinSubdir(d), BPF_PIN_SUBDIR_CHAR_ARRAY_SIZE)) return d; - } - ALOGE("unrecognized pin_subdir '%-32s'", s); - // pin_subdir affects the object's full pathname, - // and thus using the default would change the location and thus our code's ability to find it, - // hence this seems worth treating as a true error condition. - // - // Note: we cannot just abort() here as this might be a mainline module shipped optional update - // However, our callers will treat this as an error, and stop loading the specific .o, - // which will fail bpfloader if the .o is marked critical. - return domain::unrecognized; -} - static string pathToObjName(const string& path) { // extract everything after the final slash, ie. this is the filename 'foo@1.o' or 'bar.o' string filename = android::base::Split(path, "/").back(); @@ -585,7 +531,7 @@ static bool mapMatchesExpectations(const unique_fd& fd, const string& mapName, } static int createMaps(const char* elfPath, ifstream& elfFile, vector& mapFds, - const char* prefix, const unsigned long long allowedDomainBitmask) { + const char* prefix) { int ret; vector mdData; vector md; @@ -646,34 +592,10 @@ static int createMaps(const char* elfPath, ifstream& elfFile, vector& if (max_entries < page_size) max_entries = page_size; } - domain selinux_context = getDomainFromSelinuxContext(md[i].selinux_context); - if (specified(selinux_context)) { - if (!inDomainBitmask(selinux_context, allowedDomainBitmask)) { - ALOGE("map %s has invalid selinux_context of %d (allowed bitmask 0x%llx)", - mapNames[i].c_str(), selinux_context, allowedDomainBitmask); - return -EINVAL; - } - ALOGI("map %s selinux_context [%-32s] -> %d -> '%s' (%s)", mapNames[i].c_str(), - md[i].selinux_context, selinux_context, lookupSelinuxContext(selinux_context), - lookupPinSubdir(selinux_context)); - } - - domain pin_subdir = getDomainFromPinSubdir(md[i].pin_subdir); - if (unrecognized(pin_subdir)) return -ENOTDIR; - if (specified(pin_subdir)) { - if (!inDomainBitmask(pin_subdir, allowedDomainBitmask)) { - ALOGE("map %s has invalid pin_subdir of %d (allowed bitmask 0x%llx)", - mapNames[i].c_str(), pin_subdir, allowedDomainBitmask); - return -EINVAL; - } - ALOGI("map %s pin_subdir [%-32s] -> %d -> '%s'", mapNames[i].c_str(), md[i].pin_subdir, - pin_subdir, lookupPinSubdir(pin_subdir)); - } - - // Format of pin location is /sys/fs/bpf/map__ + // Format of pin location is /sys/fs/bpf/map__ // except that maps shared across .o's have empty // Note: refers to the extension-less basename of the .o file (without @ suffix). - string mapPinLoc = string(BPF_FS_PATH) + lookupPinSubdir(pin_subdir, prefix) + "map_" + + string mapPinLoc = string(BPF_FS_PATH) + prefix + "map_" + (md[i].shared ? "" : objName) + "_" + mapNames[i]; bool reuse = false; unique_fd fd; @@ -706,30 +628,11 @@ static int createMaps(const char* elfPath, ifstream& elfFile, vector& if (!mapMatchesExpectations(fd, mapNames[i], md[i], type)) return -ENOTUNIQ; if (!reuse) { - if (specified(selinux_context)) { - string createLoc = string(BPF_FS_PATH) + lookupPinSubdir(selinux_context) + - "tmp_map_" + objName + "_" + mapNames[i]; - ret = bpfFdPin(fd, createLoc.c_str()); - if (ret) { - int err = errno; - ALOGE("create %s -> %d [%d:%s]", createLoc.c_str(), ret, err, strerror(err)); - return -err; - } - ret = renameat2(AT_FDCWD, createLoc.c_str(), - AT_FDCWD, mapPinLoc.c_str(), RENAME_NOREPLACE); - if (ret) { - int err = errno; - ALOGE("rename %s %s -> %d [%d:%s]", createLoc.c_str(), mapPinLoc.c_str(), ret, - err, strerror(err)); - return -err; - } - } else { - ret = bpfFdPin(fd, mapPinLoc.c_str()); - if (ret) { - int err = errno; - ALOGE("pin %s -> %d [%d:%s]", mapPinLoc.c_str(), ret, err, strerror(err)); - return -err; - } + ret = bpfFdPin(fd, mapPinLoc.c_str()); + if (ret) { + int err = errno; + ALOGE("pin %s -> %d [%d:%s]", mapPinLoc.c_str(), ret, err, strerror(err)); + return -err; } ret = chmod(mapPinLoc.c_str(), md[i].mode); if (ret) { @@ -836,7 +739,7 @@ static void applyMapRelo(ifstream& elfFile, vector &mapFds, vector& cs, const string& license, - const char* prefix, const unsigned long long allowedDomainBitmask) { + const char* prefix) { unsigned kvers = kernelVersion(); if (!kvers) { @@ -863,32 +766,6 @@ static int loadCodeSections(const char* elfPath, vector& cs, const if (kvers < min_kver) continue; if (kvers >= max_kver) continue; - domain selinux_context = getDomainFromSelinuxContext(cs[i].prog_def->selinux_context); - domain pin_subdir = getDomainFromPinSubdir(cs[i].prog_def->pin_subdir); - - if (unrecognized(pin_subdir)) return -ENOTDIR; - - if (specified(selinux_context)) { - if (!inDomainBitmask(selinux_context, allowedDomainBitmask)) { - ALOGE("prog %s has invalid selinux_context of %d (allowed bitmask 0x%llx)", - name.c_str(), selinux_context, allowedDomainBitmask); - return -EINVAL; - } - ALOGI("prog %s selinux_context [%-32s] -> %d -> '%s' (%s)", name.c_str(), - cs[i].prog_def->selinux_context, selinux_context, - lookupSelinuxContext(selinux_context), lookupPinSubdir(selinux_context)); - } - - if (specified(pin_subdir)) { - if (!inDomainBitmask(pin_subdir, allowedDomainBitmask)) { - ALOGE("prog %s has invalid pin_subdir of %d (allowed bitmask 0x%llx)", name.c_str(), - pin_subdir, allowedDomainBitmask); - return -EINVAL; - } - ALOGI("prog %s pin_subdir [%-32s] -> %d -> '%s'", name.c_str(), - cs[i].prog_def->pin_subdir, pin_subdir, lookupPinSubdir(pin_subdir)); - } - // strip any potential $foo suffix // this can be used to provide duplicate programs // conditionally loaded based on running kernel version @@ -897,7 +774,7 @@ static int loadCodeSections(const char* elfPath, vector& cs, const bool reuse = false; // Format of pin location is // /sys/fs/bpf/prog__ - string progPinLoc = string(BPF_FS_PATH) + lookupPinSubdir(pin_subdir, prefix) + "prog_" + + string progPinLoc = string(BPF_FS_PATH) + prefix + "prog_" + objName + '_' + string(name); if (access(progPinLoc.c_str(), F_OK) == 0) { fd.reset(retrieveProgram(progPinLoc.c_str())); @@ -942,30 +819,11 @@ static int loadCodeSections(const char* elfPath, vector& cs, const if (!fd.ok()) return fd.get(); if (!reuse) { - if (specified(selinux_context)) { - string createLoc = string(BPF_FS_PATH) + lookupPinSubdir(selinux_context) + - "tmp_prog_" + objName + '_' + string(name); - ret = bpfFdPin(fd, createLoc.c_str()); - if (ret) { - int err = errno; - ALOGE("create %s -> %d [%d:%s]", createLoc.c_str(), ret, err, strerror(err)); - return -err; - } - ret = renameat2(AT_FDCWD, createLoc.c_str(), - AT_FDCWD, progPinLoc.c_str(), RENAME_NOREPLACE); - if (ret) { - int err = errno; - ALOGE("rename %s %s -> %d [%d:%s]", createLoc.c_str(), progPinLoc.c_str(), ret, - err, strerror(err)); - return -err; - } - } else { - ret = bpfFdPin(fd, progPinLoc.c_str()); - if (ret) { - int err = errno; - ALOGE("create %s -> %d [%d:%s]", progPinLoc.c_str(), ret, err, strerror(err)); - return -err; - } + ret = bpfFdPin(fd, progPinLoc.c_str()); + if (ret) { + int err = errno; + ALOGE("create %s -> %d [%d:%s]", progPinLoc.c_str(), ret, err, strerror(err)); + return -err; } if (chmod(progPinLoc.c_str(), 0440)) { int err = errno; @@ -1029,7 +887,7 @@ int loadProg(const char* elfPath, bool* isCritical, const Location& location) { /* Just for future debugging */ if (0) dumpAllCs(cs); - ret = createMaps(elfPath, elfFile, mapFds, location.prefix, location.allowedDomainBitmask); + ret = createMaps(elfPath, elfFile, mapFds, location.prefix); if (ret) { ALOGE("Failed to create maps: (ret=%d) in %s", ret, elfPath); return ret; @@ -1040,8 +898,7 @@ int loadProg(const char* elfPath, bool* isCritical, const Location& location) { applyMapRelo(elfFile, mapFds, cs); - ret = loadCodeSections(elfPath, cs, string(license.data()), location.prefix, - location.allowedDomainBitmask); + ret = loadCodeSections(elfPath, cs, string(license.data()), location.prefix); if (ret) ALOGE("Failed to load programs, loadCodeSections ret=%d", ret); return ret; diff --git a/loader/include/libbpf_android.h b/loader/include/libbpf_android.h index 172a2db..0a98a1d 100644 --- a/loader/include/libbpf_android.h +++ b/loader/include/libbpf_android.h @@ -24,55 +24,9 @@ namespace android { namespace bpf { -// Bpf programs may specify per-program & per-map selinux_context and pin_subdir. -// -// The BpfLoader needs to convert these bpf.o specified strings into an enum -// for internal use (to check that valid values were specified for the specific -// location of the bpf.o file). -// -// It also needs to map selinux_context's into pin_subdir's. -// This is because of how selinux_context is actually implemented via pin+rename. -// -// Thus 'domain' enumerates all selinux_context's/pin_subdir's that the BpfLoader -// is aware of. Thus there currently needs to be a 1:1 mapping between the two. -// -enum class domain : int { - unrecognized = -1, // invalid for this version of the bpfloader - unspecified = 0, // means just use the default for that specific pin location - platform, // fs_bpf /sys/fs/bpf - vendor, // (T+) fs_bpf_vendor /sys/fs/bpf/vendor - loader, // (U+) fs_bpf_loader /sys/fs/bpf/loader -}; - -// Note: this does not include domain::unrecognized, but does include domain::unspecified -static constexpr domain AllDomains[] = { - domain::unspecified, - domain::platform, - domain::vendor, - domain::loader, -}; - -static constexpr bool unrecognized(domain d) { - return d == domain::unrecognized; -} - -// Note: this doesn't handle unrecognized, handle it first. -static constexpr bool specified(domain d) { - return d != domain::unspecified; -} - -static constexpr unsigned long long domainToBitmask(domain d) { - return specified(d) ? 1uLL << (static_cast(d) - 1) : 0; -} - -static constexpr bool inDomainBitmask(domain d, unsigned long long v) { - return domainToBitmask(d) & v; -} - struct Location { const char* const dir = ""; const char* const prefix = ""; - unsigned long long allowedDomainBitmask = 0; const bpf_prog_type* allowedProgTypes = nullptr; size_t allowedProgTypesLength = 0; };