[RFC PATCH 09/11] bpf: mark helpers explicitly whether they may change

From: Kris Van Hees
Date: Tue May 21 2019 - 16:43:27 EST


Some helpers may update the context. Right now, various network filter
helpers may make changes to the packet data. This is verified by calling
the bpf_helper_changes_pkt_data() function with the function pointer.

This function resides in net/core/filter.c and needs to be updated for any
helper function that modifies packet data. To allow for other helpers
(possibly not part of the network filter code) to do the same, this patch
changes the code from using a central function to list all helpers that
have this feature to marking each individual helper that may change the
context data. This way, whenever a new helper is added that may change
the content of the context, there is no need to update a hardcoded list of
functions.

Signed-off-by: Kris Van Hees <kris.van.hees@xxxxxxxxxx>
Reviewed-by: Nick Alcock <nick.alcock@xxxxxxxxxx>
---
include/linux/bpf.h | 1 +
include/linux/filter.h | 1 -
kernel/bpf/core.c | 5 ----
kernel/bpf/verifier.c | 2 +-
net/core/filter.c | 59 ++++++++++++++++++------------------------
5 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index fc3eda0192fb..9e255d5b1062 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -226,6 +226,7 @@ enum bpf_return_type {
struct bpf_func_proto {
u64 (*func)(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
bool gpl_only;
+ bool ctx_update;
bool pkt_access;
enum bpf_return_type ret_type;
enum bpf_arg_type arg1_type;
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 7148bab96943..9dacca7d3ef6 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -811,7 +811,6 @@ u64 __bpf_call_base(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);

struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog);
void bpf_jit_compile(struct bpf_prog *prog);
-bool bpf_helper_changes_pkt_data(void *func);

static inline bool bpf_dump_raw_ok(void)
{
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 225b1be766b0..8e9accf90c37 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2112,11 +2112,6 @@ void __weak bpf_jit_compile(struct bpf_prog *prog)
{
}

-bool __weak bpf_helper_changes_pkt_data(void *func)
-{
- return false;
-}
-
/* To execute LD_ABS/LD_IND instructions __bpf_prog_run() may call
* skb_copy_bits(), so provide a weak definition of it for NET-less config.
*/
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5fba4e6f5424..90ae04b4d5c7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3283,7 +3283,7 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
}

/* With LD_ABS/IND some JITs save/restore skb from r1. */
- changes_data = bpf_helper_changes_pkt_data(fn->func);
+ changes_data = fn->ctx_update;
if (changes_data && fn->arg1_type != ARG_PTR_TO_CTX) {
verbose(env, "kernel subsystem misconfigured func %s#%d: r1 != ctx\n",
func_id_name(func_id), func_id);
diff --git a/net/core/filter.c b/net/core/filter.c
index 55bfc941d17a..a9e7d3174d36 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1693,6 +1693,7 @@ BPF_CALL_5(bpf_skb_store_bytes, struct sk_buff *, skb, u32, offset,
static const struct bpf_func_proto bpf_skb_store_bytes_proto = {
.func = bpf_skb_store_bytes,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -1825,6 +1826,7 @@ BPF_CALL_2(bpf_skb_pull_data, struct sk_buff *, skb, u32, len)
static const struct bpf_func_proto bpf_skb_pull_data_proto = {
.func = bpf_skb_pull_data,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -1868,6 +1870,7 @@ BPF_CALL_2(sk_skb_pull_data, struct sk_buff *, skb, u32, len)
static const struct bpf_func_proto sk_skb_pull_data_proto = {
.func = sk_skb_pull_data,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -1909,6 +1912,7 @@ BPF_CALL_5(bpf_l3_csum_replace, struct sk_buff *, skb, u32, offset,
static const struct bpf_func_proto bpf_l3_csum_replace_proto = {
.func = bpf_l3_csum_replace,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -1962,6 +1966,7 @@ BPF_CALL_5(bpf_l4_csum_replace, struct sk_buff *, skb, u32, offset,
static const struct bpf_func_proto bpf_l4_csum_replace_proto = {
.func = bpf_l4_csum_replace,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -2145,6 +2150,7 @@ BPF_CALL_3(bpf_clone_redirect, struct sk_buff *, skb, u32, ifindex, u64, flags)
static const struct bpf_func_proto bpf_clone_redirect_proto = {
.func = bpf_clone_redirect,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -2337,6 +2343,7 @@ BPF_CALL_4(bpf_msg_pull_data, struct sk_msg *, msg, u32, start,
static const struct bpf_func_proto bpf_msg_pull_data_proto = {
.func = bpf_msg_pull_data,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -2468,6 +2475,7 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
static const struct bpf_func_proto bpf_msg_push_data_proto = {
.func = bpf_msg_push_data,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -2636,6 +2644,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
static const struct bpf_func_proto bpf_msg_pop_data_proto = {
.func = bpf_msg_pop_data,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -2738,6 +2747,7 @@ BPF_CALL_3(bpf_skb_vlan_push, struct sk_buff *, skb, __be16, vlan_proto,
static const struct bpf_func_proto bpf_skb_vlan_push_proto = {
.func = bpf_skb_vlan_push,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -2759,6 +2769,7 @@ BPF_CALL_1(bpf_skb_vlan_pop, struct sk_buff *, skb)
static const struct bpf_func_proto bpf_skb_vlan_pop_proto = {
.func = bpf_skb_vlan_pop,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
};
@@ -2962,6 +2973,7 @@ BPF_CALL_3(bpf_skb_change_proto, struct sk_buff *, skb, __be16, proto,
static const struct bpf_func_proto bpf_skb_change_proto_proto = {
.func = bpf_skb_change_proto,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -3198,6 +3210,7 @@ BPF_CALL_4(bpf_skb_adjust_room, struct sk_buff *, skb, s32, len_diff,
static const struct bpf_func_proto bpf_skb_adjust_room_proto = {
.func = bpf_skb_adjust_room,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -3285,6 +3298,7 @@ BPF_CALL_3(bpf_skb_change_tail, struct sk_buff *, skb, u32, new_len,
static const struct bpf_func_proto bpf_skb_change_tail_proto = {
.func = bpf_skb_change_tail,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -3303,6 +3317,7 @@ BPF_CALL_3(sk_skb_change_tail, struct sk_buff *, skb, u32, new_len,
static const struct bpf_func_proto sk_skb_change_tail_proto = {
.func = sk_skb_change_tail,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -3351,6 +3366,7 @@ BPF_CALL_3(bpf_skb_change_head, struct sk_buff *, skb, u32, head_room,
static const struct bpf_func_proto bpf_skb_change_head_proto = {
.func = bpf_skb_change_head,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -3369,6 +3385,7 @@ BPF_CALL_3(sk_skb_change_head, struct sk_buff *, skb, u32, head_room,
static const struct bpf_func_proto sk_skb_change_head_proto = {
.func = sk_skb_change_head,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -3403,6 +3420,7 @@ BPF_CALL_2(bpf_xdp_adjust_head, struct xdp_buff *, xdp, int, offset)
static const struct bpf_func_proto bpf_xdp_adjust_head_proto = {
.func = bpf_xdp_adjust_head,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -3427,6 +3445,7 @@ BPF_CALL_2(bpf_xdp_adjust_tail, struct xdp_buff *, xdp, int, offset)
static const struct bpf_func_proto bpf_xdp_adjust_tail_proto = {
.func = bpf_xdp_adjust_tail,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -3455,6 +3474,7 @@ BPF_CALL_2(bpf_xdp_adjust_meta, struct xdp_buff *, xdp, int, offset)
static const struct bpf_func_proto bpf_xdp_adjust_meta_proto = {
.func = bpf_xdp_adjust_meta,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -4987,6 +5007,7 @@ BPF_CALL_4(bpf_lwt_xmit_push_encap, struct sk_buff *, skb, u32, type,
static const struct bpf_func_proto bpf_lwt_in_push_encap_proto = {
.func = bpf_lwt_in_push_encap,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -4997,6 +5018,7 @@ static const struct bpf_func_proto bpf_lwt_in_push_encap_proto = {
static const struct bpf_func_proto bpf_lwt_xmit_push_encap_proto = {
.func = bpf_lwt_xmit_push_encap,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -5040,6 +5062,7 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, skb, u32, offset,
static const struct bpf_func_proto bpf_lwt_seg6_store_bytes_proto = {
.func = bpf_lwt_seg6_store_bytes,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -5128,6 +5151,7 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
static const struct bpf_func_proto bpf_lwt_seg6_action_proto = {
.func = bpf_lwt_seg6_action,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -5188,6 +5212,7 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
.func = bpf_lwt_seg6_adjust_srh,
.gpl_only = false,
+ .ctx_update = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
.arg2_type = ARG_ANYTHING,
@@ -5756,40 +5781,6 @@ static const struct bpf_func_proto bpf_tcp_check_syncookie_proto = {

#endif /* CONFIG_INET */

-bool bpf_helper_changes_pkt_data(void *func)
-{
- if (func == bpf_skb_vlan_push ||
- func == bpf_skb_vlan_pop ||
- func == bpf_skb_store_bytes ||
- func == bpf_skb_change_proto ||
- func == bpf_skb_change_head ||
- func == sk_skb_change_head ||
- func == bpf_skb_change_tail ||
- func == sk_skb_change_tail ||
- func == bpf_skb_adjust_room ||
- func == bpf_skb_pull_data ||
- func == sk_skb_pull_data ||
- func == bpf_clone_redirect ||
- func == bpf_l3_csum_replace ||
- func == bpf_l4_csum_replace ||
- func == bpf_xdp_adjust_head ||
- func == bpf_xdp_adjust_meta ||
- func == bpf_msg_pull_data ||
- func == bpf_msg_push_data ||
- func == bpf_msg_pop_data ||
- func == bpf_xdp_adjust_tail ||
-#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
- func == bpf_lwt_seg6_store_bytes ||
- func == bpf_lwt_seg6_adjust_srh ||
- func == bpf_lwt_seg6_action ||
-#endif
- func == bpf_lwt_in_push_encap ||
- func == bpf_lwt_xmit_push_encap)
- return true;
-
- return false;
-}
-
static const struct bpf_func_proto *
bpf_base_func_proto(enum bpf_func_id func_id)
{
--
2.20.1