From 32be06f45f8c6dd2a7fe32fa53c209bc0a733c76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 10 Dec 2022 11:40:13 +0000 Subject: [PATCH] verify java map key/value struct size matches file descriptor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (this should avoid kernel reading/writing from out of bounds) Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: I71fe71eee4e4e6a917477eef5fd2266439e803f3 --- .../com/android/net/module/util/BpfMap.java | 10 ++-- .../native/bpf_headers/include/bpf/BpfUtils.h | 26 ++-------- .../bpf_headers/include/bpf/KernelVersion.h | 49 +++++++++++++++++++ staticlibs/native/bpfmapjni/Android.bp | 2 +- .../com_android_net_module_util_BpfMap.cpp | 25 ++++++++-- 5 files changed, 81 insertions(+), 31 deletions(-) create mode 100644 staticlibs/native/bpf_headers/include/bpf/KernelVersion.h diff --git a/staticlibs/device/com/android/net/module/util/BpfMap.java b/staticlibs/device/com/android/net/module/util/BpfMap.java index 2267f318df..9042085952 100644 --- a/staticlibs/device/com/android/net/module/util/BpfMap.java +++ b/staticlibs/device/com/android/net/module/util/BpfMap.java @@ -67,8 +67,10 @@ public class BpfMap implements IBpfMap private static ConcurrentHashMap, ParcelFileDescriptor> sFdCache = new ConcurrentHashMap<>(); - private static ParcelFileDescriptor cachedBpfFdGet(String path, int mode) + private static ParcelFileDescriptor cachedBpfFdGet(String path, int mode, + int keySize, int valueSize) throws ErrnoException, NullPointerException { + // TODO: key should include keySize & valueSize, but really we should match specific types Pair key = Pair.create(path, mode); // unlocked fetch is safe: map is concurrent read capable, and only inserted into ParcelFileDescriptor fd = sFdCache.get(key); @@ -79,7 +81,7 @@ public class BpfMap implements IBpfMap fd = sFdCache.get(key); if (fd != null) return fd; // okay, we really haven't opened this before... - fd = ParcelFileDescriptor.adoptFd(nativeBpfFdGet(path, mode)); + fd = ParcelFileDescriptor.adoptFd(nativeBpfFdGet(path, mode, keySize, valueSize)); sFdCache.put(key, fd); return fd; } @@ -94,11 +96,11 @@ public class BpfMap implements IBpfMap */ public BpfMap(@NonNull final String path, final int flag, final Class key, final Class value) throws ErrnoException, NullPointerException { - mMapFd = cachedBpfFdGet(path, flag); mKeyClass = key; mValueClass = value; mKeySize = Struct.getSize(key); mValueSize = Struct.getSize(value); + mMapFd = cachedBpfFdGet(path, flag, mKeySize, mValueSize); } /** @@ -295,7 +297,7 @@ public class BpfMap implements IBpfMap } } - private static native int nativeBpfFdGet(String path, int mode) + private static native int nativeBpfFdGet(String path, int mode, int keySize, int valueSize) throws ErrnoException, NullPointerException; // Note: the following methods appear to not require the object by virtue of taking the diff --git a/staticlibs/native/bpf_headers/include/bpf/BpfUtils.h b/staticlibs/native/bpf_headers/include/bpf/BpfUtils.h index 28d4b8bba5..e2cb67681c 100644 --- a/staticlibs/native/bpf_headers/include/bpf/BpfUtils.h +++ b/staticlibs/native/bpf_headers/include/bpf/BpfUtils.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include #include @@ -25,9 +26,10 @@ #include #include -#include #include +#include "KernelVersion.h" + namespace android { namespace bpf { @@ -84,27 +86,5 @@ static inline int setrlimitForTest() { return res; } -#define KVER(a, b, c) (((a) << 24) + ((b) << 16) + (c)) - -static inline unsigned uncachedKernelVersion() { - struct utsname buf; - if (uname(&buf)) return 0; - - unsigned kver_major = 0; - unsigned kver_minor = 0; - unsigned kver_sub = 0; - (void)sscanf(buf.release, "%u.%u.%u", &kver_major, &kver_minor, &kver_sub); - return KVER(kver_major, kver_minor, kver_sub); -} - -static inline unsigned kernelVersion() { - static unsigned kver = uncachedKernelVersion(); - return kver; -} - -static inline bool isAtLeastKernelVersion(unsigned major, unsigned minor, unsigned sub) { - return kernelVersion() >= KVER(major, minor, sub); -} - } // namespace bpf } // namespace android diff --git a/staticlibs/native/bpf_headers/include/bpf/KernelVersion.h b/staticlibs/native/bpf_headers/include/bpf/KernelVersion.h new file mode 100644 index 0000000000..e0e53a932e --- /dev/null +++ b/staticlibs/native/bpf_headers/include/bpf/KernelVersion.h @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +namespace android { +namespace bpf { + +#define KVER(a, b, c) (((a) << 24) + ((b) << 16) + (c)) + +static inline unsigned uncachedKernelVersion() { + struct utsname buf; + if (uname(&buf)) return 0; + + unsigned kver_major = 0; + unsigned kver_minor = 0; + unsigned kver_sub = 0; + (void)sscanf(buf.release, "%u.%u.%u", &kver_major, &kver_minor, &kver_sub); + return KVER(kver_major, kver_minor, kver_sub); +} + +static inline unsigned kernelVersion() { + static unsigned kver = uncachedKernelVersion(); + return kver; +} + +static inline bool isAtLeastKernelVersion(unsigned major, unsigned minor, unsigned sub) { + return kernelVersion() >= KVER(major, minor, sub); +} + +} // namespace bpf +} // namespace android diff --git a/staticlibs/native/bpfmapjni/Android.bp b/staticlibs/native/bpfmapjni/Android.bp index cd254d4c73..8babcce3f7 100644 --- a/staticlibs/native/bpfmapjni/Android.bp +++ b/staticlibs/native/bpfmapjni/Android.bp @@ -23,7 +23,7 @@ cc_library_static { "com_android_net_module_util_TcUtils.cpp", ], header_libs: [ - "bpf_syscall_wrappers", + "bpf_headers", "jni_headers", ], shared_libs: [ diff --git a/staticlibs/native/bpfmapjni/com_android_net_module_util_BpfMap.cpp b/staticlibs/native/bpfmapjni/com_android_net_module_util_BpfMap.cpp index 2e88fc83dc..2146d17c7f 100644 --- a/staticlibs/native/bpfmapjni/com_android_net_module_util_BpfMap.cpp +++ b/staticlibs/native/bpfmapjni/com_android_net_module_util_BpfMap.cpp @@ -25,15 +25,34 @@ #define BPF_FD_JUST_USE_INT #include "BpfSyscallWrappers.h" +#include "bpf/KernelVersion.h" + namespace android { static jint com_android_net_module_util_BpfMap_nativeBpfFdGet(JNIEnv *env, jclass clazz, - jstring path, jint mode) { + jstring path, jint mode, jint keySize, jint valueSize) { ScopedUtfChars pathname(env, path); jint fd = bpf::bpfFdGet(pathname.c_str(), static_cast(mode)); - if (fd < 0) jniThrowErrnoException(env, "nativeBpfFdGet", errno); + if (fd < 0) { + jniThrowErrnoException(env, "nativeBpfFdGet", errno); + return -1; + } + + if (bpf::isAtLeastKernelVersion(4, 14, 0)) { + // These likely fail with -1 and set errno to EINVAL on <4.14 + if (bpf::bpfGetFdKeySize(fd) != keySize) { + close(fd); + jniThrowErrnoException(env, "nativeBpfFdGet KeySize", EBADFD); + return -1; + } + if (bpf::bpfGetFdValueSize(fd) != valueSize) { + close(fd); + jniThrowErrnoException(env, "nativeBpfFdGet ValueSize", EBADFD); + return -1; + } + } return fd; } @@ -103,7 +122,7 @@ static jboolean com_android_net_module_util_BpfMap_nativeFindMapEntry(JNIEnv *en */ static const JNINativeMethod gMethods[] = { /* name, signature, funcPtr */ - { "nativeBpfFdGet", "(Ljava/lang/String;I)I", + { "nativeBpfFdGet", "(Ljava/lang/String;III)I", (void*) com_android_net_module_util_BpfMap_nativeBpfFdGet }, { "nativeWriteToMapEntry", "(I[B[BI)V", (void*) com_android_net_module_util_BpfMap_nativeWriteToMapEntry },