Bug 388786 - Support bpf syscall in amd64 Linux
Summary: Support bpf syscall in amd64 Linux
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: 3.14 SVN
Platform: unspecified Linux
: NOR task
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
: 400878 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-01-10 15:44 UTC by spacewanderlzx@gmail.com
Modified: 2018-11-12 16:24 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Tha patch for supporting bpf syscall in amd64 Linux (4.74 KB, patch)
2018-01-10 15:44 UTC, spacewanderlzx@gmail.com
Details
[PATCH valgrind 1/3] Linux: Get pre_check for ASCII string out of PRE(sys_prctl) (3.44 KB, patch)
2018-04-19 13:46 UTC, Quentin Monnet
Details
[PATCH valgrind 2/3] Linux: amd64: Add support for bpf() syscall. (22.42 KB, patch)
2018-04-19 13:47 UTC, Quentin Monnet
Details
[PATCH valgrind 3/3] Linux: Add file descriptors tracking in wrappers for bpf() system call. (11.53 KB, patch)
2018-04-19 13:48 UTC, Quentin Monnet
Details
[PATCH valgrind v2 1/3] Linux: Get pre_check for ASCII string out of PRE(sys_prctl) (3.44 KB, patch)
2018-07-30 09:46 UTC, Quentin Monnet
Details
[PATCH valgrind v2 2/3] Linux: amd64: Add support for bpf() syscall. (24.68 KB, patch)
2018-07-30 09:47 UTC, Quentin Monnet
Details
[PATCH valgrind v2 3/3] Linux: Add file descriptors tracking in wrappers for bpf() system call. (15.55 KB, patch)
2018-07-30 09:48 UTC, Quentin Monnet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description spacewanderlzx@gmail.com 2018-01-10 15:44:03 UTC
Created attachment 109783 [details]
Tha patch for supporting bpf syscall in amd64 Linux

Hi everyone.
I just wrote a patch to support bpf syscall in amd64 Linux, following
this guide: http://valgrind.org/docs/manual/dist.readme-missing.html
It is my first time to hack valgrind, please let me know if I made any mistake.
I am glad to see this patch could be reviewed and accepted.
Thanks!

diff --git a/coregrind/m_syswrap/syswrap-amd64-linux.c
b/coregrind/m_syswrap/syswrap-amd64-linux.c
index 14ad6499e..a75048397 100644
--- a/coregrind/m_syswrap/syswrap-amd64-linux.c
+++ b/coregrind/m_syswrap/syswrap-amd64-linux.c
@@ -201,6 +201,7 @@ DECL_TEMPLATE(amd64_linux, sys_arch_prctl);
 DECL_TEMPLATE(amd64_linux, sys_ptrace);
 DECL_TEMPLATE(amd64_linux, sys_fadvise64);
 DECL_TEMPLATE(amd64_linux, sys_mmap);
+DECL_TEMPLATE(amd64_linux, sys_bpf);
 DECL_TEMPLATE(amd64_linux, sys_syscall184);


@@ -401,6 +402,14 @@ PRE(sys_mmap)
    SET_STATUS_from_SysRes(r);
 }

