search

iovisor / bcc

BCC - Tools for BPF-based Linux IO analysis, networking, monitoring, and more
Apache License 2.0
20.36k stars 3.86k forks source link

Backported probe_user_read* probe_kernel_read* are not used by bcc #5006

Closed nitanmarcel closed 4 months ago

nitanmarcel commented 4 months ago

Table of contents

I'm trying to backport probe_{user/kernel}_read* to my android 4.9 kernel on adnroid 14 and I run into some issues with bcc not using them.

While running opensnoop, the argument is still empty hint that it's still using bpf_probe_read.

Testing the newly added methods they all have a return value of -14.


# b' id.gms.unstable-6051  [001] d..1  1271.064263: : bpf_probe_read_user: ret=-14, data=""'
# b' id.gms.unstable-6051  [001] d..1  1271.064271: : bpf_probe_read_kernel: ret=-14, data=""'
# b' id.gms.unstable-6051  [001] d..1  1271.064277: : bpf_probe_read_user_str: ret=-14, data=""'
# b' id.gms.unstable-6051  [001] d..1  1271.064281: : bpf_probe_read_kernel_str: ret=-14, data=""'

from bcc import BPF

# BPF program code
bpf_code = """
#include <linux/uaccess.h>

int test_bpf_functions(void *ctx) {
    char user_buf[100];
    char kernel_buf[100];
    const char *addr = "Hello, World!";

    // Test bpf_probe_read_user
    int ret_user = bpf_probe_read_user(user_buf, sizeof(user_buf), addr);
    bpf_trace_printk("bpf_probe_read_user: ret=%d, data=\\"%s\\"\\n", ret_user, user_buf);

    // Test bpf_probe_read_kernel
    int ret_kernel = bpf_probe_read_kernel(kernel_buf, sizeof(kernel_buf), addr);
    bpf_trace_printk("bpf_probe_read_kernel: ret=%d, data=\\"%s\\"\\n", ret_kernel, kernel_buf);

    // Test bpf_probe_read_user_str
    int ret_user_str = bpf_probe_read_user_str(user_buf, sizeof(user_buf), addr);
    bpf_trace_printk("bpf_probe_read_user_str: ret=%d, data=\\"%s\\"\\n", ret_user_str, user_buf);

    // Test bpf_probe_read_kernel_str
    int ret_kernel_str = bpf_probe_read_kernel_str(kernel_buf, sizeof(kernel_buf), addr);
    bpf_trace_printk("bpf_probe_read_kernel_str: ret=%d, data=\\"%s\\"\\n", ret_kernel_str, kernel_buf);

    return 0;
}
"""

# Load BPF program
bpf = BPF(text=bpf_code)
bpf.attach_kprobe(event=bpf.get_syscall_fnname("clone"), fn_name="test_bpf_functions")

# Print trace output
bpf.trace_print()

I've been using bcc built from ExtendedAndroidTools tool repo, could maybe that cause an issue?

This is the current diff I'm working on

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 744b4763b80e..de94c13b7193 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -559,6 +559,43 @@ enum bpf_func_id {
    */
    BPF_FUNC_probe_read_user,

+   /**
+   * int bpf_probe_read_kernel(void *dst, int size, void *src)
+   *     Read a kernel pointer safely.
+   *     Return: 0 on success or negative error
+   */
+   BPF_FUNC_probe_read_kernel,
+
+   /**
+    * int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)
+    *     Copy a NUL terminated string from user unsafe address. In case the string
+    *     length is smaller than size, the target is not padded with further NUL
+    *     bytes. In case the string length is larger than size, just count-1
+    *     bytes are copied and the last byte is set to NUL.
+    *     @dst: destination address
+    *     @size: maximum number of bytes to copy, including the trailing NUL
+    *     @unsafe_ptr: unsafe address
+    *     Return:
+    *       > 0 length of the string including the trailing NUL on success
+    *       < 0 error
+    */
+   BPF_FUNC_probe_read_user_str,
+
+   /**
+    * int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)
+    *     Copy a NUL terminated string from unsafe address. In case the string
+    *     length is smaller than size, the target is not padded with further NUL
+    *     bytes. In case the string length is larger than size, just count-1
+    *     bytes are copied and the last byte is set to NUL.
+    *     @dst: destination address
+    *     @size: maximum number of bytes to copy, including the trailing NUL
+    *     @unsafe_ptr: unsafe address
+    *     Return:
+    *       > 0 length of the string including the trailing NUL on success
+    *       < 0 error
+    */
+   BPF_FUNC_probe_read_kernel_str,
+
    __BPF_FUNC_MAX_ID,
 };

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index a1e37a5d8c88..3478ca744a45 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -94,7 +94,7 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
    .arg3_type  = ARG_ANYTHING,
 };

