* [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)
From: Alexei Starovoitov @ 2014-09-17 0:39 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1410914370-29883-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
add optional attributes for BPF_PROG_LOAD syscall:
union bpf_attr {
struct {
...
__u32 log_level; /* verbosity level of eBPF verifier */
__u32 log_size; /* size of user buffer */
__aligned_u64 log_buf; /* user supplied 'char *buffer' */
};
};
when log_level > 0 the verifier will return its verification log in the user
supplied buffer 'log_buf' which can be used by program author to analyze why
verifier rejected given program.
'Understanding eBPF verifier messages' section of Documentation/networking/filter.txt
provides several examples of these messages, like the program:
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
BPF_LD_MAP_FD(BPF_REG_1, 0),
BPF_CALL_FUNC(BPF_FUNC_map_lookup_elem),
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
BPF_ST_MEM(BPF_DW, BPF_REG_0, 4, 0),
BPF_EXIT_INSN(),
will be rejected with the following multi-line message in log_buf:
0: (7a) *(u64 *)(r10 -8) = 0
1: (bf) r2 = r10
2: (07) r2 += -8
3: (b7) r1 = 0
4: (85) call 1
5: (15) if r0 == 0x0 goto pc+1
R0=map_ptr R10=fp
6: (7a) *(u64 *)(r0 +4) = 0
misaligned access off 4 size 8
The format of the output can change at any time as verifier evolves.
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
include/uapi/linux/bpf.h | 3 +
kernel/bpf/syscall.c | 2 +-
kernel/bpf/verifier.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 239 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 424f442016e7..31b0ac208a52 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -138,6 +138,9 @@ union bpf_attr {
__u32 insn_cnt;
__aligned_u64 insns;
__aligned_u64 license;
+ __u32 log_level; /* verbosity level of verifier */
+ __u32 log_size; /* size of user buffer */
+ __aligned_u64 log_buf; /* user supplied buffer */
};
} __attribute__((aligned(8)));
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 67b5e29f183e..c7be7163bd11 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -458,7 +458,7 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
}
/* last field in 'union bpf_attr' used by this command */
-#define BPF_PROG_LOAD_LAST_FIELD license
+#define BPF_PROG_LOAD_LAST_FIELD log_buf
static int bpf_prog_load(union bpf_attr *attr)
{
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d6f9c3d6b4d7..871edc1f2e1f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -125,9 +125,244 @@
* are set to NOT_INIT to indicate that they are no longer readable.
*/
+/* single container for all structs
+ * one verifier_env per bpf_check() call
+ */
+struct verifier_env {
+};
+
+/* verbose verifier prints what it's seeing
+ * bpf_check() is called under lock, so no race to access these global vars
+ */
+static u32 log_level, log_size, log_len;
+static char *log_buf;
+
+static DEFINE_MUTEX(bpf_verifier_lock);
+
+/* log_level controls verbosity level of eBPF verifier.
+ * verbose() is used to dump the verification trace to the log, so the user
+ * can figure out what's wrong with the program
+ */
+static void verbose(const char *fmt, ...)
+{
+ va_list args;
+
+ if (log_level == 0 || log_len >= log_size - 1)
+ return;
+
+ va_start(args, fmt);
+ log_len += vscnprintf(log_buf + log_len, log_size - log_len, fmt, args);
+ va_end(args);
+}
+
+static const char *const bpf_class_string[] = {
+ [BPF_LD] = "ld",
+ [BPF_LDX] = "ldx",
+ [BPF_ST] = "st",
+ [BPF_STX] = "stx",
+ [BPF_ALU] = "alu",
+ [BPF_JMP] = "jmp",
+ [BPF_RET] = "BUG",
+ [BPF_ALU64] = "alu64",
+};
+
+static const char *const bpf_alu_string[] = {
+ [BPF_ADD >> 4] = "+=",
+ [BPF_SUB >> 4] = "-=",
+ [BPF_MUL >> 4] = "*=",
+ [BPF_DIV >> 4] = "/=",
+ [BPF_OR >> 4] = "|=",
+ [BPF_AND >> 4] = "&=",
+ [BPF_LSH >> 4] = "<<=",
+ [BPF_RSH >> 4] = ">>=",
+ [BPF_NEG >> 4] = "neg",
+ [BPF_MOD >> 4] = "%=",
+ [BPF_XOR >> 4] = "^=",
+ [BPF_MOV >> 4] = "=",
+ [BPF_ARSH >> 4] = "s>>=",
+ [BPF_END >> 4] = "endian",
+};
+
+static const char *const bpf_ldst_string[] = {
+ [BPF_W >> 3] = "u32",
+ [BPF_H >> 3] = "u16",
+ [BPF_B >> 3] = "u8",
+ [BPF_DW >> 3] = "u64",
+};
+
+static const char *const bpf_jmp_string[] = {
+ [BPF_JA >> 4] = "jmp",
+ [BPF_JEQ >> 4] = "==",
+ [BPF_JGT >> 4] = ">",
+ [BPF_JGE >> 4] = ">=",
+ [BPF_JSET >> 4] = "&",
+ [BPF_JNE >> 4] = "!=",
+ [BPF_JSGT >> 4] = "s>",
+ [BPF_JSGE >> 4] = "s>=",
+ [BPF_CALL >> 4] = "call",
+ [BPF_EXIT >> 4] = "exit",
+};
+
+static void print_bpf_insn(struct bpf_insn *insn)
+{
+ u8 class = BPF_CLASS(insn->code);
+
+ if (class == BPF_ALU || class == BPF_ALU64) {
+ if (BPF_SRC(insn->code) == BPF_X)
+ verbose("(%02x) %sr%d %s %sr%d\n",
+ insn->code, class == BPF_ALU ? "(u32) " : "",
+ insn->dst_reg,
+ bpf_alu_string[BPF_OP(insn->code) >> 4],
+ class == BPF_ALU ? "(u32) " : "",
+ insn->src_reg);
+ else
+ verbose("(%02x) %sr%d %s %s%d\n",
+ insn->code, class == BPF_ALU ? "(u32) " : "",
+ insn->dst_reg,
+ bpf_alu_string[BPF_OP(insn->code) >> 4],
+ class == BPF_ALU ? "(u32) " : "",
+ insn->imm);
+ } else if (class == BPF_STX) {
+ if (BPF_MODE(insn->code) == BPF_MEM)
+ verbose("(%02x) *(%s *)(r%d %+d) = r%d\n",
+ insn->code,
+ bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
+ insn->dst_reg,
+ insn->off, insn->src_reg);
+ else if (BPF_MODE(insn->code) == BPF_XADD)
+ verbose("(%02x) lock *(%s *)(r%d %+d) += r%d\n",
+ insn->code,
+ bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
+ insn->dst_reg, insn->off,
+ insn->src_reg);
+ else
+ verbose("BUG_%02x\n", insn->code);
+ } else if (class == BPF_ST) {
+ if (BPF_MODE(insn->code) != BPF_MEM) {
+ verbose("BUG_st_%02x\n", insn->code);
+ return;
+ }
+ verbose("(%02x) *(%s *)(r%d %+d) = %d\n",
+ insn->code,
+ bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
+ insn->dst_reg,
+ insn->off, insn->imm);
+ } else if (class == BPF_LDX) {
+ if (BPF_MODE(insn->code) != BPF_MEM) {
+ verbose("BUG_ldx_%02x\n", insn->code);
+ return;
+ }
+ verbose("(%02x) r%d = *(%s *)(r%d %+d)\n",
+ insn->code, insn->dst_reg,
+ bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
+ insn->src_reg, insn->off);
+ } else if (class == BPF_LD) {
+ if (BPF_MODE(insn->code) == BPF_ABS) {
+ verbose("(%02x) r0 = *(%s *)skb[%d]\n",
+ insn->code,
+ bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
+ insn->imm);
+ } else if (BPF_MODE(insn->code) == BPF_IND) {
+ verbose("(%02x) r0 = *(%s *)skb[r%d + %d]\n",
+ insn->code,
+ bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
+ insn->src_reg, insn->imm);
+ } else if (BPF_MODE(insn->code) == BPF_IMM) {
+ verbose("(%02x) r%d = 0x%x\n",
+ insn->code, insn->dst_reg, insn->imm);
+ } else {
+ verbose("BUG_ld_%02x\n", insn->code);
+ return;
+ }
+ } else if (class == BPF_JMP) {
+ u8 opcode = BPF_OP(insn->code);
+
+ if (opcode == BPF_CALL) {
+ verbose("(%02x) call %d\n", insn->code, insn->imm);
+ } else if (insn->code == (BPF_JMP | BPF_JA)) {
+ verbose("(%02x) goto pc%+d\n",
+ insn->code, insn->off);
+ } else if (insn->code == (BPF_JMP | BPF_EXIT)) {
+ verbose("(%02x) exit\n", insn->code);
+ } else if (BPF_SRC(insn->code) == BPF_X) {
+ verbose("(%02x) if r%d %s r%d goto pc%+d\n",
+ insn->code, insn->dst_reg,
+ bpf_jmp_string[BPF_OP(insn->code) >> 4],
+ insn->src_reg, insn->off);
+ } else {
+ verbose("(%02x) if r%d %s 0x%x goto pc%+d\n",
+ insn->code, insn->dst_reg,
+ bpf_jmp_string[BPF_OP(insn->code) >> 4],
+ insn->imm, insn->off);
+ }
+ } else {
+ verbose("(%02x) %s\n", insn->code, bpf_class_string[class]);
+ }
+}
+
int bpf_check(struct bpf_prog *prog, union bpf_attr *attr)
{
+ char __user *log_ubuf = NULL;
+ struct verifier_env *env;
int ret = -EINVAL;
+ if (prog->len <= 0 || prog->len > BPF_MAXINSNS)
+ return -E2BIG;
+
+ /* 'struct verifier_env' can be global, but since it's not small,
+ * allocate/free it every time bpf_check() is called
+ */
+ env = kzalloc(sizeof(struct verifier_env), GFP_KERNEL);
+ if (!env)
+ return -ENOMEM;
+
+ /* grab the mutex to protect few globals used by verifier */
+ mutex_lock(&bpf_verifier_lock);
+
+ if (attr->log_level || attr->log_buf || attr->log_size) {
+ /* user requested verbose verifier output
+ * and supplied buffer to store the verification trace
+ */
+ log_level = attr->log_level;
+ log_ubuf = (char __user *) (unsigned long) attr->log_buf;
+ log_size = attr->log_size;
+ log_len = 0;
+
+ ret = -EINVAL;
+ /* log_* values have to be sane */
+ if (log_size < 128 || log_size > UINT_MAX >> 8 ||
+ log_level == 0 || log_ubuf == NULL)
+ goto free_env;
+
+ ret = -ENOMEM;
+ log_buf = vmalloc(log_size);
+ if (!log_buf)
+ goto free_env;
+ } else {
+ log_level = 0;
+ }
+
+ /* ret = do_check(env); */
+
+ if (log_level && log_len >= log_size - 1) {
+ BUG_ON(log_len >= log_size);
+ /* verifier log exceeded user supplied buffer */
+ ret = -ENOSPC;
+ /* fall through to return what was recorded */
+ }
+
+ /* copy verifier log back to user space including trailing zero */
+ if (log_level && copy_to_user(log_ubuf, log_buf, log_len + 1) != 0) {
+ ret = -EFAULT;
+ goto free_log_buf;
+ }
+
+
+free_log_buf:
+ if (log_level)
+ vfree(log_buf);
+free_env:
+ kfree(env);
+ mutex_unlock(&bpf_verifier_lock);
return ret;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v13 net-next 06/11] bpf: verifier (add docs)
From: Alexei Starovoitov @ 2014-09-17 0:39 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1410914370-29883-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
this patch adds all of eBPF verfier documentation and empty bpf_check()
The end goal for the verifier is to statically check safety of the program.
Verifier will catch:
- loops
- out of range jumps
- unreachable instructions
- invalid instructions
- uninitialized register access
- uninitialized stack access
- misaligned stack access
- out of range stack access
- invalid calling convention
More details in Documentation/networking/filter.txt
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
Documentation/networking/filter.txt | 224 +++++++++++++++++++++++++++++++++++
include/linux/bpf.h | 2 +
kernel/bpf/Makefile | 2 +-
kernel/bpf/syscall.c | 2 +-
kernel/bpf/verifier.c | 133 +++++++++++++++++++++
5 files changed, 361 insertions(+), 2 deletions(-)
create mode 100644 kernel/bpf/verifier.c
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index 1900d29518f1..f1c90967f748 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -1001,6 +1001,99 @@ instruction that loads 64-bit immediate value into a dst_reg.
Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
32-bit immediate value into a register.
+eBPF verifier
+-------------
+The safety of the eBPF program is determined in two steps.
+
+First step does DAG check to disallow loops and other CFG validation.
+In particular it will detect programs that have unreachable instructions.
+(though classic BPF checker allows them)
+
+Second step starts from the first insn and descends all possible paths.
+It simulates execution of every insn and observes the state change of
+registers and stack.
+
+At the start of the program the register R1 contains a pointer to context
+and has type PTR_TO_CTX.
+If verifier sees an insn that does R2=R1, then R2 has now type
+PTR_TO_CTX as well and can be used on the right hand side of expression.
+If R1=PTR_TO_CTX and insn is R2=R1+R1, then R2=UNKNOWN_VALUE,
+since addition of two valid pointers makes invalid pointer.
+(In 'secure' mode verifier will reject any type of pointer arithmetic to make
+sure that kernel addresses don't leak to unprivileged users)
+
+If register was never written to, it's not readable:
+ bpf_mov R0 = R2
+ bpf_exit
+will be rejected, since R2 is unreadable at the start of the program.
+
+After kernel function call, R1-R5 are reset to unreadable and
+R0 has a return type of the function.
+
+Since R6-R9 are callee saved, their state is preserved across the call.
+ bpf_mov R6 = 1
+ bpf_call foo
+ bpf_mov R0 = R6
+ bpf_exit
+is a correct program. If there was R1 instead of R6, it would have
+been rejected.
+
+load/store instructions are allowed only with registers of valid types, which
+are PTR_TO_CTX, PTR_TO_MAP, FRAME_PTR. They are bounds and alignment checked.
+For example:
+ bpf_mov R1 = 1
+ bpf_mov R2 = 2
+ bpf_xadd *(u32 *)(R1 + 3) += R2
+ bpf_exit
+will be rejected, since R1 doesn't have a valid pointer type at the time of
+execution of instruction bpf_xadd.
+
+At the start R1 type is PTR_TO_CTX (a pointer to generic 'struct bpf_context')
+A callback is used to customize verifier to restrict eBPF program access to only
+certain fields within ctx structure with specified size and alignment.
+
+For example, the following insn:
+ bpf_ld R0 = *(u32 *)(R6 + 8)
+intends to load a word from address R6 + 8 and store it into R0
+If R6=PTR_TO_CTX, via is_valid_access() callback the verifier will know
+that offset 8 of size 4 bytes can be accessed for reading, otherwise
+the verifier will reject the program.
+If R6=FRAME_PTR, then access should be aligned and be within
+stack bounds, which are [-MAX_BPF_STACK, 0). In this example offset is 8,
+so it will fail verification, since it's out of bounds.
+
+The verifier will allow eBPF program to read data from stack only after
+it wrote into it.
+Classic BPF verifier does similar check with M[0-15] memory slots.
+For example:
+ bpf_ld R0 = *(u32 *)(R10 - 4)
+ bpf_exit
+is invalid program.
+Though R10 is correct read-only register and has type FRAME_PTR
+and R10 - 4 is within stack bounds, there were no stores into that location.
+
+Pointer register spill/fill is tracked as well, since four (R6-R9)
+callee saved registers may not be enough for some programs.
+
+Allowed function calls are customized with bpf_verifier_ops->get_func_proto()
+The eBPF verifier will check that registers match argument constraints.
+After the call register R0 will be set to return type of the function.
+
+Function calls is a main mechanism to extend functionality of eBPF programs.
+Socket filters may let programs to call one set of functions, whereas tracing
+filters may allow completely different set.
+
+If a function made accessible to eBPF program, it needs to be thought through
+from safety point of view. The verifier will guarantee that the function is
+called with valid arguments.
+
+seccomp vs socket filters have different security restrictions for classic BPF.
+Seccomp solves this by two stage verifier: classic BPF verifier is followed
+by seccomp verifier. In case of eBPF one configurable verifier is shared for
+all use cases.
+
+See details of eBPF verifier in kernel/bpf/verifier.c
+
eBPF maps
---------
'maps' is a generic storage of different types for sharing data between kernel
@@ -1040,6 +1133,137 @@ The map is defined by:
. key size in bytes
. value size in bytes
+Understanding eBPF verifier messages
+------------------------------------
+
+The following are few examples of invalid eBPF programs and verifier error
+messages as seen in the log:
+
+Program with unreachable instructions:
+static struct bpf_insn prog[] = {
+ BPF_EXIT_INSN(),
+ BPF_EXIT_INSN(),
+};
+Error:
+ unreachable insn 1
+
+Program that reads uninitialized register:
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+ BPF_EXIT_INSN(),
+Error:
+ 0: (bf) r0 = r2
+ R2 !read_ok
+
+Program that doesn't initialize R0 before exiting:
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_1),
+ BPF_EXIT_INSN(),
+Error:
+ 0: (bf) r2 = r1
+ 1: (95) exit
+ R0 !read_ok
+
+Program that accesses stack out of bounds:
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, 8, 0),
+ BPF_EXIT_INSN(),
+Error:
+ 0: (7a) *(u64 *)(r10 +8) = 0
+ invalid stack off=8 size=8
+
+Program that doesn't initialize stack before passing its address into function:
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_EXIT_INSN(),
+Error:
+ 0: (bf) r2 = r10
+ 1: (07) r2 += -8
+ 2: (b7) r1 = 0x0
+ 3: (85) call 1
+ invalid indirect read from stack off -8+0 size 8
+
+Program that uses invalid map_fd=0 while calling to map_lookup_elem() function:
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_EXIT_INSN(),
+Error:
+ 0: (7a) *(u64 *)(r10 -8) = 0
+ 1: (bf) r2 = r10
+ 2: (07) r2 += -8
+ 3: (b7) r1 = 0x0
+ 4: (85) call 1
+ fd 0 is not pointing to valid bpf_map
+
+Program that doesn't check return value of map_lookup_elem() before accessing
+map element:
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, 0),
+ BPF_EXIT_INSN(),
+Error:
+ 0: (7a) *(u64 *)(r10 -8) = 0
+ 1: (bf) r2 = r10
+ 2: (07) r2 += -8
+ 3: (b7) r1 = 0x0
+ 4: (85) call 1
+ 5: (7a) *(u64 *)(r0 +0) = 0
+ R0 invalid mem access 'map_value_or_null'
+
+Program that correctly checks map_lookup_elem() returned value for NULL, but
+accesses the memory with incorrect alignment:
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 4, 0),
+ BPF_EXIT_INSN(),
+Error:
+ 0: (7a) *(u64 *)(r10 -8) = 0
+ 1: (bf) r2 = r10
+ 2: (07) r2 += -8
+ 3: (b7) r1 = 1
+ 4: (85) call 1
+ 5: (15) if r0 == 0x0 goto pc+1
+ R0=map_ptr R10=fp
+ 6: (7a) *(u64 *)(r0 +4) = 0
+ misaligned access off 4 size 8
+
+Program that correctly checks map_lookup_elem() returned value for NULL and
+accesses memory with correct alignment in one side of 'if' branch, but fails
+to do so in the other side of 'if' branch:
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, 0),
+ BPF_EXIT_INSN(),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, 1),
+ BPF_EXIT_INSN(),
+Error:
+ 0: (7a) *(u64 *)(r10 -8) = 0
+ 1: (bf) r2 = r10
+ 2: (07) r2 += -8
+ 3: (b7) r1 = 1
+ 4: (85) call 1
+ 5: (15) if r0 == 0x0 goto pc+2
+ R0=map_ptr R10=fp
+ 6: (7a) *(u64 *)(r0 +0) = 0
+ 7: (95) exit
+
+ from 5 to 8: R0=imm0 R10=fp
+ 8: (7a) *(u64 *)(r0 +0) = 1
+ R0 invalid mem access 'imm'
+
Testing
-------
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 92979182be81..9dfeb36f8971 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -83,5 +83,7 @@ struct bpf_prog_aux {
void bpf_prog_put(struct bpf_prog *prog);
struct bpf_prog *bpf_prog_get(u32 ufd);
+/* verify correctness of eBPF program */
+int bpf_check(struct bpf_prog *fp, union bpf_attr *attr);
#endif /* _LINUX_BPF_H */
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index e9f7334ed07a..3c726b0995b7 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -1 +1 @@
-obj-y := core.o syscall.o
+obj-y := core.o syscall.o verifier.o
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 5a45cdc511e1..67b5e29f183e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -507,7 +507,7 @@ static int bpf_prog_load(union bpf_attr *attr)
goto free_prog;
/* run eBPF verifier */
- /* err = bpf_check(prog, tb); */
+ err = bpf_check(prog, attr);
if (err < 0)
goto free_used_maps;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
new file mode 100644
index 000000000000..d6f9c3d6b4d7
--- /dev/null
+++ b/kernel/bpf/verifier.c
@@ -0,0 +1,133 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/bpf.h>
+#include <linux/filter.h>
+#include <net/netlink.h>
+#include <linux/file.h>
+#include <linux/vmalloc.h>
+
+/* bpf_check() is a static code analyzer that walks eBPF program
+ * instruction by instruction and updates register/stack state.
+ * All paths of conditional branches are analyzed until 'bpf_exit' insn.
+ *
+ * The first pass is depth-first-search to check that the program is a DAG.
+ * It rejects the following programs:
+ * - larger than BPF_MAXINSNS insns
+ * - if loop is present (detected via back-edge)
+ * - unreachable insns exist (shouldn't be a forest. program = one function)
+ * - out of bounds or malformed jumps
+ * The second pass is all possible path descent from the 1st insn.
+ * Since it's analyzing all pathes through the program, the length of the
+ * analysis is limited to 32k insn, which may be hit even if total number of
+ * insn is less then 4K, but there are too many branches that change stack/regs.
+ * Number of 'branches to be analyzed' is limited to 1k
+ *
+ * On entry to each instruction, each register has a type, and the instruction
+ * changes the types of the registers depending on instruction semantics.
+ * If instruction is BPF_MOV64_REG(BPF_REG_1, BPF_REG_5), then type of R5 is
+ * copied to R1.
+ *
+ * All registers are 64-bit.
+ * R0 - return register
+ * R1-R5 argument passing registers
+ * R6-R9 callee saved registers
+ * R10 - frame pointer read-only
+ *
+ * At the start of BPF program the register R1 contains a pointer to bpf_context
+ * and has type PTR_TO_CTX.
+ *
+ * Verifier tracks arithmetic operations on pointers in case:
+ * BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ * BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -20),
+ * 1st insn copies R10 (which has FRAME_PTR) type into R1
+ * and 2nd arithmetic instruction is pattern matched to recognize
+ * that it wants to construct a pointer to some element within stack.
+ * So after 2nd insn, the register R1 has type PTR_TO_STACK
+ * (and -20 constant is saved for further stack bounds checking).
+ * Meaning that this reg is a pointer to stack plus known immediate constant.
+ *
+ * Most of the time the registers have UNKNOWN_VALUE type, which
+ * means the register has some value, but it's not a valid pointer.
+ * (like pointer plus pointer becomes UNKNOWN_VALUE type)
+ *
+ * When verifier sees load or store instructions the type of base register
+ * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, FRAME_PTR. These are three pointer
+ * types recognized by check_mem_access() function.
+ *
+ * PTR_TO_MAP_VALUE means that this register is pointing to 'map element value'
+ * and the range of [ptr, ptr + map's value_size) is accessible.
+ *
+ * registers used to pass values to function calls are checked against
+ * function argument constraints.
+ *
+ * ARG_PTR_TO_MAP_KEY is one of such argument constraints.
+ * It means that the register type passed to this function must be
+ * PTR_TO_STACK and it will be used inside the function as
+ * 'pointer to map element key'
+ *
+ * For example the argument constraints for bpf_map_lookup_elem():
+ * .ret_type = RET_PTR_TO_MAP_VALUE_OR_NULL,
+ * .arg1_type = ARG_CONST_MAP_PTR,
+ * .arg2_type = ARG_PTR_TO_MAP_KEY,
+ *
+ * ret_type says that this function returns 'pointer to map elem value or null'
+ * function expects 1st argument to be a const pointer to 'struct bpf_map' and
+ * 2nd argument should be a pointer to stack, which will be used inside
+ * the helper function as a pointer to map element key.
+ *
+ * On the kernel side the helper function looks like:
+ * u64 bpf_map_lookup_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+ * {
+ * struct bpf_map *map = (struct bpf_map *) (unsigned long) r1;
+ * void *key = (void *) (unsigned long) r2;
+ * void *value;
+ *
+ * here kernel can access 'key' and 'map' pointers safely, knowing that
+ * [key, key + map->key_size) bytes are valid and were initialized on
+ * the stack of eBPF program.
+ * }
+ *
+ * Corresponding eBPF program may look like:
+ * BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), // after this insn R2 type is FRAME_PTR
+ * BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), // after this insn R2 type is PTR_TO_STACK
+ * BPF_LD_MAP_FD(BPF_REG_1, map_fd), // after this insn R1 type is CONST_PTR_TO_MAP
+ * BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ * here verifier looks at prototype of map_lookup_elem() and sees:
+ * .arg1_type == ARG_CONST_MAP_PTR and R1->type == CONST_PTR_TO_MAP, which is ok,
+ * Now verifier knows that this map has key of R1->map_ptr->key_size bytes
+ *
+ * Then .arg2_type == ARG_PTR_TO_MAP_KEY and R2->type == PTR_TO_STACK, ok so far,
+ * Now verifier checks that [R2, R2 + map's key_size) are within stack limits
+ * and were initialized prior to this call.
+ * If it's ok, then verifier allows this BPF_CALL insn and looks at
+ * .ret_type which is RET_PTR_TO_MAP_VALUE_OR_NULL, so it sets
+ * R0->type = PTR_TO_MAP_VALUE_OR_NULL which means bpf_map_lookup_elem() function
+ * returns ether pointer to map value or NULL.
+ *
+ * When type PTR_TO_MAP_VALUE_OR_NULL passes through 'if (reg != 0) goto +off'
+ * insn, the register holding that pointer in the true branch changes state to
+ * PTR_TO_MAP_VALUE and the same register changes state to CONST_IMM in the false
+ * branch. See check_cond_jmp_op().
+ *
+ * After the call R0 is set to return type of the function and registers R1-R5
+ * are set to NOT_INIT to indicate that they are no longer readable.
+ */
+
+int bpf_check(struct bpf_prog *prog, union bpf_attr *attr)
+{
+ int ret = -EINVAL;
+
+ return ret;
+}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v13 net-next 05/11] bpf: handle pseudo BPF_CALL insn
From: Alexei Starovoitov @ 2014-09-17 0:39 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1410914370-29883-1-git-send-email-ast@plumgrid.com>
in native eBPF programs userspace is using pseudo BPF_CALL instructions
which encode one of 'enum bpf_func_id' inside insn->imm field.
Verifier checks that program using correct function arguments to given func_id.
If all checks passed, kernel needs to fixup BPF_CALL->imm fields by
replacing func_id with in-kernel function pointer.
eBPF interpreter just calls the function.
In-kernel eBPF users continue to use generic BPF_CALL.
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
kernel/bpf/syscall.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index a5b5c7ca0264..5a45cdc511e1 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -357,6 +357,40 @@ void bpf_register_prog_type(struct bpf_prog_type_list *tl)
list_add(&tl->list_node, &bpf_prog_types);
}
+/* fixup insn->imm field of bpf_call instructions:
+ * if (insn->imm == BPF_FUNC_map_lookup_elem)
+ * insn->imm = bpf_map_lookup_elem - __bpf_call_base;
+ * else if (insn->imm == BPF_FUNC_map_update_elem)
+ * insn->imm = bpf_map_update_elem - __bpf_call_base;
+ * else ...
+ *
+ * this function is called after eBPF program passed verification
+ */
+static void fixup_bpf_calls(struct bpf_prog *prog)
+{
+ const struct bpf_func_proto *fn;
+ int i;
+
+ for (i = 0; i < prog->len; i++) {
+ struct bpf_insn *insn = &prog->insnsi[i];
+
+ if (insn->code == (BPF_JMP | BPF_CALL)) {
+ /* we reach here when program has bpf_call instructions
+ * and it passed bpf_check(), means that
+ * ops->get_func_proto must have been supplied, check it
+ */
+ BUG_ON(!prog->aux->ops->get_func_proto);
+
+ fn = prog->aux->ops->get_func_proto(insn->imm);
+ /* all functions that have prototype and verifier allowed
+ * programs to call them, must be real in-kernel functions
+ */
+ BUG_ON(!fn->func);
+ insn->imm = fn->func - __bpf_call_base;
+ }
+ }
+}
+
/* drop refcnt on maps used by eBPF program and free auxilary data */
static void free_used_maps(struct bpf_prog_aux *aux)
{
@@ -478,6 +512,9 @@ static int bpf_prog_load(union bpf_attr *attr)
if (err < 0)
goto free_used_maps;
+ /* fixup BPF_CALL->imm field */
+ fixup_bpf_calls(prog);
+
/* eBPF program is ready to be JITed */
bpf_prog_select_runtime(prog);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v13 net-next 04/11] bpf: expand BPF syscall with program load/unload
From: Alexei Starovoitov @ 2014-09-17 0:39 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1410914370-29883-1-git-send-email-ast@plumgrid.com>
eBPF programs are similar to kernel modules. They are loaded by the user
process and automatically unloaded when process exits. Each eBPF program is
a safe run-to-completion set of instructions. eBPF verifier statically
determines that the program terminates and is safe to execute.
The following syscall wrapper can be used to load the program:
int bpf_prog_load(enum bpf_prog_type prog_type,
const struct bpf_insn *insns, int insn_cnt,
const char *license)
{
union bpf_attr attr = {
.prog_type = prog_type,
.insns = ptr_to_u64(insns),
.insn_cnt = insn_cnt,
.license = ptr_to_u64(license),
};
return bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
}
where 'insns' is an array of eBPF instructions and 'license' is a string
that must be GPL compatible to call helper functions marked gpl_only
Upon succesful load the syscall returns prog_fd.
Use close(prog_fd) to unload the program.
User space tests and examples follow in the later patches
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
include/linux/bpf.h | 38 +++++++++++
include/linux/filter.h | 8 +--
include/uapi/linux/bpf.h | 26 ++++++++
kernel/bpf/core.c | 29 ++++----
kernel/bpf/syscall.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 246 insertions(+), 20 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2887f3f9da59..92979182be81 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -46,4 +46,42 @@ void bpf_register_map_type(struct bpf_map_type_list *tl);
void bpf_map_put(struct bpf_map *map);
struct bpf_map *bpf_map_get(struct fd f);
+/* eBPF function prototype used by verifier to allow BPF_CALLs from eBPF programs
+ * to in-kernel helper functions and for adjusting imm32 field in BPF_CALL
+ * instructions after verifying
+ */
+struct bpf_func_proto {
+ u64 (*func)(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
+ bool gpl_only;
+};
+
+struct bpf_verifier_ops {
+ /* return eBPF function prototype for verification */
+ const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id);
+};
+
+struct bpf_prog_type_list {
+ struct list_head list_node;
+ struct bpf_verifier_ops *ops;
+ enum bpf_prog_type type;
+};
+
+void bpf_register_prog_type(struct bpf_prog_type_list *tl);
+
+struct bpf_prog;
+
+struct bpf_prog_aux {
+ atomic_t refcnt;
+ bool is_gpl_compatible;
+ enum bpf_prog_type prog_type;
+ struct bpf_verifier_ops *ops;
+ struct bpf_map **used_maps;
+ u32 used_map_cnt;
+ struct bpf_prog *prog;
+ struct work_struct work;
+};
+
+void bpf_prog_put(struct bpf_prog *prog);
+struct bpf_prog *bpf_prog_get(u32 ufd);
+
#endif /* _LINUX_BPF_H */
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 1a0bc6d134d7..4ffc0958d85e 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -21,6 +21,7 @@
struct sk_buff;
struct sock;
struct seccomp_data;
+struct bpf_prog_aux;
/* ArgX, context and stack frame pointer register positions. Note,
* Arg1, Arg2, Arg3, etc are used as argument mappings of function
@@ -300,17 +301,12 @@ struct bpf_binary_header {
u8 image[];
};
-struct bpf_work_struct {
- struct bpf_prog *prog;
- struct work_struct work;
-};
-
struct bpf_prog {
u16 pages; /* Number of allocated pages */
bool jited; /* Is our filter JIT'ed? */
u32 len; /* Number of filter blocks */
struct sock_fprog_kern *orig_prog; /* Original BPF program */
- struct bpf_work_struct *work; /* Deferred free work struct */
+ struct bpf_prog_aux *aux; /* Auxiliary fields */
unsigned int (*bpf_func)(const struct sk_buff *skb,
const struct bpf_insn *filter);
/* Instructions for interpreter */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 395cabd2ca0a..424f442016e7 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -99,12 +99,23 @@ enum bpf_cmd {
* returns zero and stores next key or negative error
*/
BPF_MAP_GET_NEXT_KEY,
+
+ /* verify and load eBPF program
+ * prog_fd = bpf(BPF_PROG_LOAD, union bpf_attr *attr, u32 size)
+ * Using attr->prog_type, attr->insns, attr->license
+ * returns fd or negative error
+ */
+ BPF_PROG_LOAD,
};
enum bpf_map_type {
BPF_MAP_TYPE_UNSPEC,
};
+enum bpf_prog_type {
+ BPF_PROG_TYPE_UNSPEC,
+};
+
union bpf_attr {
struct { /* anonymous struct used by BPF_MAP_CREATE command */
__u32 map_type; /* one of enum bpf_map_type */
@@ -121,6 +132,21 @@ union bpf_attr {
__aligned_u64 next_key;
};
};
+
+ struct { /* anonymous struct used by BPF_PROG_LOAD command */
+ __u32 prog_type; /* one of enum bpf_prog_type */
+ __u32 insn_cnt;
+ __aligned_u64 insns;
+ __aligned_u64 license;
+ };
} __attribute__((aligned(8)));
+/* integer value in 'imm' field of BPF_CALL instruction selects which helper
+ * function eBPF program intends to call
+ */
+enum bpf_func_id {
+ BPF_FUNC_unspec,
+ __BPF_FUNC_MAX_ID,
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 8b7002488251..f0c30c59b317 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -27,6 +27,7 @@
#include <linux/random.h>
#include <linux/moduleloader.h>
#include <asm/unaligned.h>
+#include <linux/bpf.h>
/* Registers */
#define BPF_R0 regs[BPF_REG_0]
@@ -71,7 +72,7 @@ struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
{
gfp_t gfp_flags = GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO |
gfp_extra_flags;
- struct bpf_work_struct *ws;
+ struct bpf_prog_aux *aux;
struct bpf_prog *fp;
size = round_up(size, PAGE_SIZE);
@@ -79,14 +80,14 @@ struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
if (fp == NULL)
return NULL;
- ws = kmalloc(sizeof(*ws), GFP_KERNEL | gfp_extra_flags);
- if (ws == NULL) {
+ aux = kzalloc(sizeof(*aux), GFP_KERNEL | gfp_extra_flags);
+ if (aux == NULL) {
vfree(fp);
return NULL;
}
fp->pages = size / PAGE_SIZE;
- fp->work = ws;
+ fp->aux = aux;
return fp;
}
@@ -110,10 +111,10 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
memcpy(fp, fp_old, fp_old->pages * PAGE_SIZE);
fp->pages = size / PAGE_SIZE;
- /* We keep fp->work from fp_old around in the new
+ /* We keep fp->aux from fp_old around in the new
* reallocated structure.
*/
- fp_old->work = NULL;
+ fp_old->aux = NULL;
__bpf_prog_free(fp_old);
}
@@ -123,7 +124,7 @@ EXPORT_SYMBOL_GPL(bpf_prog_realloc);
void __bpf_prog_free(struct bpf_prog *fp)
{
- kfree(fp->work);
+ kfree(fp->aux);
vfree(fp);
}
EXPORT_SYMBOL_GPL(__bpf_prog_free);
@@ -638,19 +639,19 @@ EXPORT_SYMBOL_GPL(bpf_prog_select_runtime);
static void bpf_prog_free_deferred(struct work_struct *work)
{
- struct bpf_work_struct *ws;
+ struct bpf_prog_aux *aux;
- ws = container_of(work, struct bpf_work_struct, work);
- bpf_jit_free(ws->prog);
+ aux = container_of(work, struct bpf_prog_aux, work);
+ bpf_jit_free(aux->prog);
}
/* Free internal BPF program */
void bpf_prog_free(struct bpf_prog *fp)
{
- struct bpf_work_struct *ws = fp->work;
+ struct bpf_prog_aux *aux = fp->aux;
- INIT_WORK(&ws->work, bpf_prog_free_deferred);
- ws->prog = fp;
- schedule_work(&ws->work);
+ INIT_WORK(&aux->work, bpf_prog_free_deferred);
+ aux->prog = fp;
+ schedule_work(&aux->work);
}
EXPORT_SYMBOL_GPL(bpf_prog_free);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index d2d6491c21b5..a5b5c7ca0264 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -14,6 +14,8 @@
#include <linux/slab.h>
#include <linux/anon_inodes.h>
#include <linux/file.h>
+#include <linux/license.h>
+#include <linux/filter.h>
static LIST_HEAD(bpf_map_types);
@@ -334,6 +336,166 @@ err_put:
return err;
}
+static LIST_HEAD(bpf_prog_types);
+
+static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog)
+{
+ struct bpf_prog_type_list *tl;
+
+ list_for_each_entry(tl, &bpf_prog_types, list_node) {
+ if (tl->type == type) {
+ prog->aux->ops = tl->ops;
+ prog->aux->prog_type = type;
+ return 0;
+ }
+ }
+ return -EINVAL;
+}
+
+void bpf_register_prog_type(struct bpf_prog_type_list *tl)
+{
+ list_add(&tl->list_node, &bpf_prog_types);
+}
+
+/* drop refcnt on maps used by eBPF program and free auxilary data */
+static void free_used_maps(struct bpf_prog_aux *aux)
+{
+ int i;
+
+ for (i = 0; i < aux->used_map_cnt; i++)
+ bpf_map_put(aux->used_maps[i]);
+
+ kfree(aux->used_maps);
+}
+
+void bpf_prog_put(struct bpf_prog *prog)
+{
+ if (atomic_dec_and_test(&prog->aux->refcnt)) {
+ free_used_maps(prog->aux);
+ bpf_prog_free(prog);
+ }
+}
+
+static int bpf_prog_release(struct inode *inode, struct file *filp)
+{
+ struct bpf_prog *prog = filp->private_data;
+
+ bpf_prog_put(prog);
+ return 0;
+}
+
+static const struct file_operations bpf_prog_fops = {
+ .release = bpf_prog_release,
+};
+
+static struct bpf_prog *get_prog(struct fd f)
+{
+ struct bpf_prog *prog;
+
+ if (!f.file)
+ return ERR_PTR(-EBADF);
+
+ if (f.file->f_op != &bpf_prog_fops) {
+ fdput(f);
+ return ERR_PTR(-EINVAL);
+ }
+
+ prog = f.file->private_data;
+
+ return prog;
+}
+
+/* called by sockets/tracing/seccomp before attaching program to an event
+ * pairs with bpf_prog_put()
+ */
+struct bpf_prog *bpf_prog_get(u32 ufd)
+{
+ struct fd f = fdget(ufd);
+ struct bpf_prog *prog;
+
+ prog = get_prog(f);
+
+ if (IS_ERR(prog))
+ return prog;
+
+ atomic_inc(&prog->aux->refcnt);
+ fdput(f);
+ return prog;
+}
+
+/* last field in 'union bpf_attr' used by this command */
+#define BPF_PROG_LOAD_LAST_FIELD license
+
+static int bpf_prog_load(union bpf_attr *attr)
+{
+ enum bpf_prog_type type = attr->prog_type;
+ struct bpf_prog *prog;
+ int err;
+ char license[128];
+ bool is_gpl;
+
+ if (CHECK_ATTR(BPF_PROG_LOAD))
+ return -EINVAL;
+
+ /* copy eBPF program license from user space */
+ if (strncpy_from_user(license, u64_to_ptr(attr->license),
+ sizeof(license) - 1) < 0)
+ return -EFAULT;
+ license[sizeof(license) - 1] = 0;
+
+ /* eBPF programs must be GPL compatible to use GPL-ed functions */
+ is_gpl = license_is_gpl_compatible(license);
+
+ if (attr->insn_cnt >= BPF_MAXINSNS)
+ return -EINVAL;
+
+ /* plain bpf_prog allocation */
+ prog = bpf_prog_alloc(bpf_prog_size(attr->insn_cnt), GFP_USER);
+ if (!prog)
+ return -ENOMEM;
+
+ prog->len = attr->insn_cnt;
+
+ err = -EFAULT;
+ if (copy_from_user(prog->insns, u64_to_ptr(attr->insns),
+ prog->len * sizeof(struct bpf_insn)) != 0)
+ goto free_prog;
+
+ prog->orig_prog = NULL;
+ prog->jited = false;
+
+ atomic_set(&prog->aux->refcnt, 1);
+ prog->aux->is_gpl_compatible = is_gpl;
+
+ /* find program type: socket_filter vs tracing_filter */
+ err = find_prog_type(type, prog);
+ if (err < 0)
+ goto free_prog;
+
+ /* run eBPF verifier */
+ /* err = bpf_check(prog, tb); */
+
+ if (err < 0)
+ goto free_used_maps;
+
+ /* eBPF program is ready to be JITed */
+ bpf_prog_select_runtime(prog);
+
+ err = anon_inode_getfd("bpf-prog", &bpf_prog_fops, prog, O_RDWR | O_CLOEXEC);
+
+ if (err < 0)
+ /* failed to allocate fd */
+ goto free_used_maps;
+
+ return err;
+
+free_used_maps:
+ free_used_maps(prog->aux);
+free_prog:
+ bpf_prog_free(prog);
+ return err;
+}
+
SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
{
union bpf_attr attr = {};
@@ -370,6 +532,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
case BPF_MAP_GET_NEXT_KEY:
err = map_get_next_key(&attr);
break;
+ case BPF_PROG_LOAD:
+ err = bpf_prog_load(&attr);
+ break;
default:
err = -EINVAL;
break;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v13 net-next 03/11] bpf: add lookup/update/delete/iterate methods to BPF maps
From: Alexei Starovoitov @ 2014-09-17 0:39 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1410914370-29883-1-git-send-email-ast@plumgrid.com>
'maps' is a generic storage of different types for sharing data between kernel
and userspace.
The maps are accessed from user space via BPF syscall, which has commands:
- create a map with given type and attributes
fd = bpf(BPF_MAP_CREATE, union bpf_attr *attr, u32 size)
returns fd or negative error
- lookup key in a given map referenced by fd
err = bpf(BPF_MAP_LOOKUP_ELEM, union bpf_attr *attr, u32 size)
using attr->map_fd, attr->key, attr->value
returns zero and stores found elem into value or negative error
- create or update key/value pair in a given map
err = bpf(BPF_MAP_UPDATE_ELEM, union bpf_attr *attr, u32 size)
using attr->map_fd, attr->key, attr->value
returns zero or negative error
- find and delete element by key in a given map
err = bpf(BPF_MAP_DELETE_ELEM, union bpf_attr *attr, u32 size)
using attr->map_fd, attr->key
- iterate map elements (based on input key return next_key)
err = bpf(BPF_MAP_GET_NEXT_KEY, union bpf_attr *attr, u32 size)
using attr->map_fd, attr->key, attr->next_key
- close(fd) deletes the map
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
include/linux/bpf.h | 8 ++
include/uapi/linux/bpf.h | 38 ++++++++
kernel/bpf/syscall.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 281 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 48014a71f0fe..2887f3f9da59 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -9,6 +9,7 @@
#include <uapi/linux/bpf.h>
#include <linux/workqueue.h>
+#include <linux/file.h>
struct bpf_map;
@@ -17,6 +18,12 @@ struct bpf_map_ops {
/* funcs callable from userspace (via syscall) */
struct bpf_map *(*map_alloc)(union bpf_attr *attr);
void (*map_free)(struct bpf_map *);
+ int (*map_get_next_key)(struct bpf_map *map, void *key, void *next_key);
+
+ /* funcs callable from userspace and from eBPF programs */
+ void *(*map_lookup_elem)(struct bpf_map *map, void *key);
+ int (*map_update_elem)(struct bpf_map *map, void *key, void *value);
+ int (*map_delete_elem)(struct bpf_map *map, void *key);
};
struct bpf_map {
@@ -37,5 +44,6 @@ struct bpf_map_type_list {
void bpf_register_map_type(struct bpf_map_type_list *tl);
void bpf_map_put(struct bpf_map *map);
+struct bpf_map *bpf_map_get(struct fd f);
#endif /* _LINUX_BPF_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index f58a10f9670c..395cabd2ca0a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -70,6 +70,35 @@ enum bpf_cmd {
* map is deleted when fd is closed
*/
BPF_MAP_CREATE,
+
+ /* lookup key in a given map
+ * err = bpf(BPF_MAP_LOOKUP_ELEM, union bpf_attr *attr, u32 size)
+ * Using attr->map_fd, attr->key, attr->value
+ * returns zero and stores found elem into value
+ * or negative error
+ */
+ BPF_MAP_LOOKUP_ELEM,
+
+ /* create or update key/value pair in a given map
+ * err = bpf(BPF_MAP_UPDATE_ELEM, union bpf_attr *attr, u32 size)
+ * Using attr->map_fd, attr->key, attr->value
+ * returns zero or negative error
+ */
+ BPF_MAP_UPDATE_ELEM,
+
+ /* find and delete elem by key in a given map
+ * err = bpf(BPF_MAP_DELETE_ELEM, union bpf_attr *attr, u32 size)
+ * Using attr->map_fd, attr->key
+ * returns zero or negative error
+ */
+ BPF_MAP_DELETE_ELEM,
+
+ /* lookup key in a given map and return next key
+ * err = bpf(BPF_MAP_GET_NEXT_KEY, union bpf_attr *attr, u32 size)
+ * Using attr->map_fd, attr->key, attr->next_key
+ * returns zero and stores next key or negative error
+ */
+ BPF_MAP_GET_NEXT_KEY,
};
enum bpf_map_type {
@@ -83,6 +112,15 @@ union bpf_attr {
__u32 value_size; /* size of value in bytes */
__u32 max_entries; /* max number of entries in a map */
};
+
+ struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
+ __u32 map_fd;
+ __aligned_u64 key;
+ union {
+ __aligned_u64 value;
+ __aligned_u64 next_key;
+ };
+ };
} __attribute__((aligned(8)));
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 328a45a6d038..d2d6491c21b5 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -13,6 +13,7 @@
#include <linux/syscalls.h>
#include <linux/slab.h>
#include <linux/anon_inodes.h>
+#include <linux/file.h>
static LIST_HEAD(bpf_map_types);
@@ -111,6 +112,228 @@ free_map:
return err;
}
+/* if error is returned, fd is released.
+ * On success caller should complete fd access with matching fdput()
+ */
+struct bpf_map *bpf_map_get(struct fd f)
+{
+ struct bpf_map *map;
+
+ if (!f.file)
+ return ERR_PTR(-EBADF);
+
+ if (f.file->f_op != &bpf_map_fops) {
+ fdput(f);
+ return ERR_PTR(-EINVAL);
+ }
+
+ map = f.file->private_data;
+
+ return map;
+}
+
+/* helper to convert user pointers passed inside __aligned_u64 fields */
+static void __user *u64_to_ptr(__u64 val)
+{
+ return (void __user *) (unsigned long) val;
+}
+
+/* last field in 'union bpf_attr' used by this command */
+#define BPF_MAP_LOOKUP_ELEM_LAST_FIELD value
+
+static int map_lookup_elem(union bpf_attr *attr)
+{
+ void __user *ukey = u64_to_ptr(attr->key);
+ void __user *uvalue = u64_to_ptr(attr->value);
+ int ufd = attr->map_fd;
+ struct fd f = fdget(ufd);
+ struct bpf_map *map;
+ void *key, *value;
+ int err;
+
+ if (CHECK_ATTR(BPF_MAP_LOOKUP_ELEM))
+ return -EINVAL;
+
+ map = bpf_map_get(f);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ err = -ENOMEM;
+ key = kmalloc(map->key_size, GFP_USER);
+ if (!key)
+ goto err_put;
+
+ err = -EFAULT;
+ if (copy_from_user(key, ukey, map->key_size) != 0)
+ goto free_key;
+
+ err = -ESRCH;
+ rcu_read_lock();
+ value = map->ops->map_lookup_elem(map, key);
+ if (!value)
+ goto err_unlock;
+
+ err = -EFAULT;
+ if (copy_to_user(uvalue, value, map->value_size) != 0)
+ goto err_unlock;
+
+ err = 0;
+
+err_unlock:
+ rcu_read_unlock();
+free_key:
+ kfree(key);
+err_put:
+ fdput(f);
+ return err;
+}
+
+#define BPF_MAP_UPDATE_ELEM_LAST_FIELD value
+
+static int map_update_elem(union bpf_attr *attr)
+{
+ void __user *ukey = u64_to_ptr(attr->key);
+ void __user *uvalue = u64_to_ptr(attr->value);
+ int ufd = attr->map_fd;
+ struct fd f = fdget(ufd);
+ struct bpf_map *map;
+ void *key, *value;
+ int err;
+
+ if (CHECK_ATTR(BPF_MAP_UPDATE_ELEM))
+ return -EINVAL;
+
+ map = bpf_map_get(f);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ err = -ENOMEM;
+ key = kmalloc(map->key_size, GFP_USER);
+ if (!key)
+ goto err_put;
+
+ err = -EFAULT;
+ if (copy_from_user(key, ukey, map->key_size) != 0)
+ goto free_key;
+
+ err = -ENOMEM;
+ value = kmalloc(map->value_size, GFP_USER);
+ if (!value)
+ goto free_key;
+
+ err = -EFAULT;
+ if (copy_from_user(value, uvalue, map->value_size) != 0)
+ goto free_value;
+
+ /* eBPF program that use maps are running under rcu_read_lock(),
+ * therefore all map accessors rely on this fact, so do the same here
+ */
+ rcu_read_lock();
+ err = map->ops->map_update_elem(map, key, value);
+ rcu_read_unlock();
+
+free_value:
+ kfree(value);
+free_key:
+ kfree(key);
+err_put:
+ fdput(f);
+ return err;
+}
+
+#define BPF_MAP_DELETE_ELEM_LAST_FIELD key
+
+static int map_delete_elem(union bpf_attr *attr)
+{
+ void __user *ukey = u64_to_ptr(attr->key);
+ int ufd = attr->map_fd;
+ struct fd f = fdget(ufd);
+ struct bpf_map *map;
+ void *key;
+ int err;
+
+ if (CHECK_ATTR(BPF_MAP_DELETE_ELEM))
+ return -EINVAL;
+
+ map = bpf_map_get(f);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ err = -ENOMEM;
+ key = kmalloc(map->key_size, GFP_USER);
+ if (!key)
+ goto err_put;
+
+ err = -EFAULT;
+ if (copy_from_user(key, ukey, map->key_size) != 0)
+ goto free_key;
+
+ rcu_read_lock();
+ err = map->ops->map_delete_elem(map, key);
+ rcu_read_unlock();
+
+free_key:
+ kfree(key);
+err_put:
+ fdput(f);
+ return err;
+}
+
+/* last field in 'union bpf_attr' used by this command */
+#define BPF_MAP_GET_NEXT_KEY_LAST_FIELD next_key
+
+static int map_get_next_key(union bpf_attr *attr)
+{
+ void __user *ukey = u64_to_ptr(attr->key);
+ void __user *unext_key = u64_to_ptr(attr->next_key);
+ int ufd = attr->map_fd;
+ struct fd f = fdget(ufd);
+ struct bpf_map *map;
+ void *key, *next_key;
+ int err;
+
+ if (CHECK_ATTR(BPF_MAP_GET_NEXT_KEY))
+ return -EINVAL;
+
+ map = bpf_map_get(f);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ err = -ENOMEM;
+ key = kmalloc(map->key_size, GFP_USER);
+ if (!key)
+ goto err_put;
+
+ err = -EFAULT;
+ if (copy_from_user(key, ukey, map->key_size) != 0)
+ goto free_key;
+
+ err = -ENOMEM;
+ next_key = kmalloc(map->key_size, GFP_USER);
+ if (!next_key)
+ goto free_key;
+
+ rcu_read_lock();
+ err = map->ops->map_get_next_key(map, key, next_key);
+ rcu_read_unlock();
+ if (err)
+ goto free_next_key;
+
+ err = -EFAULT;
+ if (copy_to_user(unext_key, next_key, map->key_size) != 0)
+ goto free_next_key;
+
+ err = 0;
+
+free_next_key:
+ kfree(next_key);
+free_key:
+ kfree(key);
+err_put:
+ fdput(f);
+ return err;
+}
+
SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
{
union bpf_attr attr = {};
@@ -135,6 +358,18 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
case BPF_MAP_CREATE:
err = map_create(&attr);
break;
+ case BPF_MAP_LOOKUP_ELEM:
+ err = map_lookup_elem(&attr);
+ break;
+ case BPF_MAP_UPDATE_ELEM:
+ err = map_update_elem(&attr);
+ break;
+ case BPF_MAP_DELETE_ELEM:
+ err = map_delete_elem(&attr);
+ break;
+ case BPF_MAP_GET_NEXT_KEY:
+ err = map_get_next_key(&attr);
+ break;
default:
err = -EINVAL;
break;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v13 net-next 02/11] bpf: enable bpf syscall on x64 and i386
From: Alexei Starovoitov @ 2014-09-17 0:39 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1410914370-29883-1-git-send-email-ast@plumgrid.com>
done as separate commit to ease conflict resolution
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
arch/x86/syscalls/syscall_32.tbl | 1 +
arch/x86/syscalls/syscall_64.tbl | 1 +
include/linux/syscalls.h | 3 ++-
include/uapi/asm-generic/unistd.h | 4 +++-
kernel/sys_ni.c | 3 +++
5 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
index 028b78168d85..9fe1b5d002f0 100644
--- a/arch/x86/syscalls/syscall_32.tbl
+++ b/arch/x86/syscalls/syscall_32.tbl
@@ -363,3 +363,4 @@
354 i386 seccomp sys_seccomp
355 i386 getrandom sys_getrandom
356 i386 memfd_create sys_memfd_create
+357 i386 bpf sys_bpf
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 35dd922727b9..281150b539a2 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -327,6 +327,7 @@
318 common getrandom sys_getrandom
319 common memfd_create sys_memfd_create
320 common kexec_file_load sys_kexec_file_load
+321 common bpf sys_bpf
#
# x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 0f86d85a9ce4..bda9b81357cc 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -65,6 +65,7 @@ struct old_linux_dirent;
struct perf_event_attr;
struct file_handle;
struct sigaltstack;
+union bpf_attr;
#include <linux/types.h>
#include <linux/aio_abi.h>
@@ -875,5 +876,5 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
const char __user *uargs);
asmlinkage long sys_getrandom(char __user *buf, size_t count,
unsigned int flags);
-
+asmlinkage long sys_bpf(int cmd, union bpf_attr *attr, unsigned int size);
#endif
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 11d11bc5c78f..22749c134117 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -705,9 +705,11 @@ __SYSCALL(__NR_seccomp, sys_seccomp)
__SYSCALL(__NR_getrandom, sys_getrandom)
#define __NR_memfd_create 279
__SYSCALL(__NR_memfd_create, sys_memfd_create)
+#define __NR_bpf 280
+__SYSCALL(__NR_bpf, sys_bpf)
#undef __NR_syscalls
-#define __NR_syscalls 280
+#define __NR_syscalls 281
/*
* All syscalls below here should go away really,
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 391d4ddb6f4b..b4b5083f5f5e 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -218,3 +218,6 @@ cond_syscall(sys_kcmp);
/* operate on Secure Computing state */
cond_syscall(sys_seccomp);
+
+/* access BPF programs and maps */
+cond_syscall(sys_bpf);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v13 net-next 01/11] bpf: introduce BPF syscall and maps
From: Alexei Starovoitov @ 2014-09-17 0:39 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1410914370-29883-1-git-send-email-ast@plumgrid.com>
BPF syscall is a multiplexor for a range of different operations on eBPF.
This patch introduces syscall with single command to create a map.
Next patch adds commands to access maps.
'maps' is a generic storage of different types for sharing data between kernel
and userspace.
Userspace example:
/* this syscall wrapper creates a map with given type and attributes
* and returns map_fd on success.
* use close(map_fd) to delete the map
*/
int bpf_create_map(enum bpf_map_type map_type, int key_size,
int value_size, int max_entries)
{
union bpf_attr attr = {
.map_type = map_type,
.key_size = key_size,
.value_size = value_size,
.max_entries = max_entries
};
return bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
}
syscall is using 'union bpf_attr' to be backwards compatible with future
extensions. Different syscall commands will use different attributes.
More details in Documentation/networking/filter.txt and in manpage
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
Documentation/networking/filter.txt | 39 ++++++++++
include/linux/bpf.h | 41 ++++++++++
include/uapi/linux/bpf.h | 23 ++++++
kernel/bpf/Makefile | 2 +-
kernel/bpf/syscall.c | 144 +++++++++++++++++++++++++++++++++++
5 files changed, 248 insertions(+), 1 deletion(-)
create mode 100644 include/linux/bpf.h
create mode 100644 kernel/bpf/syscall.c
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index 81916ab5d96f..1900d29518f1 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -1001,6 +1001,45 @@ instruction that loads 64-bit immediate value into a dst_reg.
Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
32-bit immediate value into a register.
+eBPF maps
+---------
+'maps' is a generic storage of different types for sharing data between kernel
+and userspace.
+
+The maps are accessed from user space via BPF syscall, which has commands:
+- create a map with given type and attributes
+ map_fd = bpf(BPF_MAP_CREATE, union bpf_attr *attr, u32 size)
+ using attr->map_type, attr->key_size, attr->value_size, attr->max_entries
+ returns process-local file descriptor or negative error
+
+- lookup key in a given map
+ err = bpf(BPF_MAP_LOOKUP_ELEM, union bpf_attr *attr, u32 size)
+ using attr->map_fd, attr->key, attr->value
+ returns zero and stores found elem into value or negative error
+
+- create or update key/value pair in a given map
+ err = bpf(BPF_MAP_UPDATE_ELEM, union bpf_attr *attr, u32 size)
+ using attr->map_fd, attr->key, attr->value
+ returns zero or negative error
+
+- find and delete element by key in a given map
+ err = bpf(BPF_MAP_DELETE_ELEM, union bpf_attr *attr, u32 size)
+ using attr->map_fd, attr->key
+
+- to delete map: close(fd)
+ Exiting process will delete maps automatically
+
+userspace programs use this syscall to create/access maps that eBPF programs
+are concurrently updating.
+
+maps can have different types: hash, array, bloom filter, radix-tree, etc.
+
+The map is defined by:
+ . type
+ . max number of elements
+ . key size in bytes
+ . value size in bytes
+
Testing
-------
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
new file mode 100644
index 000000000000..48014a71f0fe
--- /dev/null
+++ b/include/linux/bpf.h
@@ -0,0 +1,41 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#ifndef _LINUX_BPF_H
+#define _LINUX_BPF_H 1
+
+#include <uapi/linux/bpf.h>
+#include <linux/workqueue.h>
+
+struct bpf_map;
+
+/* map is generic key/value storage optionally accesible by eBPF programs */
+struct bpf_map_ops {
+ /* funcs callable from userspace (via syscall) */
+ struct bpf_map *(*map_alloc)(union bpf_attr *attr);
+ void (*map_free)(struct bpf_map *);
+};
+
+struct bpf_map {
+ atomic_t refcnt;
+ enum bpf_map_type map_type;
+ u32 key_size;
+ u32 value_size;
+ u32 max_entries;
+ struct bpf_map_ops *ops;
+ struct work_struct work;
+};
+
+struct bpf_map_type_list {
+ struct list_head list_node;
+ struct bpf_map_ops *ops;
+ enum bpf_map_type type;
+};
+
+void bpf_register_map_type(struct bpf_map_type_list *tl);
+void bpf_map_put(struct bpf_map *map);
+
+#endif /* _LINUX_BPF_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 479ed0b6be16..f58a10f9670c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -62,4 +62,27 @@ struct bpf_insn {
__s32 imm; /* signed immediate constant */
};
+/* BPF syscall commands */
+enum bpf_cmd {
+ /* create a map with given type and attributes
+ * fd = bpf(BPF_MAP_CREATE, union bpf_attr *, u32 size)
+ * returns fd or negative error
+ * map is deleted when fd is closed
+ */
+ BPF_MAP_CREATE,
+};
+
+enum bpf_map_type {
+ BPF_MAP_TYPE_UNSPEC,
+};
+
+union bpf_attr {
+ struct { /* anonymous struct used by BPF_MAP_CREATE command */
+ __u32 map_type; /* one of enum bpf_map_type */
+ __u32 key_size; /* size of key in bytes */
+ __u32 value_size; /* size of value in bytes */
+ __u32 max_entries; /* max number of entries in a map */
+ };
+} __attribute__((aligned(8)));
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 6a71145e2769..e9f7334ed07a 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -1 +1 @@
-obj-y := core.o
+obj-y := core.o syscall.o
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
new file mode 100644
index 000000000000..328a45a6d038
--- /dev/null
+++ b/kernel/bpf/syscall.c
@@ -0,0 +1,144 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+#include <linux/bpf.h>
+#include <linux/syscalls.h>
+#include <linux/slab.h>
+#include <linux/anon_inodes.h>
+
+static LIST_HEAD(bpf_map_types);
+
+static struct bpf_map *find_and_alloc_map(union bpf_attr *attr)
+{
+ struct bpf_map_type_list *tl;
+ struct bpf_map *map;
+
+ list_for_each_entry(tl, &bpf_map_types, list_node) {
+ if (tl->type == attr->map_type) {
+ map = tl->ops->map_alloc(attr);
+ if (IS_ERR(map))
+ return map;
+ map->ops = tl->ops;
+ map->map_type = attr->map_type;
+ return map;
+ }
+ }
+ return ERR_PTR(-EINVAL);
+}
+
+/* boot time registration of different map implementations */
+void bpf_register_map_type(struct bpf_map_type_list *tl)
+{
+ list_add(&tl->list_node, &bpf_map_types);
+}
+
+/* called from workqueue */
+static void bpf_map_free_deferred(struct work_struct *work)
+{
+ struct bpf_map *map = container_of(work, struct bpf_map, work);
+
+ /* implementation dependent freeing */
+ map->ops->map_free(map);
+}
+
+/* decrement map refcnt and schedule it for freeing via workqueue
+ * (unrelying map implementation ops->map_free() might sleep)
+ */
+void bpf_map_put(struct bpf_map *map)
+{
+ if (atomic_dec_and_test(&map->refcnt)) {
+ INIT_WORK(&map->work, bpf_map_free_deferred);
+ schedule_work(&map->work);
+ }
+}
+
+static int bpf_map_release(struct inode *inode, struct file *filp)
+{
+ struct bpf_map *map = filp->private_data;
+
+ bpf_map_put(map);
+ return 0;
+}
+
+static const struct file_operations bpf_map_fops = {
+ .release = bpf_map_release,
+};
+
+/* helper macro to check that unused fields 'union bpf_attr' are zero */
+#define CHECK_ATTR(CMD) \
+ memchr_inv((void *) &attr->CMD##_LAST_FIELD + \
+ sizeof(attr->CMD##_LAST_FIELD), 0, \
+ sizeof(*attr) - \
+ offsetof(union bpf_attr, CMD##_LAST_FIELD) - \
+ sizeof(attr->CMD##_LAST_FIELD)) != NULL
+
+#define BPF_MAP_CREATE_LAST_FIELD max_entries
+/* called via syscall */
+static int map_create(union bpf_attr *attr)
+{
+ struct bpf_map *map;
+ int err;
+
+ err = CHECK_ATTR(BPF_MAP_CREATE);
+ if (err)
+ return -EINVAL;
+
+ /* find map type and init map: hashtable vs rbtree vs bloom vs ... */
+ map = find_and_alloc_map(attr);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ atomic_set(&map->refcnt, 1);
+
+ err = anon_inode_getfd("bpf-map", &bpf_map_fops, map, O_RDWR | O_CLOEXEC);
+
+ if (err < 0)
+ /* failed to allocate fd */
+ goto free_map;
+
+ return err;
+
+free_map:
+ map->ops->map_free(map);
+ return err;
+}
+
+SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
+{
+ union bpf_attr attr = {};
+ int err;
+
+ /* the syscall is limited to root temporarily. This restriction will be
+ * lifted when security audit is clean. Note that eBPF+tracing must have
+ * this restriction, since it may pass kernel data to user space
+ */
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ /* newer userspace cannot run with older kernel */
+ if (size > sizeof(attr))
+ return -EINVAL;
+
+ /* copy attributes from user space, may be less than sizeof(bpf_attr) */
+ if (copy_from_user(&attr, uattr, size) != 0)
+ return -EFAULT;
+
+ switch (cmd) {
+ case BPF_MAP_CREATE:
+ err = map_create(&attr);
+ break;
+ default:
+ err = -EINVAL;
+ break;
+ }
+
+ return err;
+}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v13 net-next 00/11] eBPF syscall, verifier, testsuite
From: Alexei Starovoitov @ 2014-09-17 0:39 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi All,
v12 -> v13:
- replaced 'foo __user *' pointers with __aligned_u64 (suggested by David)
- added __attribute__((aligned(8)) to 'union bpf_attr' to keep
constant alignment between patches
- updated manpage and syscall wrappers due to __aligned_u64
- rebased, retested on x64 with 32-bit and 64-bit userspace and on i386,
build tested on arm32,sparc64
v11 -> v12:
- dropped patch 11 and copied few macros to libbpf.h (suggested by Daniel)
- replaced 'enum bpf_prog_type' with u32 to be safe in compat (.. Andy)
- implemented and tested compat support (not part of this set) (.. Daniel)
- changed 'void *log_buf' to 'char *' (.. Daniel)
- combined struct bpf_work_struct and bpf_prog_info (.. Daniel)
- added better return value explanation to manpage (.. Andy)
- added log_buf/log_size explanation to manpage (.. Andy & Daniel)
- added a lot more info about prog_type and map_type to manpage (.. Andy)
- rebased, tweaked test_stubs
Patches 1-4 establish BPF syscall shell for maps and programs.
Patches 5-10 add verifier step by step
Patch 11 adds test stubs for 'unspec' program type and verifier testsuite
from user space
Note that patches 1,3,4,7 add commands and attributes to the syscall
while being backwards compatible from each other, which should demonstrate
how other commands can be added in the future.
After this set the programs can be loaded for testing only. They cannot
be attached to any events. Though manpage talks about tracing and sockets,
it will be a subject of future patches.
Please take a look at manpage:
BPF(2) Linux Programmer's Manual BPF(2)
NAME
bpf - perform a command on eBPF map or program
SYNOPSIS
#include <linux/bpf.h>
int bpf(int cmd, union bpf_attr *attr, unsigned int size);
DESCRIPTION
bpf() syscall is a multiplexor for a range of different operations on
eBPF which can be characterized as "universal in-kernel virtual
machine". eBPF is similar to original Berkeley Packet Filter (or
"classic BPF") used to filter network packets. Both statically analyze
the programs before loading them into the kernel to ensure that
programs cannot harm the running system.
eBPF extends classic BPF in multiple ways including ability to call in-
kernel helper functions and access shared data structures like eBPF
maps. The programs can be written in a restricted C that is compiled
into eBPF bytecode and executed on the eBPF virtual machine or JITed
into native instruction set.
eBPF Design/Architecture
eBPF maps is a generic storage of different types. User process can
create multiple maps (with key/value being opaque bytes of data) and
access them via file descriptor. In parallel eBPF programs can access
maps from inside the kernel. It's up to user process and eBPF program
to decide what they store inside maps.
eBPF programs are similar to kernel modules. They are loaded by the
user process and automatically unloaded when process exits. Each eBPF
program is a safe run-to-completion set of instructions. eBPF verifier
statically determines that the program terminates and is safe to
execute. During verification the program takes a hold of maps that it
intends to use, so selected maps cannot be removed until the program is
unloaded. The program can be attached to different events. These events
can be packets, tracepoint events and other types in the future. A new
event triggers execution of the program which may store information
about the event in the maps. Beyond storing data the programs may call
into in-kernel helper functions which may, for example, dump stack, do
trace_printk or other forms of live kernel debugging. The same program
can be attached to multiple events. Different programs can access the
same map:
tracepoint tracepoint tracepoint sk_buff sk_buff
event A event B event C on eth0 on eth1
| | | | |
| | | | |
--> tracing <-- tracing socket socket
prog_1 prog_2 prog_3 prog_4
| | | |
|--- -----| |-------| map_3
map_1 map_2
Syscall Arguments
bpf() syscall operation is determined by cmd which can be one of the
following:
BPF_MAP_CREATE
Create a map with given type and attributes and return map FD
BPF_MAP_LOOKUP_ELEM
Lookup element by key in a given map and return its value
BPF_MAP_UPDATE_ELEM
Create or update element (key/value pair) in a given map
BPF_MAP_DELETE_ELEM
Lookup and delete element by key in a given map
BPF_MAP_GET_NEXT_KEY
Lookup element by key in a given map and return key of next
element
BPF_PROG_LOAD
Verify and load eBPF program
attr is a pointer to a union of type bpf_attr as defined below.
size is the size of the union.
union bpf_attr {
struct { /* anonymous struct used by BPF_MAP_CREATE command */
__u32 map_type;
__u32 key_size; /* size of key in bytes */
__u32 value_size; /* size of value in bytes */
__u32 max_entries; /* max number of entries in a map */
};
struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
__u32 map_fd;
__aligned_u64 key;
union {
__aligned_u64 value;
__aligned_u64 next_key;
};
};
struct { /* anonymous struct used by BPF_PROG_LOAD command */
__u32 prog_type;
__u32 insn_cnt;
__aligned_u64 insns; /* 'const struct bpf_insn *' */
__aligned_u64 license; /* 'const char *' */
__u32 log_level; /* verbosity level of eBPF verifier */
__u32 log_size; /* size of user buffer */
__aligned_u64 log_buf; /* user supplied 'char *' buffer */
};
} __attribute__((aligned(8)));
eBPF maps
maps is a generic storage of different types for sharing data between
kernel and userspace.
Any map type has the following attributes:
. type
. max number of elements
. key size in bytes
. value size in bytes
The following wrapper functions demonstrate how this syscall can be
used to access the maps. The functions use the cmd argument to invoke
different operations.
BPF_MAP_CREATE
int bpf_create_map(enum bpf_map_type map_type, int key_size,
int value_size, int max_entries)
{
union bpf_attr attr = {
.map_type = map_type,
.key_size = key_size,
.value_size = value_size,
.max_entries = max_entries
};
return bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
}
bpf() syscall creates a map of map_type type and given
attributes key_size, value_size, max_entries. On success it
returns process-local file descriptor. On error, -1 is returned
and errno is set to EINVAL or EPERM or ENOMEM.
The attributes key_size and value_size will be used by verifier
during program loading to check that program is calling
bpf_map_*_elem() helper functions with correctly initialized key
and that program doesn't access map element value beyond
specified value_size. For example, when map is created with
key_size = 8 and program does:
bpf_map_lookup_elem(map_fd, fp - 4)
such program will be rejected, since in-kernel helper function
bpf_map_lookup_elem(map_fd, void *key) expects to read 8 bytes
from 'key' pointer, but 'fp - 4' starting address will cause out
of bounds stack access.
Similarly, when map is created with value_size = 1 and program
does:
value = bpf_map_lookup_elem(...);
*(u32 *)value = 1;
such program will be rejected, since it accesses value pointer
beyond specified 1 byte value_size limit.
Currently only hash table map_type is supported:
enum bpf_map_type {
BPF_MAP_TYPE_UNSPEC,
BPF_MAP_TYPE_HASH,
};
map_type selects one of the available map implementations in
kernel. For all map_types eBPF programs access maps with the
same bpf_map_lookup_elem()/bpf_map_update_elem() helper
functions.
BPF_MAP_LOOKUP_ELEM
int bpf_lookup_elem(int fd, void *key, void *value)
{
union bpf_attr attr = {
.map_fd = fd,
.key = ptr_to_u64(key),
.value = ptr_to_u64(value),
};
return bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
}
bpf() syscall looks up an element with given key in a map fd.
If element is found it returns zero and stores element's value
into value. If element is not found it returns -1 and sets
errno to ENOENT.
BPF_MAP_UPDATE_ELEM
int bpf_update_elem(int fd, void *key, void *value)
{
union bpf_attr attr = {
.map_fd = fd,
.key = ptr_to_u64(key),
.value = ptr_to_u64(value),
};
return bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
}
The call creates or updates element with given key/value in a
map fd. On success it returns zero. On error, -1 is returned
and errno is set to EINVAL or EPERM or ENOMEM or E2BIG. E2BIG
indicates that number of elements in the map reached max_entries
limit specified at map creation time.
BPF_MAP_DELETE_ELEM
int bpf_delete_elem(int fd, void *key)
{
union bpf_attr attr = {
.map_fd = fd,
.key = ptr_to_u64(key),
};
return bpf(BPF_MAP_DELETE_ELEM, &attr, sizeof(attr));
}
The call deletes an element in a map fd with given key. Returns
zero on success. If element is not found it returns -1 and sets
errno to ENOENT.
BPF_MAP_GET_NEXT_KEY
int bpf_get_next_key(int fd, void *key, void *next_key)
{
union bpf_attr attr = {
.map_fd = fd,
.key = ptr_to_u64(key),
.next_key = ptr_to_u64(next_key),
};
return bpf(BPF_MAP_GET_NEXT_KEY, &attr, sizeof(attr));
}
The call looks up an element by key in a given map fd and
returns key of the next element into next_key pointer. If key is
not found, it return zero and returns key of the first element
into next_key. If key is the last element, it returns -1 and
sets errno to ENOENT. Other possible errno values are ENOMEM,
EFAULT, EPERM, EINVAL. This method can be used to iterate over
all elements of the map.
close(map_fd)
will delete the map map_fd. Exiting process will delete all
maps automatically.
eBPF programs
BPF_PROG_LOAD
This cmd is used to load eBPF program into the kernel.
char bpf_log_buf[LOG_BUF_SIZE];
int bpf_prog_load(enum bpf_prog_type prog_type,
const struct bpf_insn *insns, int insn_cnt,
const char *license)
{
union bpf_attr attr = {
.prog_type = prog_type,
.insns = ptr_to_u64(insns),
.insn_cnt = insn_cnt,
.license = ptr_to_u64(license),
.log_buf = ptr_to_u64(bpf_log_buf),
.log_size = LOG_BUF_SIZE,
.log_level = 1,
};
return bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
}
prog_type is one of the available program types:
enum bpf_prog_type {
BPF_PROG_TYPE_UNSPEC,
BPF_PROG_TYPE_SOCKET,
BPF_PROG_TYPE_TRACING,
};
By picking prog_type program author selects a set of helper
functions callable from eBPF program and corresponding format of
struct bpf_context (which is the data blob passed into the
program as the first argument). For example, the programs
loaded with prog_type = TYPE_TRACING may call bpf_printk()
helper, whereas TYPE_SOCKET programs may not. The set of
functions available to the programs under given type may
increase in the future.
Currently the set of functions for TYPE_TRACING is:
bpf_map_lookup_elem(map_fd, void *key) // lookup key in a map_fd
bpf_map_update_elem(map_fd, void *key, void *value) // update key/value
bpf_map_delete_elem(map_fd, void *key) // delete key in a map_fd
bpf_ktime_get_ns(void) // returns current ktime
bpf_printk(char *fmt, int fmt_size, ...) // prints into trace buffer
bpf_memcmp(void *ptr1, void *ptr2, int size) // non-faulting memcmp
bpf_fetch_ptr(void *ptr) // non-faulting load pointer from any address
bpf_fetch_u8(void *ptr) // non-faulting 1 byte load
bpf_fetch_u16(void *ptr) // other non-faulting loads
bpf_fetch_u32(void *ptr)
bpf_fetch_u64(void *ptr)
and bpf_context is defined as:
struct bpf_context {
/* argN fields match one to one to arguments passed to trace events */
u64 arg1, arg2, arg3, arg4, arg5, arg6;
/* return value from kretprobe event or from syscall_exit event */
u64 ret;
};
The set of helper functions for TYPE_SOCKET is TBD.
More program types may be added in the future. Like
BPF_PROG_TYPE_USER_TRACING for unprivileged programs.
BPF_PROG_TYPE_UNSPEC is used for testing only. Such programs
cannot be attached to events.
insns array of "struct bpf_insn" instructions
insn_cnt number of instructions in the program
license license string, which must be GPL compatible to call
helper functions marked gpl_only
log_buf user supplied buffer that in-kernel verifier is using to
store verification log. Log is a multi-line string that should
be used by program author to understand how verifier came to
conclusion that program is unsafe. The format of the output can
change at any time as verifier evolves.
log_size size of user buffer. If size of the buffer is not large
enough to store all verifier messages, -1 is returned and errno
is set to ENOSPC.
log_level verbosity level of eBPF verifier, where zero means no
logs provided
close(prog_fd)
will unload eBPF program
The maps are accesible from programs and generally tie the two
together. Programs process various events (like tracepoint, kprobe,
packets) and store the data into maps. User space fetches data from
maps. Either the same or a different map may be used by user space as
configuration space to alter program behavior on the fly.
Events
Once an eBPF program is loaded, it can be attached to an event. Various
kernel subsystems have different ways to do so. For example:
setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd, sizeof(prog_fd));
will attach the program prog_fd to socket sock which was received by
prior call to socket().
ioctl(event_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
will attach the program prog_fd to perf event event_fd which was
received by prior call to perf_event_open().
Another way to attach the program to a tracing event is:
event_fd = open("/sys/kernel/debug/tracing/events/skb/kfree_skb/filter");
write(event_fd, "bpf-123"); /* where 123 is eBPF program FD */
/* here program is attached and will be triggered by events */
close(event_fd); /* to detach from event */
EXAMPLES
/* eBPF+sockets example:
* 1. create map with maximum of 2 elements
* 2. set map[6] = 0 and map[17] = 0
* 3. load eBPF program that counts number of TCP and UDP packets received
* via map[skb->ip->proto]++
* 4. attach prog_fd to raw socket via setsockopt()
* 5. print number of received TCP/UDP packets every second
*/
int main(int ac, char **av)
{
int sock, map_fd, prog_fd, key;
long long value = 0, tcp_cnt, udp_cnt;
map_fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value), 2);
if (map_fd < 0) {
printf("failed to create map '%s'\n", strerror(errno));
/* likely not run as root */
return 1;
}
key = 6; /* ip->proto == tcp */
assert(bpf_update_elem(map_fd, &key, &value) == 0);
key = 17; /* ip->proto == udp */
assert(bpf_update_elem(map_fd, &key, &value) == 0);
struct bpf_insn prog[] = {
BPF_MOV64_REG(BPF_REG_6, BPF_REG_1), /* r6 = r1 */
BPF_LD_ABS(BPF_B, 14 + 9), /* r0 = ip->proto */
BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4),/* *(u32 *)(fp - 4) = r0 */
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), /* r2 = fp */
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), /* r2 = r2 - 4 */
BPF_LD_MAP_FD(BPF_REG_1, map_fd), /* r1 = map_fd */
BPF_CALL_FUNC(BPF_FUNC_map_lookup_elem), /* r0 = map_lookup(r1, r2) */
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2), /* if (r0 == 0) goto pc+2 */
BPF_MOV64_IMM(BPF_REG_1, 1), /* r1 = 1 */
BPF_XADD(BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* lock *(u64 *)r0 += r1 */
BPF_MOV64_IMM(BPF_REG_0, 0), /* r0 = 0 */
BPF_EXIT_INSN(), /* return r0 */
};
prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET, prog, sizeof(prog), "GPL");
assert(prog_fd >= 0);
sock = open_raw_sock("lo");
assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
sizeof(prog_fd)) == 0);
for (;;) {
key = 6;
assert(bpf_lookup_elem(map_fd, &key, &tcp_cnt) == 0);
key = 17;
assert(bpf_lookup_elem(map_fd, &key, &udp_cnt) == 0);
printf("TCP %lld UDP %lld packets0, tcp_cnt, udp_cnt);
sleep(1);
}
return 0;
}
RETURN VALUE
For a successful call, the return value depends on the operation:
BPF_MAP_CREATE
The new file descriptor associated with eBPF map.
BPF_PROG_LOAD
The new file descriptor associated with eBPF program.
All other commands
Zero.
On error, -1 is returned, and errno is set appropriately.
ERRORS
EPERM bpf() syscall was made without sufficient privilege (without the
CAP_SYS_ADMIN capability).
ENOMEM Cannot allocate sufficient memory.
EBADF fd is not an open file descriptor
EFAULT One of the pointers ( key or value or log_buf or insns ) is
outside accessible address space.
EINVAL The value specified in cmd is not recognized by this kernel.
EINVAL For BPF_MAP_CREATE, either map_type or attributes are invalid.
EINVAL For BPF_MAP_*_ELEM commands, some of the fields of "union
bpf_attr" unused by this command are not set to zero.
EINVAL For BPF_PROG_LOAD, attempt to load invalid program (unrecognized
instruction or uses reserved fields or jumps out of range or
loop detected or calls unknown function).
EACCES For BPF_PROG_LOAD, though program has valid instructions, it was
rejected, since it was deemed unsafe (may access disallowed
memory region or uninitialized stack/register or function
constraints don't match actual types or misaligned access). In
such case it is recommended to call bpf() again with log_level =
1 and examine log_buf for specific reason provided by verifier.
ENOENT For BPF_MAP_LOOKUP_ELEM or BPF_MAP_DELETE_ELEM, indicates that
element with given key was not found.
E2BIG program is too large or a map reached max_entries limit (max
number of elements).
NOTES
These commands may be used only by a privileged process (one having the
CAP_SYS_ADMIN capability).
SEE ALSO
eBPF architecture and instruction set is explained in
Documentation/networking/filter.txt
Linux 2014-09-16 BPF(2)
^ permalink raw reply
* Re: [PATCH] x86/vdso: Add prctl to set per-process VDSO load
From: Andy Lutomirski @ 2014-09-17 0:27 UTC (permalink / raw)
To: Richard Larocque
Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
filbranden-hpIqsD4AKlfQT0dZR+AlfA, md-hpIqsD4AKlfQT0dZR+AlfA,
gthelen-hpIqsD4AKlfQT0dZR+AlfA, X86 ML, Linux API,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1410912351-31273-1-git-send-email-rlarocque-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On Tue, Sep 16, 2014 at 5:05 PM, Richard Larocque <rlarocque-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> Adds new prctl calls to enable or disable VDSO loading for a process
> and its children.
>
> The PR_SET_DISABLE_VDSO call takes one argument, which is interpreted as
> a boolean value. If true, it disables the loading of the VDSO on exec()
> for this process and any children created after this call. A false
> value unsets the flag.
>
> The PR_GET_DISABLE_VDSO option returns a non-negative true value if VDSO
> loading has been disabled for this process, zero if it has not been
> disabled, and a negative value in case of error.
>
> These prctl calls are hidden behind a new Kconfig,
> CONFIG_VDSO_DISABLE_PRCTL. This feature is available only on x86.
>
> The command line option vdso=0 overrides the behavior of
> PR_SET_DISABLE_VDSO, however, PR_GET_DISABLE_VDSO will coninue to return
> whetever setting was last set with PR_SET_DISABLE_VDSO.
>
> Signed-off-by: Richard Larocque <rlarocque-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> This patch is part of some work to better handle times and CRIU migration.
> I suspect that there are other use cases out there, so I'm offering this
> patch separately.
>
> When considering CRIU migration and times, we put some thought into how
> to handle the rdtsc instruction. If we migrate between machines or across
> reboots, the migrated process will see values that could break its assumptions
> about how rdtsc is supposed to work.
I don't get it.
If __vdso_clock_gettime returns the wrong value in any scenario, we
should fix that. Simiarly, CRIU *already works*, unless there's
something I don't know of.
That being said, I would like an option to gate off RDTSC for a
process and its children in order to make PR_TSC_SIGSEGV more useful.
All the prerequisites are there now.
What problem are you trying to solve exactly?
--Andy
^ permalink raw reply
* Re: [PATCH] x86/vdso: Add prctl to set per-process VDSO load
From: Richard Larocque @ 2014-09-17 0:21 UTC (permalink / raw)
To: Andi Kleen; +Cc: lkml, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <8738brhzoc.fsf-KWJ+5VKanrL29G5dvP0v1laTQe2KTcn/@public.gmane.org>
On Tue, Sep 16, 2014 at 5:13 PM, Andi Kleen <andi-Vw/NltI1exuRpAAqCnN02g@public.gmane.org> wrote:
> Richard Larocque <rlarocque-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> writes:
>
> Perhaps I'm missing something, but how do you modify the AUX vector
> for the children?
>
>> +config VDSO_DISABLE_PRCTL
>> + depends on X86
>> + bool "prctl to disable VDSO loading"
>> + ---help---
>> + Enabling this option adds support for prctl calls that
>> + set and retrieve a per-process flag to disable VDSO loading on
>> + exec() for this process and all of its children.
>
> I don't think it makes any sense to have a config for a simple
> feature like this. Just do it unconditionally.
>
> -Andi
>
> --
> ak-VuQAYsv1563Yd54FQh9/CA@public.gmane.org -- Speaking for myself only
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
The AUX vector issue is a mistake on my part.
Speaking of mistakes, the IDs in the commit message do not match those
in the patch itself. That needs to be fixed, too.
Point taken about the config. I'll remove it in the next version of the patch.
Thanks for the feedback.
^ permalink raw reply
* Re: [PATCH] x86/vdso: Add prctl to set per-process VDSO load
From: Andi Kleen @ 2014-09-17 0:13 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-api
In-Reply-To: <1410912351-31273-1-git-send-email-rlarocque@google.com>
Richard Larocque <rlarocque@google.com> writes:
Perhaps I'm missing something, but how do you modify the AUX vector
for the children?
> +config VDSO_DISABLE_PRCTL
> + depends on X86
> + bool "prctl to disable VDSO loading"
> + ---help---
> + Enabling this option adds support for prctl calls that
> + set and retrieve a per-process flag to disable VDSO loading on
> + exec() for this process and all of its children.
I don't think it makes any sense to have a config for a simple
feature like this. Just do it unconditionally.
-Andi
--
ak@linux.intel.com -- Speaking for myself only
^ permalink raw reply
* [PATCH] x86/vdso: Add prctl to set per-process VDSO load
From: Richard Larocque @ 2014-09-17 0:05 UTC (permalink / raw)
To: mingo, tglx, hpa, luto, filbranden, md, gthelen
Cc: x86, linux-api, linux-kernel, Richard Larocque
Adds new prctl calls to enable or disable VDSO loading for a process
and its children.
The PR_SET_DISABLE_VDSO call takes one argument, which is interpreted as
a boolean value. If true, it disables the loading of the VDSO on exec()
for this process and any children created after this call. A false
value unsets the flag.
The PR_GET_DISABLE_VDSO option returns a non-negative true value if VDSO
loading has been disabled for this process, zero if it has not been
disabled, and a negative value in case of error.
These prctl calls are hidden behind a new Kconfig,
CONFIG_VDSO_DISABLE_PRCTL. This feature is available only on x86.
The command line option vdso=0 overrides the behavior of
PR_SET_DISABLE_VDSO, however, PR_GET_DISABLE_VDSO will coninue to return
whetever setting was last set with PR_SET_DISABLE_VDSO.
Signed-off-by: Richard Larocque <rlarocque@google.com>
---
This patch is part of some work to better handle times and CRIU migration.
I suspect that there are other use cases out there, so I'm offering this
patch separately.
When considering CRIU migration and times, we put some thought into how
to handle the rdtsc instruction. If we migrate between machines or across
reboots, the migrated process will see values that could break its assumptions
about how rdtsc is supposed to work. To deal with this, we could:
* let the application handle it
* ban the instruction (ie. PR_TSC_SIGSEGV), to make sure the application
doesn't use it by accident
* trap the instruction then mark the process as "tainted" for migration
purposes
* trap the instruction then apply an adjustment to keep values consistent
across machines
* do something really crazy involving the VMCS
There's no great option here. Which one we choose probably depends on
what kind of process is being migrated.
Many of these options involve setting a trap for the rdtsc instruction in the
process, which creates problems for the vDSO. The vDSO implementations of
clock_gettime() make use of that instruction from userspace. We can use
workarounds in the kernel to turn the trap into a no-op when it comes from vDSO
code, but that somewhat defeats the purpose of having a vDSO in the first
place. It was supposed to avoid unnecessary calls to kernel space. Trapping
its instructions goes against that goal.
So we think it would be nice to disable the vDSO for some processes on a
machine. This would allow us to implement some of the rdtsc handling options
without having to worry about the vDSO.
We have some additional plans for clock_gettime() and friends that may or
may not depend on disabling the vDSO, but it might be best if we defer that
part of the discussion to the next patch set. I think this patch and its
use case can stand on their own.
arch/x86/Kconfig | 8 ++++++++
arch/x86/vdso/vma.c | 18 ++++++++++++++++++
include/linux/sched.h | 5 +++++
include/uapi/linux/prctl.h | 9 +++++++++
kernel/fork.c | 4 ++++
kernel/sys.c | 16 ++++++++++++++++
6 files changed, 60 insertions(+)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3632743..ff54ead 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1875,6 +1875,14 @@ config COMPAT_VDSO
If unsure, say N: if you are compiling your own kernel, you
are unlikely to be using a buggy version of glibc.
+config VDSO_DISABLE_PRCTL
+ depends on X86
+ bool "prctl to disable VDSO loading"
+ ---help---
+ Enabling this option adds support for prctl calls that
+ set and retrieve a per-process flag to disable VDSO loading on
+ exec() for this process and all of its children.
+
config CMDLINE_BOOL
bool "Built-in kernel command line"
---help---
diff --git a/arch/x86/vdso/vma.c b/arch/x86/vdso/vma.c
index 970463b..496c48b 100644
--- a/arch/x86/vdso/vma.c
+++ b/arch/x86/vdso/vma.c
@@ -23,6 +23,15 @@ unsigned int __read_mostly vdso64_enabled = 1;
extern unsigned short vdso_sync_cpuid;
#endif
+static int vdso_enabled_for_current_process(void)
+{
+#if defined(CONFIG_VDSO_DISABLE_PRCTL)
+ return !current->signal->disable_vdso;
+#else
+ return 1;
+#endif
+}
+
void __init init_vdso_image(const struct vdso_image *image)
{
int i;
@@ -185,6 +194,9 @@ static int load_vdso32(void)
if (vdso32_enabled != 1) /* Other values all mean "disabled" */
return 0;
+ if (!vdso_enabled_for_current_process())
+ return 0;
+
ret = map_vdso(selected_vdso32, false);
if (ret)
return ret;
@@ -204,6 +216,9 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
if (!vdso64_enabled)
return 0;
+ if (!vdso_enabled_for_current_process())
+ return 0;
+
return map_vdso(&vdso_image_64, true);
}
@@ -216,6 +231,9 @@ int compat_arch_setup_additional_pages(struct linux_binprm *bprm,
if (!vdso64_enabled)
return 0;
+ if (!vdso_enabled_for_current_process())
+ return 0;
+
return map_vdso(&vdso_image_x32, true);
}
#endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 5c2c885..37f6a7a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -708,6 +708,11 @@ struct signal_struct {
struct mutex cred_guard_mutex; /* guard against foreign influences on
* credential calculations
* (notably. ptrace) */
+
+#ifdef CONFIG_VDSO_DISABLE_PRCTL
+ unsigned int disable_vdso; /* If true, prevents loading of VDSO on
+ next exec() */
+#endif
};
/*
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 58afc04..3dbbeda 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -152,4 +152,13 @@
#define PR_SET_THP_DISABLE 41
#define PR_GET_THP_DISABLE 42
+/*
+ * These can be used to flag a process so that neither it nor its children will
+ * receive VDSO mappings on their next exec() call.
+ */
+#define PR_SET_VDSO 43
+#define PR_GET_VDSO 44
+# define PR_VDSO_DISABLE 0 /* prevent loading of VDSO on exec() */
+# define PR_VDSO_ENABLE 1 /* allow loading of VDSO on exec() */
+
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/fork.c b/kernel/fork.c
index 0cf9cdb..11ede19 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1091,6 +1091,10 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
sig->has_child_subreaper = current->signal->has_child_subreaper ||
current->signal->is_child_subreaper;
+#ifdef CONFIG_VDSO_DISABLE_PRCTL
+ sig->disable_vdso = current->signal->disable_vdso;
+#endif
+
mutex_init(&sig->cred_guard_mutex);
return 0;
diff --git a/kernel/sys.c b/kernel/sys.c
index ce81291..eb94a96 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2011,6 +2011,22 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
me->mm->def_flags &= ~VM_NOHUGEPAGE;
up_write(&me->mm->mmap_sem);
break;
+#ifdef CONFIG_VDSO_DISABLE_PRCTL
+ case PR_SET_VDSO:
+ if (arg2 == PR_VDSO_ENABLE)
+ me->signal->disable_vdso = 0;
+ else if (arg2 == PR_VDSO_DISABLE)
+ me->signal->disable_vdso = 1;
+ else
+ return -EINVAL;
+ break;
+ case PR_GET_VDSO:
+ if (!me->signal->disable_vdso)
+ error = put_user(PR_VDSO_ENABLE, (int __user *)arg2);
+ else
+ error = put_user(PR_VDSO_DISABLE, (int __user *)arg2);
+ break;
+#endif
default:
error = -EINVAL;
break;
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [PATCH v12 net-next 03/11] bpf: add lookup/update/delete/iterate methods to BPF maps
From: Alexei Starovoitov @ 2014-09-16 22:06 UTC (permalink / raw)
To: David Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140916.174901.2174503796369572537.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Tue, Sep 16, 2014 at 2:49 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
> Date: Tue, 16 Sep 2014 14:23:27 -0700
>
>> no compat layer and type checking will be done
>> by syscall wrappers. Ok?
>
> Why are you against using strong typing just for everything other
> than the user pointer blobs?
>
> I don't understand the resistence to my suggestion to just use
> aligned_u64 instead of "void __user *" in the union members.
All these different variants are ok to me. There are pros and
cons to either approach. I'm not against strong typing.
I just thought it would be cleaner not to use 'union' and
was asking for opinion. That's all. Sure, I will keep 'union'
and only change pointers to __aligned_u64.
^ permalink raw reply
* Re: [PATCH v12 net-next 03/11] bpf: add lookup/update/delete/iterate methods to BPF maps
From: David Miller @ 2014-09-16 21:49 UTC (permalink / raw)
To: ast
Cc: mingo, torvalds, luto, dborkman, hannes, chema, edumazet,
a.p.zijlstra, pablo, hpa, akpm, keescook, linux-api, netdev,
linux-kernel
In-Reply-To: <CAMEtUuzy0Hvjr0f4XBrH+FAxaKD7aijnYzDe3-MPrGWe6+MD4Q@mail.gmail.com>
From: Alexei Starovoitov <ast@plumgrid.com>
Date: Tue, 16 Sep 2014 14:23:27 -0700
> no compat layer and type checking will be done
> by syscall wrappers. Ok?
Why are you against using strong typing just for everything other
than the user pointer blobs?
I don't understand the resistence to my suggestion to just use
aligned_u64 instead of "void __user *" in the union members.
^ permalink raw reply
* Re: [PATCH v12 net-next 03/11] bpf: add lookup/update/delete/iterate methods to BPF maps
From: Alexei Starovoitov @ 2014-09-16 21:23 UTC (permalink / raw)
To: David Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140916.165452.1747695341192703967.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Tue, Sep 16, 2014 at 1:54 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
>
> The cost of the compat layer must be considered and weighted
> against this cast, which I think is really no big deal.
>
>> I think compat layer is a better option.
>
> It's overhead you'll have to support forever, I think you should
> reconsider.
>
> All of the "ugly casting" will be hidden, or can be hidden, in the
> syscall wrappers and/or interfaces in userspace.
ahh, ok. I thought you're strongly against any type of casts.
In such case I can get rid of 'union bpf_attr' as well and simply
define a struct per command, then syscall will look like:
sys_bpf(int cmd, void __user *attr, unsigned int size);
and uapi/linux/bpf.h will have:
struct bpf_prog_load_attr { /* for BPF_PROG_LOAD cmd */
__u32 prog_type;
__u32 insn_cnt;
__aligned_u64 insns;
__aligned_u64 license;
__u32 log_level;
__u32 log_size;
__aligned_u64 log_buf;
};
and similar for other commands.
no compat layer and type checking will be done
by syscall wrappers. Ok?
^ permalink raw reply
* Re: [PATCH v12 net-next 03/11] bpf: add lookup/update/delete/iterate methods to BPF maps
From: David Miller @ 2014-09-16 20:54 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
luto-kltTT9wpgjJwATOyAt5JVQ, dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, pablo-Cap9r6Oaw4JrovVCs/uTlw,
hpa-YMNOUZJC4hwAvxtiuMwx3w, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAMEtUuxp6709mdygiQyCqwjv-0kF-CbNOzCCZmKw_mUckF4NZA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Tue, 16 Sep 2014 13:44:12 -0700
> the union indeed changes alignment from patch #1 to #3,
> but, imo, it is not a problem, since kernel does:
It changes the alignment of the datastructures in userspace.
> I haven't thought of 'aligned_u64' for this case.
> For counters and masks it would be perfect, but here user is
> passing real pointers to key and value, so they have to
> be 'void __user *', otherwise user would need to type cast
> them, which I want to avoid.
The cost of the compat layer must be considered and weighted
against this cast, which I think is really no big deal.
> I think compat layer is a better option.
It's overhead you'll have to support forever, I think you should
reconsider.
All of the "ugly casting" will be hidden, or can be hidden, in the
syscall wrappers and/or interfaces in userspace.
^ permalink raw reply
* Re: [PATCH v12 net-next 03/11] bpf: add lookup/update/delete/iterate methods to BPF maps
From: Alexei Starovoitov @ 2014-09-16 20:44 UTC (permalink / raw)
To: David Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140916.151606.448751391205382097.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Tue, Sep 16, 2014 at 12:16 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
> Date: Mon, 15 Sep 2014 12:18:33 -0700
>
>> @@ -83,6 +112,15 @@ union bpf_attr {
>> __u32 value_size; /* size of value in bytes */
>> __u32 max_entries; /* max number of entries in a map */
>> };
>> +
>> + struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
>> + int map_fd;
>> + void __user *key;
>> + union {
>> + void __user *value;
>> + void __user *next_key;
>> + };
>> + };
>> };
>>
>> #endif /* _UAPI__LINUX_BPF_H__ */
>
> Depending upon the processor ABI, this change can increase the
> alignment requirements of union bpf_attr. So the structure is not
> compatible between patch #1 and patch #3 here.
the union indeed changes alignment from patch #1 to #3,
but, imo, it is not a problem, since kernel does:
/* copy attributes from user space, may be less than sizeof(bpf_attr) */
if (copy_from_user(attr, uattr, size) != 0)
and then proceeds with further bpf_attr validation, so even
if user space alignment is 4 and user assumes contents
from patch 1, but kernel alignment is 8 and contents from
patch 3, it is still ok. Backwards compatibility is preserved.
> Also, you haven't implemented any compat layer whatsoever for the
> necessary translations. This happens because you are using pointers
> which are different sized between 32-bit and 64-bit ABIs.
I mentioned it in the cover letter:
- implemented and tested compat support (not part of this set)
I'll roll compat layer into the set to make it less confusing.
> I would suggest you use instead something like "aligned_u64" since
> these are just arbitrary userland cookies and using "aligned_u64"
> vs. "u64" will make it so that you don't have to deal with the 64-bit
> type alignment differences between x86-32 and x86-64 while writing the
> compat wrappers (if any).
I haven't thought of 'aligned_u64' for this case.
For counters and masks it would be perfect, but here user is
passing real pointers to key and value, so they have to
be 'void __user *', otherwise user would need to type cast
them, which I want to avoid.
>> + struct { /* anonymous struct used by BPF_PROG_LOAD command */
>> + __u32 prog_type; /* one of enum bpf_prog_type */
>> + __u32 insn_cnt;
>> + const struct bpf_insn __user *insns;
>> + const char __user *license;
>> + };
>> +};
>
> Again, these need syscall compat handling. You may want to redesign
> the types so that perhaps no compat layer translations will be needed.
'foo __user *' pointers vs 'aligned_u64'... It's a trade off.
I can make all pointer fields to be u64 and then, indeed,
no compat layer will be needed, but user would need to
do ugly type casts. I think compat layer is a better option.
It's simple enough. I'll roll it into the set and respin.
Thanks!
^ permalink raw reply
* Re: [PATCH v12 net-next 04/11] bpf: expand BPF syscall with program load/unload
From: David Miller @ 2014-09-16 19:17 UTC (permalink / raw)
To: ast
Cc: mingo, torvalds, luto, dborkman, hannes, chema, edumazet,
a.p.zijlstra, pablo, hpa, akpm, keescook, linux-api, netdev,
linux-kernel
In-Reply-To: <1410808721-27493-5-git-send-email-ast@plumgrid.com>
From: Alexei Starovoitov <ast@plumgrid.com>
Date: Mon, 15 Sep 2014 12:18:34 -0700
> @@ -121,6 +132,21 @@ union bpf_attr {
> void __user *next_key;
> };
> };
> +
> + struct { /* anonymous struct used by BPF_PROG_LOAD command */
> + __u32 prog_type; /* one of enum bpf_prog_type */
> + __u32 insn_cnt;
> + const struct bpf_insn __user *insns;
> + const char __user *license;
> + };
> +};
Again, these need syscall compat handling. You may want to redesign
the types so that perhaps no compat layer translations will be needed.
^ permalink raw reply
* Re: [PATCH v12 net-next 03/11] bpf: add lookup/update/delete/iterate methods to BPF maps
From: David Miller @ 2014-09-16 19:16 UTC (permalink / raw)
To: ast
Cc: mingo, torvalds, luto, dborkman, hannes, chema, edumazet,
a.p.zijlstra, pablo, hpa, akpm, keescook, linux-api, netdev,
linux-kernel
In-Reply-To: <1410808721-27493-4-git-send-email-ast@plumgrid.com>
From: Alexei Starovoitov <ast@plumgrid.com>
Date: Mon, 15 Sep 2014 12:18:33 -0700
> @@ -83,6 +112,15 @@ union bpf_attr {
> __u32 value_size; /* size of value in bytes */
> __u32 max_entries; /* max number of entries in a map */
> };
> +
> + struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
> + int map_fd;
> + void __user *key;
> + union {
> + void __user *value;
> + void __user *next_key;
> + };
> + };
> };
>
> #endif /* _UAPI__LINUX_BPF_H__ */
Depending upon the processor ABI, this change can increase the
alignment requirements of union bpf_attr. So the structure is not
compatible between patch #1 and patch #3 here.
Also, you haven't implemented any compat layer whatsoever for the
necessary translations. This happens because you are using pointers
which are different sized between 32-bit and 64-bit ABIs.
I would suggest you use instead something like "aligned_u64" since
these are just arbitrary userland cookies and using "aligned_u64"
vs. "u64" will make it so that you don't have to deal with the 64-bit
type alignment differences between x86-32 and x86-64 while writing the
compat wrappers (if any).
^ permalink raw reply
* Re: [RFC 2/2] perf: Marker software event and ioctl
From: Ingo Molnar @ 2014-09-16 17:58 UTC (permalink / raw)
To: Pawel Moll
Cc: David Ahern, Arnaldo Carvalho de Melo, Richard Cochran,
Steven Rostedt, Ingo Molnar, Peter Zijlstra, Paul Mackerras,
John Stultz, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1410885463.12376.24.camel@hornet>
* Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org> wrote:
> On Tue, 2014-09-16 at 08:44 +0100, Ingo Molnar wrote:
> > I think adding an ioctl to inject user-provided data into the
> > event stream is sensible, as long as there's a separate 'user
> > generated data' event for it, etc.
> >
> > The main usecase I could see would be to introduce a
> > perf_printf() variant, supported by 'perf trace' by default, to
> > add various tracable printouts to apps.
> >
> > Timestamps generated by apps would be another usecase. It would
> > probably be wise to add a 32-bit (or 64-bit) message type ID,
> > plus a length field, with a message type registry somewhere in
> > tools/perf/ (and reference implementation for each new subtype),
> > to keep things organized yet flexible going forward.
>
> Right, so this is pretty much what I got talking to Arnaldo...
>
> > { u64 type; /* 0 means zero-terminated string in data */
> > u32 size;
> > char data[size]; } && PERF_SAMPLE_MARKER
>
> ... with one type - 0 - defined as a "universal" string (so any
> possible tool knows what to do about it), the rest being left
> to userspace (this "registry" you mention).
>
> Before I proceed any further, is the term "marker" acceptable?
> Maybe a "printf" instead? Or a "log"? As we know naming is
> often single most discussed subject when it comes to new things
> in the kernel ;-)
Well, it's a user-space generated trace/event entry, so lets call
it that?
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH 1/7] selftests: add header file for test exit code defines
From: Shuah Khan @ 2014-09-16 17:47 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Davidlohr Bueso, Andrew Morton, Greg KH,
colin.king-Z7WLFzj8eWMS+FvcfC7Uqw, Eric W. Biederman,
Serge Hallyn, thierry-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
felipensp-Re5JQEeQqe8AvxtiuMwx3w, Linux API,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Shuah Khan
In-Reply-To: <CALCETrUw21Bgd9N4wakP08OiFshxzPS6wBE6=Fd2K=k33RDgXA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 09/16/2014 11:40 AM, Andy Lutomirski wrote:
> On Tue, Sep 16, 2014 at 10:31 AM, Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
>
> I think we will want a framework in the tree, but it can be very
> minimal. But I also think that using exit(2) for this is wrong. Why
> not:
>
> enum ktest_result {
> KTEST_PASS,
> ...,
> };
>
> void ktest_exit(enum ktest_result result);
>
> With the possibility of further extensions for more than one test (and
> associated result) per execution of the test binary.
>
Nothing wrong with the approach. It is one of the options I considered
and decided against it primarily because tests need more changes than
the ones needed for return codes. However, maybe it is worth while to
do so. I will play with this and see how extensive the changes are.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH 1/7] selftests: add header file for test exit code defines
From: Andy Lutomirski @ 2014-09-16 17:40 UTC (permalink / raw)
To: Shuah Khan
Cc: Davidlohr Bueso, Andrew Morton, Greg KH,
colin.king-Z7WLFzj8eWMS+FvcfC7Uqw, Eric W. Biederman,
Serge Hallyn, thierry-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
felipensp-Re5JQEeQqe8AvxtiuMwx3w, Linux API,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <54187400.10506-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Tue, Sep 16, 2014 at 10:31 AM, Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
> On 09/16/2014 10:04 AM, Davidlohr Bueso wrote:
>> On Mon, 2014-09-15 at 16:33 -0600, Shuah Khan wrote:
>>> Add a new header file that defines exit codes for individual
>>> tests to use to communicate test results. These defines are
>>> intended to provide a common and uniform way for selftests
>>> to report results. pass/fail/xfail/xpass/skip/unsupported
>>> are defined.
>>>
>>> Signed-off-by: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>>> ---
>>> tools/testing/selftests/kselftest.h | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>> create mode 100644 tools/testing/selftests/kselftest.h
>>>
>>> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
>>> new file mode 100644
>>> index 0000000..1b1c9cb
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/kselftest.h
>>> @@ -0,0 +1,20 @@
>>> +/*
>>> + * kselftest.h - kselftest framework return codes to include from
>>> + * selftests.
>>> + *
>>> + * Copyright (c) 2014 Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>>> + *
>>> + * This file is released under the GPLv2.
>>> + */
>>> +#ifndef __KSELFTEST_H
>>> +#define __KSELFTEST_H
>>> +
>>> +#define EXIT_PASS 0
>>> +#define EXIT_FAIL 1
>>> +#define EXIT_XFAIL 2
>>> +#define EXIT_XPASS 3
>>> +#define EXIT_SKIP 4
>>> +#define EXIT_UNSUPPORTED EXIT_SKIP
>>
>> Looks to me like a potential name clashes here.
>>
>> What's the difference between XFAIL/XPASS and regular FAIL/PASS (I don't
>> see the former used in patchset either, only PASS/FAIL)? What's the
>> purpose of EXIT_SKIP? I think overall these should be commented.
>
> Yeah Comments would have been nice. :) I will add them.
>
>>
>> Also, in the bigger picture, I'm guessing you have a reason for not
>> recycling errno and inventing your own exit codes... How do you plan on
>> using these? In addition I'm seeing things like:
>>
>> - exit(EXIT_FAILURE);
>> + exit(EXIT_FAIL);
>>
>> which isn't a very good idea in general.
>
> EXIT_FAILURE happens to have the same value as EXIT_FAIL. That said,
> I do understand what you are saying. EXIT_FAILURE and EXIT_SUCCESS
> are defined in stdlib.h - I would have liked to simply use them,
> however that won't meet the needs. More on this below.
>
> At the moment there is no clear way to tell why a test failed. Some
> tests exit with -1, some exit 1, and some with errno. One of the
> requests/requirements that was discussed at the kernel summit kselftest
> session was to enhance tests to report why an individual test failed.
> Returning and/or exiting with -1, 1 and errno doesn't tell the user
> anything other than that the test failed. Even without this request, it
> will helps us developers if we have a uniform reporting in place for
> all tests to use.
>
> We have two kinds of users for these tests.
>
> 1. Developers that simply want to regression test their individual areas
> These are the users that don't care about the categories of failures.
> 2. Users that want to run them from their user-space test suites. These
> users care to know why a test failed, not just that it failed.
>
> errno is useful in pin-pointing the failure for a developer, however it
> is not very useful for somebody that is running sanity checks. We need
> both, hence I changed some of the tests in this series to print errno.
> Several tests print errno in their error legs and there a few places
> that don't.
>
> In either case, it would be good to report if a test failed because
> a modules it needs isn't configured or it just failed.
>
> There is also a need to report the following cases in addition a simple
> pass/fail:
>
> pass - test passed
> fail - it failed
> xfail - a test that expected to fail failed as expected (this is really
> a pass case)
> xpass - a test that is expected to fail passed.
> xskip or xunsupported - test couldn't run because of unmet dependencies.
>
> These types of decisions on why test failed, can only be made in the
> individual tests.
>
> I picked the POSIX conforming test codes that are used by various user
> space test suites. POSIX right, I can't go wrong :)
>
> I also want to avoid adding some test framework in kernel tree, hence
> I simply defined these in a header file. Another goal is to not make it
> hard for developers write these tests and think too much about the
> reporting. We need some way to report these and hence the need for a
> common defines so tests can simply use them.
I think we will want a framework in the tree, but it can be very
minimal. But I also think that using exit(2) for this is wrong. Why
not:
enum ktest_result {
KTEST_PASS,
...,
};
void ktest_exit(enum ktest_result result);
With the possibility of further extensions for more than one test (and
associated result) per execution of the test binary.
--Andy
>
> I am trying to balance the needs of the two types of users and also
> do minimal changes to existing test with a light weight framework.
> Hope this helps explain this patch series better.
>
> I am open to suggestions as always.
>
> thanks,
> -- Shuah
>
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Samsung Research America (Silicon Valley)
> shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH 1/7] selftests: add header file for test exit code defines
From: Shuah Khan @ 2014-09-16 17:31 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
colin.king-Z7WLFzj8eWMS+FvcfC7Uqw,
ebiederm-aS9lmoZGLiVWk0Htik3J/w,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
thierry-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
felipensp-Re5JQEeQqe8AvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Shuah Khan
In-Reply-To: <1410883441.4630.9.camel-dxKd5G12XOI1EaDjlw0dpg@public.gmane.org>
On 09/16/2014 10:04 AM, Davidlohr Bueso wrote:
> On Mon, 2014-09-15 at 16:33 -0600, Shuah Khan wrote:
>> Add a new header file that defines exit codes for individual
>> tests to use to communicate test results. These defines are
>> intended to provide a common and uniform way for selftests
>> to report results. pass/fail/xfail/xpass/skip/unsupported
>> are defined.
>>
>> Signed-off-by: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>> ---
>> tools/testing/selftests/kselftest.h | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>> create mode 100644 tools/testing/selftests/kselftest.h
>>
>> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
>> new file mode 100644
>> index 0000000..1b1c9cb
>> --- /dev/null
>> +++ b/tools/testing/selftests/kselftest.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + * kselftest.h - kselftest framework return codes to include from
>> + * selftests.
>> + *
>> + * Copyright (c) 2014 Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>> + *
>> + * This file is released under the GPLv2.
>> + */
>> +#ifndef __KSELFTEST_H
>> +#define __KSELFTEST_H
>> +
>> +#define EXIT_PASS 0
>> +#define EXIT_FAIL 1
>> +#define EXIT_XFAIL 2
>> +#define EXIT_XPASS 3
>> +#define EXIT_SKIP 4
>> +#define EXIT_UNSUPPORTED EXIT_SKIP
>
> Looks to me like a potential name clashes here.
>
> What's the difference between XFAIL/XPASS and regular FAIL/PASS (I don't
> see the former used in patchset either, only PASS/FAIL)? What's the
> purpose of EXIT_SKIP? I think overall these should be commented.
Yeah Comments would have been nice. :) I will add them.
>
> Also, in the bigger picture, I'm guessing you have a reason for not
> recycling errno and inventing your own exit codes... How do you plan on
> using these? In addition I'm seeing things like:
>
> - exit(EXIT_FAILURE);
> + exit(EXIT_FAIL);
>
> which isn't a very good idea in general.
EXIT_FAILURE happens to have the same value as EXIT_FAIL. That said,
I do understand what you are saying. EXIT_FAILURE and EXIT_SUCCESS
are defined in stdlib.h - I would have liked to simply use them,
however that won't meet the needs. More on this below.
At the moment there is no clear way to tell why a test failed. Some
tests exit with -1, some exit 1, and some with errno. One of the
requests/requirements that was discussed at the kernel summit kselftest
session was to enhance tests to report why an individual test failed.
Returning and/or exiting with -1, 1 and errno doesn't tell the user
anything other than that the test failed. Even without this request, it
will helps us developers if we have a uniform reporting in place for
all tests to use.
We have two kinds of users for these tests.
1. Developers that simply want to regression test their individual areas
These are the users that don't care about the categories of failures.
2. Users that want to run them from their user-space test suites. These
users care to know why a test failed, not just that it failed.
errno is useful in pin-pointing the failure for a developer, however it
is not very useful for somebody that is running sanity checks. We need
both, hence I changed some of the tests in this series to print errno.
Several tests print errno in their error legs and there a few places
that don't.
In either case, it would be good to report if a test failed because
a modules it needs isn't configured or it just failed.
There is also a need to report the following cases in addition a simple
pass/fail:
pass - test passed
fail - it failed
xfail - a test that expected to fail failed as expected (this is really
a pass case)
xpass - a test that is expected to fail passed.
xskip or xunsupported - test couldn't run because of unmet dependencies.
These types of decisions on why test failed, can only be made in the
individual tests.
I picked the POSIX conforming test codes that are used by various user
space test suites. POSIX right, I can't go wrong :)
I also want to avoid adding some test framework in kernel tree, hence
I simply defined these in a header file. Another goal is to not make it
hard for developers write these tests and think too much about the
reporting. We need some way to report these and hence the need for a
common defines so tests can simply use them.
I am trying to balance the needs of the two types of users and also
do minimal changes to existing test with a light weight framework.
Hope this helps explain this patch series better.
I am open to suggestions as always.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [RFC 2/2] perf: Marker software event and ioctl
From: Pawel Moll @ 2014-09-16 16:37 UTC (permalink / raw)
To: Ingo Molnar
Cc: David Ahern, Arnaldo Carvalho de Melo, Richard Cochran,
Steven Rostedt, Ingo Molnar, Peter Zijlstra, Paul Mackerras,
John Stultz, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20140916074421.GA21295-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Tue, 2014-09-16 at 08:44 +0100, Ingo Molnar wrote:
> I think adding an ioctl to inject user-provided data into the
> event stream is sensible, as long as there's a separate 'user
> generated data' event for it, etc.
>
> The main usecase I could see would be to introduce a
> perf_printf() variant, supported by 'perf trace' by default, to
> add various tracable printouts to apps.
>
> Timestamps generated by apps would be another usecase. It would
> probably be wise to add a 32-bit (or 64-bit) message type ID,
> plus a length field, with a message type registry somewhere in
> tools/perf/ (and reference implementation for each new subtype),
> to keep things organized yet flexible going forward.
Right, so this is pretty much what I got talking to Arnaldo...
> { u64 type; /* 0 means zero-terminated string in data */
> u32 size;
> char data[size]; } && PERF_SAMPLE_MARKER
... with one type - 0 - defined as a "universal" string (so any possible
tool knows what to do about it), the rest being left to userspace (this
"registry" you mention).
Before I proceed any further, is the term "marker" acceptable? Maybe a
"printf" instead? Or a "log"? As we know naming is often single most
discussed subject when it comes to new things in the kernel ;-)
Pawel
^ permalink raw reply
* Re: [RFC 2/2] perf: Marker software event and ioctl
From: Pawel Moll @ 2014-09-16 16:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Christopher Covington, Richard Cochran, Steven Rostedt,
Ingo Molnar, Peter Zijlstra, Paul Mackerras, John Stultz,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20140915183101.GE11199-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On Mon, 2014-09-15 at 19:31 +0100, Arnaldo Carvalho de Melo wrote:
> Em Mon, Sep 15, 2014 at 06:27:14PM +0100, Pawel Moll escreveu:
> > On Fri, 2014-09-12 at 17:19 +0100, Arnaldo Carvalho de Melo wrote:
> > > Em Fri, Sep 12, 2014 at 02:58:55PM +0100, Pawel Moll escreveu:
> > > > On Fri, 2014-09-12 at 14:49 +0100, Arnaldo Carvalho de Melo wrote:
> > > > > Perhaps both? I.e. an u64 followed from a string, if the u64 is zero,
> > > > > then there is a string right after it?
> > >
> > > > How would this look like in userspace? Something like this?
> > >
> > > > 8<----
> > > > struct perf_event_marker {
> > > > uint64_t value;
> > > > char *string;
> > > > } arg;
> > >
> > > > arg.value = 0x1234;
> > >
> > > > /* or */
> > >
> > > > arg.value = 0;
> > > > arg.string = "abcd";
> > >
> > > > ioctl(fd, PERF_EVENT_IOC_MARKER, &arg)
> > > > 8<----
> > >
> > > > If so, maybe it would simpler just to go for classic size/data
> > > > structure?
> > >
> > > > 8<-----
> > > > struct perf_event_marker {
> > > > uint32_t size;
> > > > void *data;
> > > > }
> > > > 8<-----
> > >
> > > > This would directly map into struct perf_raw_record...
> > >
> > > I can see the usefulness of having it all, i.e. if we do just:
> > >
> > > perf trace --pid `pidof some-tool-in-debug-mode-using-this-interface`
> >
> > Hm. I haven't thought about a situation when 3rd party wants to inject
> > something into "my" data stream... I guess it could be implemented (a
>
> I was thinking about intercepting calls that pass some logging data, as
> strings, and 'tee' them to the 'perf trace' 'data stream'.
Right, ok, like LD_PRELOADing printf (a stupid example of course) and
piping it inside perf... So if I'm getting it right, it's the perf
process that would eventually do the ioctl(PERF_EVENT_IOC_MARKER), not
the traced process, correct? This makes sense. Another use case for
ioctl justification, thanks :-)
> > "myself"?), but will definitely complicate the patch. Should I have a
> > look at it now or maybe leave it till we get a general agreement about
> > the marker ioctl existence?
> >
> > > Then 'perf trace' doesn't know about any binary format a tool may have,
> > > getting strings there (hey, LD_PRELOADing some logging library to hook
> > > into this comes to mind) and having it merged with other events
> > > (syscalls, pagefaults, etc) looks useful.
> >
> > But do you still mean a "magic" u64 before the rest? Injecting a string
> > would just mean:
> >
> > marker.size = strlen(s) + 1;
> > marker.data = s;
> >
> > > As well as some specialized version of 'perf trace' that knows about
> > > some binary protocol that would get app specific stats or lock status,
> > > etc, perhaps even plugins for 'perf trace' that would be selected by
> > > that first u64? Also seems useful.
> > >
> > > I.e. having a way to provide just strings and another that would allow
> > > passing perf_raw_record.
> >
> > Sounds interesting. But then maybe this stuff shouldn't go into "raw"
> > then? It could be something like this in the sample:
> >
> > { u64 type; /* 0 means zero-terminated string in data */
> > u32 size;
> > char data[size]; } && PERF_SAMPLE_MARKER
>
> Yes, this is how I think it should be.
Seems that Ingo had exactly the same thing on mind. I'll get a patch
done.
Paweł
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox