Skip to content
This repository has been archived by the owner on Oct 17, 2022. It is now read-only.

Commit

Permalink
Kill all bpf_get_obj_by_fd syscall usage [2/3]
Browse files Browse the repository at this point in the history
* Seems our kernel doesn't like this

Change-Id: Ic4296b394914e9ebf04e841374576f1faacc9460
  • Loading branch information
asuka-mio committed Aug 17, 2022
1 parent 82eed90 commit 50a4dec
Showing 1 changed file with 0 additions and 60 deletions.
60 changes: 0 additions & 60 deletions libbpf_android/Loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -638,52 +638,6 @@ static std::optional<unique_fd> getMapBtfInfo(const char* elfPath,
return btfFd;
}

static bool mapMatchesExpectations(unique_fd& fd, string& mapName, struct bpf_map_def& mapDef,
enum bpf_map_type type) {
// bpfGetFd... family of functions require at minimum a 4.14 kernel,
// so on 4.9 kernels just pretend the map matches our expectations.
// This isn't really a problem as we only really support 4.14+ anyway...
// Additionally we'll get almost equivalent test coverage on newer devices/kernels.
// This is because the primary failure mode we're trying to detect here
// is either a source code misconfiguration (which is likely kernel independent)
// or a newly introduced kernel feature/bug (which is unlikely to get backported to 4.9).
if (!isAtLeastKernelVersion(4, 14, 0)) return true;

// These asserts should *never* trigger, if one of them somehow does,
// it probably means a bpf .o file has been changed/replaced at runtime
// and bpfloader was manually rerun (normally it should only run *once*
// early during the boot process).
// Another possibility is that something is misconfigured in the code:
// most likely a shared map is declared twice differently.
// But such a change should never be checked into the source tree...
int fd_type = bpfGetFdMapType(fd);
int fd_key_size = bpfGetFdKeySize(fd);
int fd_value_size = bpfGetFdValueSize(fd);
int fd_max_entries = bpfGetFdMaxEntries(fd);
int fd_map_flags = bpfGetFdMapFlags(fd);

// DEVMAPs are readonly from the bpf program side's point of view, as such
// the kernel in kernel/bpf/devmap.c dev_map_init_map() will set the flag
int desired_map_flags = (int)mapDef.map_flags;
if (type == BPF_MAP_TYPE_DEVMAP || type == BPF_MAP_TYPE_DEVMAP_HASH)
desired_map_flags |= BPF_F_RDONLY_PROG;

// If anything doesn't match, just close the fd - it's of no use anyway.
if (fd_type != type) fd.reset();
if (fd_key_size != (int)mapDef.key_size) fd.reset();
if (fd_value_size != (int)mapDef.value_size) fd.reset();
if (fd_max_entries != (int)mapDef.max_entries) fd.reset();
if (fd_map_flags != desired_map_flags) fd.reset();

if (fd.ok()) return true;

ALOGE("bpf map name %s mismatch: desired/found: "
"type:%d/%d key:%u/%d value:%u/%d entries:%u/%d flags:%u/%d",
mapName.c_str(), type, fd_type, mapDef.key_size, fd_key_size, mapDef.value_size,
fd_value_size, mapDef.max_entries, fd_max_entries, desired_map_flags, fd_map_flags);
return false;
}

static int createMaps(const char* elfPath, ifstream& elfFile, vector<unique_fd>& mapFds,
const char* prefix, const unsigned long long allowedDomainBitmask,
const size_t sizeOfBpfMapDef) {
Expand Down Expand Up @@ -844,11 +798,6 @@ static int createMaps(const char* elfPath, ifstream& elfFile, vector<unique_fd>&

if (!fd.ok()) return -saved_errno;

// When reusing a pinned map, we need to check the map type/sizes/etc match, but for
// safety (since reuse code path is rare) run these checks even if we just created it.
// We assume failure is due to pinned map mismatch, hence the 'NOT UNIQUE' return code.
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) +
Expand Down Expand Up @@ -886,15 +835,6 @@ static int createMaps(const char* elfPath, ifstream& elfFile, vector<unique_fd>&
}
}

struct bpf_map_info map_info = {};
__u32 map_info_len = sizeof(map_info);
int rv = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
if (rv) {
ALOGE("bpf_obj_get_info_by_fd failed, ret: %d [%d]", rv, errno);
} else {
ALOGI("map %s id %d", mapPinLoc.c_str(), map_info.id);
}

mapFds.push_back(std::move(fd));
}

Expand Down

4 comments on commit 50a4dec

@EmanuelCN
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a proper way of fixing it, EmanuelCN/kernel_xiaomi_sm8250@299989f to fix your issuse.

@asuka-mio
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a proper way of fixing it, EmanuelCN/kernel_xiaomi_sm8250@299989f to fix your issuse.

Okay thx

@asuka-mio
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a proper way of fixing it, EmanuelCN/kernel_xiaomi_sm8250@299989f to fix your issuse.

But we should consider prebuilt kernel compatibility so we must keep this workaround

@EmanuelCN
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a proper way of fixing it, EmanuelCN/kernel_xiaomi_sm8250@299989f to fix your issuse.

But we should consider prebuilt kernel compatibility so we must keep this workaround

I don't think prebuilt kernels should be used anymore (especially that source is available), however this is only for xiaomi 4.19 platform any other device won't have this issuse, it's just xiaomi's modifications. Up to you now.

Please sign in to comment.