+PRE(sys_bpf)
+{
+
+   PRINT("sys_bpf ( %ld, %#lx, %lu )" , SARG1, ARG2, ARG3);
+   PRE_REG_READ3(int, "bpf",
+                 int, cmd, union vki_bpf_attr *, attr, unsigned int, size);
+}
+

 /* ---------------------------------------------------------------
    PRE/POST wrappers for AMD64/Linux-variant specific syscalls
@@ -839,10 +848,10 @@ static SyscallTableEntry syscall_table[] = {
    LINX_(__NR_renameat2,         sys_renameat2),        // 316
 //   LIN__(__NR_seccomp,           sys_ni_syscall),       // 317
    LINXY(__NR_getrandom,         sys_getrandom),        // 318
-   LINXY(__NR_memfd_create,      sys_memfd_create)      // 319
+   LINXY(__NR_memfd_create,      sys_memfd_create),      // 319

 //   LIN__(__NR_kexec_file_load,   sys_ni_syscall),       // 320
-//   LIN__(__NR_bpf,               sys_ni_syscall)        // 321
+   PLAX_(__NR_bpf,               sys_bpf),              // 321
 };

 SyscallTableEntry* ML_(get_linux_syscall_entry) ( UInt sysno )
diff --git a/include/vki/vki-amd64-linux.h b/include/vki/vki-amd64-linux.h
index a506ade06..293c4edf0 100644
--- a/include/vki/vki-amd64-linux.h
+++ b/include/vki/vki-amd64-linux.h
@@ -48,6 +48,7 @@ typedef unsigned int __vki_u32;

 typedef __signed__ long long __vki_s64;
 typedef unsigned long long __vki_u64;
+typedef __vki_u64 __attribute__((aligned(8))) __vki_aligned_u64;

 typedef unsigned short vki_u16;

@@ -697,6 +698,86 @@ struct vki_shminfo64 {
 #define VKI_TIOCGSERIAL     0x541E
 #define VKI_TIOCSSERIAL     0x541F

+//----------------------------------------------------------------------
+// From linux-4.14.13/include/uapi/linux/bpf.h
+//----------------------------------------------------------------------
+
+union bpf_attr {
+    struct { /* anonymous struct used by BPF_MAP_CREATE command */
+        __vki_u32    map_type;    /* one of enum bpf_map_type */
+        __vki_u32    key_size;    /* size of key in bytes */
+        __vki_u32    value_size;    /* size of value in bytes */
+        __vki_u32    max_entries;    /* max number of entries in a map */
+        __vki_u32    map_flags;    /* BPF_MAP_CREATE related
+                     * flags defined above.
+                     */
+        __vki_u32    inner_map_fd;    /* fd pointing to the inner map */
+        __vki_u32    numa_node;    /* numa node (effective only if
+                                 * BPF_F_NUMA_NODE is set).
+                                 */
+    };
+
+    struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
+        __vki_u32            map_fd;
+        __vki_aligned_u64    key;
+        union {
+            __vki_aligned_u64 value;
+            __vki_aligned_u64 next_key;
+        };
+        __vki_u64        flags;
+    };
+
+    struct { /* anonymous struct used by BPF_PROG_LOAD command */
+        __vki_u32            prog_type;    /* one of enum bpf_prog_type */
+        __vki_u32            insn_cnt;
+        __vki_aligned_u64    insns;
+        __vki_aligned_u64    license;
+        __vki_u32            log_level;    /* verbosity level of verifier */
+        __vki_u32            log_size;    /* size of user buffer */
+        __vki_aligned_u64    log_buf;    /* user supplied buffer */
+        __vki_u32            kern_version;    /* checked when
prog_type=kprobe */
+        __vki_u32            prog_flags;
+    };
+
+    struct { /* anonymous struct used by BPF_OBJ_* commands */
+        __vki_aligned_u64    pathname;
+        __vki_u32            bpf_fd;
+    };
+
+    struct { /* anonymous struct used by BPF_PROG_ATTACH/DETACH commands */
+        __vki_u32        target_fd;    /* container object to attach to */
+        __vki_u32        attach_bpf_fd;    /* eBPF program to attach */
+        __vki_u32        attach_type;
+        __vki_u32        attach_flags;
+    };
+
+    struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
+        __vki_u32            prog_fd;
+        __vki_u32            retval;
+        __vki_u32            data_size_in;
+        __vki_u32            data_size_out;
+        __vki_aligned_u64    data_in;
+        __vki_aligned_u64    data_out;
+        __vki_u32            repeat;
+        __vki_u32            duration;
+    } test;
+
+    struct { /* anonymous struct used by BPF_*_GET_*_ID */
+        union {
+            __vki_u32        start_id;
+            __vki_u32        prog_id;
+            __vki_u32        map_id;
+        };
+        __vki_u32        next_id;
+    };
+
+    struct { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */
+        __vki_u32            bpf_fd;
+        __vki_u32            info_len;
+        __vki_aligned_u64    info;
+    } info;
+} __attribute__((aligned(8)));
+
 //----------------------------------------------------------------------
 // And that's it!
 //----------------------------------------------------------------------
Comment 1 Quentin Monnet 2018-04-19 13:46:40 UTC
Created attachment 112113 [details]
[PATCH valgrind 1/3] Linux: Get pre_check for ASCII string out of PRE(sys_prctl)

Hi,

I am interested as well in getting support for the bpf() sytem call in Valgrind, and would like to help getting this bug resolved.

I also have a series that implements support for the syscall. I do not want to steal the spotlight from spacewanderlzx, it simply happens that I started to develop it before that bug was created, got distracted, then finished it and only realised that this bug existed after my patches were ready. Since my version seems more complete to me, I thought I would post it anyway. I hope this is not a problem, please tell me if that was inappropriate.

So this series is a set of three patches for supporting bpf() syscall on amd64 (as was the original patch). As far as I know, the syscall is not platform-specific and it should be trivial to later extend it to other architectures.

First patch is a minor refactoring of a snippet used to process and run pre-checks for ASCII strings to read from the parameters of a system call. It is extracted from PRE(sys_prctl) and moved to a static function, as it is reused in a later patch with PRE(sys_bpf) wrapper.

The second patch is the main one, and adds the PRE() and POST() wrappers for the bpf() system call. Since bpf() uses a set of subcommands, and process its arguments in different ways depending on the selected subcommand, the wrappers reflect this and add relevant pre- and post-checks for read and write operations for each of the available subcommands.

To make review easier, validation and tracking for file descriptors are only added in a third patch. It could be squashed with the previous patch, depending on maintainers' preferences.

For me as well, this is the first attempt at adding support for a system call to Valgrind, hence there is a number of implementation details I am unsure of. Please see in particular the notes I appended to the commit log of the second patch.

The patches compile and run. I tried to run valgrind on a program using bpf() syscalls, and everything goes fine. Tracking the file descriptors works too. However, I do not use valgrind so often and I do not know how to test the wrappers in-depth. For example, how could I validate that I used the correct sizes for pre- and post- read/write checks in the wrappers? On my simple tests, Valgrind would show no output difference when I would change the sizes or even remove some pre- or post-checks from the wrappers.
Comment 2 Quentin Monnet 2018-04-19 13:47:47 UTC
Created attachment 112114 [details]
[PATCH valgrind 2/3] Linux: amd64: Add support for bpf() syscall.
Comment 3 Quentin Monnet 2018-04-19 13:48:16 UTC
Created attachment 112115 [details]
[PATCH valgrind 3/3] Linux: Add file descriptors tracking in wrappers for bpf() system call.
Comment 4 Quentin Monnet 2018-07-30 09:46:15 UTC
Created attachment 114213 [details]
[PATCH valgrind v2 1/3] Linux: Get pre_check for ASCII string out of PRE(sys_prctl)

Hi,
There has been additional work on the kernel side regarding the BPF subsystem in general and the bpf() system call in particular.

I am attaching an updated version of the patches (v2), which includes BPF definitions taken from kernel 4.18 (and relevant processing of system call arguments in the PRE and POST hooks). The patches have also been rebased on valgrind's current master branch.
Comment 5 Quentin Monnet 2018-07-30 09:47:56 UTC
Created attachment 114214 [details]
[PATCH valgrind v2 2/3] Linux: amd64: Add support for bpf() syscall.
Comment 6 Quentin Monnet 2018-07-30 09:48:45 UTC
Created attachment 114215 [details]
[PATCH valgrind v2 3/3] Linux: Add file descriptors tracking in wrappers for bpf() system call.
Comment 7 Tom Hughes 2018-08-14 19:50:34 UTC
Quentin's patches have now been committed, along with an extra patch to improve the argument checking.
Comment 8 Quentin Monnet 2018-08-14 20:13:11 UTC
Awesome, thank you Tom! I'm reading your patch carefully, it is nice to see what I missed. I will follow up these patches if the bpf() syscall changes again.
Comment 9 Mark Wielaard 2018-11-12 16:24:51 UTC
*** Bug 400878 has been marked as a duplicate of this bug. ***