-BPF_CALL_3(bpf_probe_read_user, void *, dst, u32, size, const void *, unsafe_ptr)
+BPF_CALL_3(bpf_probe_read_user, void *, dst, u32, size, const void  __user *, unsafe_ptr)
 {
    int ret;

@@ -115,6 +115,27 @@ static const struct bpf_func_proto bpf_probe_read_user_proto = {
 };

+BPF_CALL_3(bpf_probe_read_kernel, void *, dst, u32, size, const void *, unsafe_ptr)
+{
+   int ret;
+
+   ret = probe_kernel_read(dst, unsafe_ptr, size);
+   if (unlikely(ret < 0))
+       memset(dst, 0, size);
+
+   return ret;
+}
+
+static const struct bpf_func_proto bpf_probe_read_kernel_proto = {
+   .func       = bpf_probe_read_kernel,
+   .gpl_only   = true,
+   .ret_type   = RET_INTEGER,
+   .arg1_type  = ARG_PTR_TO_RAW_STACK,
+   .arg2_type  = ARG_CONST_STACK_SIZE,
+   .arg3_type  = ARG_ANYTHING,
+};
+
+
 BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
       u32, size)
 {
@@ -487,6 +508,69 @@ static const struct bpf_func_proto bpf_probe_read_str_proto = {
    .arg3_type  = ARG_ANYTHING,
 };

+
+
+BPF_CALL_3(bpf_probe_read_user_str, void *, dst, u32, size,
+      const void __user *, unsafe_ptr)
+{
+   int ret;
+
+   /*
+    * The strncpy_from_unsafe() call will likely not fill the entire
+    * buffer, but that's okay in this circumstance as we're probing
+    * arbitrary memory anyway similar to bpf_probe_read() and might
+    * as well probe the stack. Thus, memory is explicitly cleared
+    * only in error case, so that improper users ignoring return
+    * code altogether don't copy garbage; otherwise length of string
+    * is returned that can be used for bpf_perf_event_output() et al.
+    */
+   ret = strncpy_from_unsafe_user(dst, unsafe_ptr, size);
+   if (unlikely(ret < 0))
+       memset(dst, 0, size);
+
+   return ret;
+}
+
+static const struct bpf_func_proto bpf_probe_read_user_str_proto = {
+   .func       = bpf_probe_read_user_str,
+   .gpl_only   = true,
+   .ret_type   = RET_INTEGER,
+   .arg1_type  = ARG_PTR_TO_RAW_STACK,
+   .arg2_type  = ARG_CONST_STACK_SIZE,
+   .arg3_type  = ARG_ANYTHING,
+};
+
+
+BPF_CALL_3(bpf_probe_read_kernel_str, void *, dst, u32, size,
+      const void *, unsafe_ptr)
+{
+   int ret;
+
+   /*
+    * The strncpy_from_unsafe() call will likely not fill the entire
+    * buffer, but that's okay in this circumstance as we're probing
+    * arbitrary memory anyway similar to bpf_probe_read() and might
+    * as well probe the stack. Thus, memory is explicitly cleared
+    * only in error case, so that improper users ignoring return
+    * code altogether don't copy garbage; otherwise length of string
+    * is returned that can be used for bpf_perf_event_output() et al.
+    */
+   ret = strncpy_from_unsafe(dst, unsafe_ptr, size);
+   if (unlikely(ret < 0))
+       memset(dst, 0, size);
+
+   return ret;
+}
+
+static const struct bpf_func_proto bpf_probe_read_kernel_str_proto = {
+   .func       = bpf_probe_read_kernel_str,
+   .gpl_only   = true,
+   .ret_type   = RET_INTEGER,
+   .arg1_type  = ARG_PTR_TO_RAW_STACK,
+   .arg2_type  = ARG_CONST_STACK_SIZE,
+   .arg3_type  = ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
 {
    switch (func_id) {
@@ -500,8 +584,14 @@ static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
        return &bpf_probe_read_proto;
    case BPF_FUNC_probe_read_user:
        return &bpf_probe_read_user_proto;
+   case BPF_FUNC_probe_read_kernel:
+       return &bpf_probe_read_kernel_proto;
    case BPF_FUNC_probe_read_str:
        return &bpf_probe_read_str_proto;
+   case BPF_FUNC_probe_read_user_str:
+       return &bpf_probe_read_user_str_proto;
+   case BPF_FUNC_probe_read_kernel_str:
+       return &bpf_probe_read_kernel_proto;
    case BPF_FUNC_ktime_get_ns:
        return &bpf_ktime_get_ns_proto;
    case BPF_FUNC_tail_call:
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 155ce25c069d..91d5691288a7 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -522,7 +522,44 @@ enum bpf_func_id {
    *     Return: 0 on success or negative error
    */
    BPF_FUNC_probe_read_user,
+
+   /**
+   * int bpf_probe_read_kernel(void *dst, int size, void *src)
+   *     Read a kernel pointer safely.
+   *     Return: 0 on success or negative error
+   */
+   BPF_FUNC_probe_read_kernel,

+   /**
+    * int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)
+    *     Copy a NUL terminated string from user unsafe address. In case the string
+    *     length is smaller than size, the target is not padded with further NUL
+    *     bytes. In case the string length is larger than size, just count-1
+    *     bytes are copied and the last byte is set to NUL.
+    *     @dst: destination address
+    *     @size: maximum number of bytes to copy, including the trailing NUL
+    *     @unsafe_ptr: unsafe address
+    *     Return:
+    *       > 0 length of the string including the trailing NUL on success
+    *       < 0 error
+    */
+   BPF_FUNC_probe_read_user_str,
+
+   /**
+    * int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)
+    *     Copy a NUL terminated string from unsafe address. In case the string
+    *     length is smaller than size, the target is not padded with further NUL
+    *     bytes. In case the string length is larger than size, just count-1
+    *     bytes are copied and the last byte is set to NUL.
+    *     @dst: destination address
+    *     @size: maximum number of bytes to copy, including the trailing NUL
+    *     @unsafe_ptr: unsafe address
+    *     Return:
+    *       > 0 length of the string including the trailing NUL on success
+    *       < 0 error
+    */
+   BPF_FUNC_probe_read_kernel_str,
+  
   __BPF_FUNC_MAX_ID,
 };

This is also a follow-up of the following patch that adds probe_read_user which now I see it didn't worked either

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 67d7d771a944..744b4763b80e 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -552,6 +552,13 @@ enum bpf_func_id {
     */
    BPF_FUNC_get_socket_uid,

+   /**
+   * int bpf_probe_read_user(void *dst, int size, void *src)
+   *     Read a userspace pointer safely.
+   *     Return: 0 on success or negative error
+   */
+   BPF_FUNC_probe_read_user,
+
    __BPF_FUNC_MAX_ID,
 };

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 59182e6d6f51..a1e37a5d8c88 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -94,35 +94,27 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
    .arg3_type  = ARG_ANYTHING,
 };

-BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size, const void *, unsafe_ptr)
+BPF_CALL_3(bpf_probe_read_user, void *, dst, u32, size, const void *, unsafe_ptr)
 {
    int ret;

-   /*
-    * The strncpy_from_unsafe() call will likely not fill the entire
-    * buffer, but that's okay in this circumstance as we're probing
-    * arbitrary memory anyway similar to bpf_probe_read() and might
-    * as well probe the stack. Thus, memory is explicitly cleared
-    * only in error case, so that improper users ignoring return
-    * code altogether don't copy garbage; otherwise length of string
-    * is returned that can be used for bpf_perf_event_output() et al.
-    */
-   ret = strncpy_from_unsafe(dst, unsafe_ptr, size);
+   ret = probe_user_read(dst, unsafe_ptr, size);
    if (unlikely(ret < 0))
        memset(dst, 0, size);

    return ret;
 }

-static const struct bpf_func_proto bpf_probe_read_str_proto = {
-   .func           = bpf_probe_read_str,
-   .gpl_only       = true,
-   .ret_type       = RET_INTEGER,
+static const struct bpf_func_proto bpf_probe_read_user_proto = {
+   .func       = bpf_probe_read_user,
+   .gpl_only   = true,
+   .ret_type   = RET_INTEGER,
    .arg1_type  = ARG_PTR_TO_RAW_STACK,
    .arg2_type  = ARG_CONST_STACK_SIZE,
    .arg3_type  = ARG_ANYTHING,
 };

+
 BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
       u32, size)
 {
@@ -506,6 +498,8 @@ static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
        return &bpf_map_delete_elem_proto;
    case BPF_FUNC_probe_read:
        return &bpf_probe_read_proto;
+   case BPF_FUNC_probe_read_user:
+       return &bpf_probe_read_user_proto;
    case BPF_FUNC_probe_read_str:
        return &bpf_probe_read_str_proto;
    case BPF_FUNC_ktime_get_ns:
@@ -534,8 +528,6 @@ static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
        return &bpf_current_task_under_cgroup_proto;
    case BPF_FUNC_get_prandom_u32:
        return &bpf_get_prandom_u32_proto;
-   case BPF_FUNC_probe_read_str:
-       return &bpf_probe_read_str_proto;
    default:
        return NULL;
    }
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a339bea1f4c8..155ce25c069d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -516,7 +516,14 @@ enum bpf_func_id {
     */
    BPF_FUNC_get_socket_uid,

-   __BPF_FUNC_MAX_ID,
+   /**
+   * int bpf_probe_read_user(void *dst, int size, void *src)
+   *     Read a userspace pointer safely.
+   *     Return: 0 on success or negative error
+   */
+   BPF_FUNC_probe_read_user,
+   
+  __BPF_FUNC_MAX_ID,
 };

 /* All flags used by eBPF helper functions, placed here. */
nitanmarcel commented 4 months ago

Ok, this seems to habe been more an issue with adb root, doing echo 0 > /proc/sys/kernel/kptr_restrict made the kallsym adresses to work show non 0 values. Now there's another issue, invalid func 113.

Looks like it now sees the probe_read_kernel but it fails to use it

bpf: Failed to load program: Invalid argument
0: (bf) r6 = r1
1: (85) call 14
2: (7b) *(u64 *)(r10 -8) = r0
3: (b7) r1 = 0
4: (7b) *(u64 *)(r10 -16) = r1
5: (7b) *(u64 *)(r10 -24) = r1
6: (7b) *(u64 *)(r10 -32) = r1
7: (7b) *(u64 *)(r10 -40) = r1
8: (7b) *(u64 *)(r10 -48) = r1
9: (7b) *(u64 *)(r10 -56) = r1
10: (7b) *(u64 *)(r10 -64) = r1
11: (7b) *(u64 *)(r10 -72) = r1
12: (7b) *(u64 *)(r10 -80) = r1
13: (7b) *(u64 *)(r10 -88) = r1
14: (7b) *(u64 *)(r10 -96) = r1
15: (7b) *(u64 *)(r10 -104) = r1
16: (7b) *(u64 *)(r10 -112) = r1
17: (7b) *(u64 *)(r10 -120) = r1
18: (7b) *(u64 *)(r10 -128) = r1
19: (7b) *(u64 *)(r10 -136) = r1
20: (7b) *(u64 *)(r10 -144) = r1
21: (7b) *(u64 *)(r10 -152) = r1
22: (7b) *(u64 *)(r10 -160) = r1
23: (7b) *(u64 *)(r10 -168) = r1
24: (7b) *(u64 *)(r10 -176) = r1
25: (7b) *(u64 *)(r10 -184) = r1
26: (7b) *(u64 *)(r10 -192) = r1
27: (7b) *(u64 *)(r10 -200) = r1
28: (7b) *(u64 *)(r10 -208) = r1
29: (7b) *(u64 *)(r10 -216) = r1
30: (7b) *(u64 *)(r10 -224) = r1
31: (7b) *(u64 *)(r10 -232) = r1
32: (7b) *(u64 *)(r10 -240) = r1
33: (7b) *(u64 *)(r10 -248) = r1
34: (7b) *(u64 *)(r10 -256) = r1
35: (7b) *(u64 *)(r10 -264) = r1
36: (7b) *(u64 *)(r10 -272) = r1
37: (7b) *(u64 *)(r10 -280) = r1
38: (7b) *(u64 *)(r10 -288) = r1
39: (7b) *(u64 *)(r10 -296) = r1
40: (7b) *(u64 *)(r10 -304) = r1
41: (85) call 5
42: (bf) r7 = r0
43: (bf) r1 = r10
44: (07) r1 += -280
45: (b7) r2 = 16
46: (85) call 113
invalid func 113

HINT: bpf_probe_read_kernel missing (added in Linux 5.5).

Traceback (most recent call last):
  File "/data/local/tmp/out/bpftools/share/bcc/tools/opensnoop", line 385, in <module>
    b.attach_kretprobe(event=fnname_open, fn_name="trace_return")
  File "/data/local/tmp/out/bpftools/lib/python3.10/site-packages/bcc-0.29.1+eb8ede2d-py3.10.egg/bcc/__init__.py", line 885, in attach_kretprobe
  File "/data/local/tmp/out/bpftools/lib/python3.10/site-packages/bcc-0.29.1+eb8ede2d-py3.10.egg/bcc/__init__.py", line 526, in load_func
Exception: Failed to load BPF program b'trace_return': Invalid argument
nitanmarcel commented 4 months ago

bpf: Failed to load program: Invalid argument

After this error I assumed that the arguments are wrong, and somewhere around the road ARG_PTR_TO_RAW_STACK and ARG_CONST_STACK_SIZE changed.

I finally gave up on that patch and instead I've wen't with [bpf: fix userspace access for bpf_probe_read{, str}()](https://lore.kernel.org/all/358062d4-fdf8-f3da-fd8e-c55cf1a089ec@amazon.com/#r) and there seems to not be any issues so far and the string arguments work fines.

Here is my diff for kernel 4.9 if someone needs it

bpf: fix userspace access for bpf_probe_read{, str}()

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1c0b4001aca2..31675f1e489d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -5,6 +5,7 @@ config ARM
    select ARCH_HAS_DEVMEM_IS_ALLOWED
    select ARCH_HAS_ELF_RANDOMIZE
    select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
+   select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
    select ARCH_HAVE_CUSTOM_GPIO_H
    select ARCH_HAS_GCOV_PROFILE_ALL
    select ARCH_MIGHT_HAVE_PC_PARPORT
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9bba90cffb50..f0b753556b7e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -14,6 +14,7 @@ config ARM64
    select ARCH_HAS_GIGANTIC_PAGE
    select ARCH_HAS_KCOV
    select ARCH_HAS_SG_CHAIN
+   select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
    select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
    select ARCH_INLINE_READ_LOCK if !PREEMPT
    select ARCH_INLINE_READ_LOCK_BH if !PREEMPT
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2351ad8657c7..80d76a6461f8 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -85,6 +85,7 @@ config PPC
    select BINFMT_ELF
    select ARCH_HAS_ELF_RANDOMIZE
    select ARCH_HAS_FORTIFY_SOURCE
+   select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
    select OF
    select OF_DMA_DEFAULT_COHERENT      if !NOT_COHERENT_CACHE
    select OF_EARLY_FLATTREE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f5c19fe2eb2c..d47581cc7ba0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -33,6 +33,7 @@ config X86
    select ARCH_HAS_MMIO_FLUSH
    select ARCH_HAS_SG_CHAIN
    select ARCH_HAS_UBSAN_SANITIZE_ALL
+   select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
    select ARCH_HAVE_NMI_SAFE_CMPXCHG
    select ARCH_MIGHT_HAVE_ACPI_PDC     if ACPI
    select ARCH_MIGHT_HAVE_PC_PARPORT
diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks
index 2a31a580ac44..595343eac1e5 100644
--- a/kernel/Kconfig.locks
+++ b/kernel/Kconfig.locks
@@ -252,3 +252,6 @@ config QUEUED_RWLOCKS
 config RWSEM_PRIO_AWARE
        def_bool y
        depends on RWSEM_XCHGADD_ALGORITHM
+
+config ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
+       bool
\ No newline at end of file
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 83b20092b84c..22c3d2593caa 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -77,8 +77,11 @@ EXPORT_SYMBOL_GPL(trace_call_bpf);
 BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
 {
    int ret;
-
+#ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
+   ret = probe_kernel_read(dst, unsafe_ptr, size);
+#else
    ret = probe_kernel_read(dst, unsafe_ptr, size);
+#endif
    if (unlikely(ret < 0))
        memset(dst, 0, size);

@@ -107,7 +110,11 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size, const void *, unsafe_ptr)
     * code altogether don't copy garbage; otherwise length of string
     * is returned that can be used for bpf_perf_event_output() et al.
     */
+#ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
+   ret = strncpy_from_unsafe_user(dst, unsafe_ptr, size);
+#else
    ret = strncpy_from_unsafe(dst, unsafe_ptr, size);
+#endif
    if (unlikely(ret < 0))
        memset(dst, 0, size);
nitanmarcel commented 4 months ago

Properly fixed by checking if the probed pointer is within user space or not.

https://github.com/nitanmarcel/android_kernel_oneplus_sdm845-bpf/commit/7dcf378f5f14072cfce26ceeb4ef3cf568623c0b

BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
{
    int ret;

    if ((unsigned long)unsafe_ptr < TASK_SIZE) {
        ret = bpf_probe_read_user(dst, size, unsafe_ptr);
    } else {
        ret = bpf_probe_read_kernel(dst, size, unsafe_ptr);
    }

    return ret;
}