* [PATCH net-next v6 0/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb
@ 2021-06-15 0:10 Tanner Love
2021-06-15 0:10 ` [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr Tanner Love
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Tanner Love @ 2021-06-15 0:10 UTC (permalink / raw)
To: netdev
Cc: davem, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eric Dumazet, Willem de Bruijn, Petar Penkov, Jakub Kicinski,
Michael S . Tsirkin, Jason Wang, Martin KaFai Lau, Tanner Love
From: Tanner Love <tannerlove@google.com>
First patch extends the flow dissector BPF program type to accept
pointer to virtio-net header.
Second patch uses this feature to add optional flow dissection in
virtio_net_hdr_to_skb(). This allows admins to define permitted
packets more strictly, for example dropping deprecated UDP_UFO
packets.
Third patch extends kselftest to cover this feature.
Tanner Love (3):
net: flow_dissector: extend bpf flow dissector support with vnet hdr
virtio_net: add optional flow dissection in virtio_net_hdr_to_skb
selftests/net: amend bpf flow dissector prog to do vnet hdr validation
drivers/net/bonding/bond_main.c | 2 +-
include/linux/bpf.h | 3 +
include/linux/skbuff.h | 35 ++-
include/linux/virtio_net.h | 25 ++-
include/uapi/linux/bpf.h | 2 +
kernel/bpf/verifier.c | 35 +--
net/bpf/test_run.c | 2 +-
net/core/filter.c | 56 +++++
net/core/flow_dissector.c | 21 +-
net/core/sysctl_net_core.c | 9 +
tools/include/uapi/linux/bpf.h | 2 +
tools/testing/selftests/bpf/progs/bpf_flow.c | 209 ++++++++++++++----
.../selftests/bpf/test_flow_dissector.c | 181 +++++++++++++--
.../selftests/bpf/test_flow_dissector.sh | 19 ++
14 files changed, 502 insertions(+), 99 deletions(-)
--
2.32.0.272.g935e593368-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
2021-06-15 0:10 [PATCH net-next v6 0/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
@ 2021-06-15 0:10 ` Tanner Love
2021-06-15 22:25 ` Martin KaFai Lau
` (2 more replies)
2021-06-15 0:10 ` [PATCH net-next v6 2/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
2021-06-15 0:11 ` [PATCH net-next v6 3/3] selftests/net: amend bpf flow dissector prog to do vnet hdr validation Tanner Love
2 siblings, 3 replies; 11+ messages in thread
From: Tanner Love @ 2021-06-15 0:10 UTC (permalink / raw)
To: netdev
Cc: davem, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eric Dumazet, Willem de Bruijn, Petar Penkov, Jakub Kicinski,
Michael S . Tsirkin, Jason Wang, Martin KaFai Lau, Tanner Love,
Stanislav Fomichev
From: Tanner Love <tannerlove@google.com>
Amend the bpf flow dissector program type to be able to process
virtio-net headers. Do this to enable bpf flow dissector programs to
perform virtio-net header validation. The next patch in this series
will add a flow dissection hook in virtio_net_hdr_to_skb and make use
of this extended functionality. That commit message has more
background on the use case.
Add two new members to struct bpf_flow_keys: a pointer to struct
virtio_net_hdr, and vhdr_is_little_endian. The latter is required to
inform the BPF program of the endianness of the virtio-net header
fields, to handle the case of a version 1+ header on a big endian
machine.
Changes
v6:
- Move bpf_flow_dissector_btf_ids, check_flow_keys_access() to
filter.c
- Verify (off % size == 0) in check_flow_keys_access()
- Check bpf_flow_dissector_btf_ids[0] is nonzero in
check_flow_keys_access()
v5:
- Use PTR_TO_BTF_ID_OR_NULL instead of defining new
PTR_TO_VNET_HDR_OR_NULL
- Make check_flow_keys_access() disallow writes to keys->vhdr
- Make check_flow_keys_access() check loading keys->vhdr is in
sizeof(__u64)
- Use BPF_REG_AX instead of BPF_REG_TMP as scratch reg
- Describe parameter vhdr_is_little_endian in __skb_flow_dissect
documentation
v4:
- Add virtio_net_hdr pointer to struct bpf_flow_keys
- Add vhdr_is_little_endian to struct bpf_flow_keys
v2:
- Describe parameter vhdr in __skb_flow_dissect documentation
Signed-off-by: Tanner Love <tannerlove@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Reviewed-by: Petar Penkov <ppenkov@google.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
drivers/net/bonding/bond_main.c | 2 +-
include/linux/bpf.h | 3 ++
include/linux/skbuff.h | 35 ++++++++++++++++-----
include/uapi/linux/bpf.h | 2 ++
kernel/bpf/verifier.c | 35 ++++++++++++---------
net/bpf/test_run.c | 2 +-
net/core/filter.c | 56 +++++++++++++++++++++++++++++++++
net/core/flow_dissector.c | 18 ++++++++---
tools/include/uapi/linux/bpf.h | 2 ++
9 files changed, 127 insertions(+), 28 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index eb79a9f05914..36993636d56d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3554,7 +3554,7 @@ static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
case BOND_XMIT_POLICY_ENCAP34:
memset(fk, 0, sizeof(*fk));
return __skb_flow_dissect(NULL, skb, &flow_keys_bonding,
- fk, NULL, 0, 0, 0, 0);
+ fk, NULL, 0, 0, 0, 0, NULL, false);
default:
break;
}
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 9dc44ba97584..f08dee59b099 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1515,6 +1515,9 @@ static inline int bpf_map_attr_numa_node(const union bpf_attr *attr)
attr->numa_node : NUMA_NO_NODE;
}
+int check_flow_keys_access(int off, int size, enum bpf_access_type t,
+ struct bpf_insn_access_aux *info);
+
struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type type);
int array_map_alloc_check(union bpf_attr *attr);
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b2db9cd9a73f..4e390cd8f72a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1314,21 +1314,27 @@ void skb_flow_dissector_init(struct flow_dissector *flow_dissector,
unsigned int key_count);
struct bpf_flow_dissector;
+struct virtio_net_hdr;
bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
- __be16 proto, int nhoff, int hlen, unsigned int flags);
+ __be16 proto, int nhoff, int hlen, unsigned int flags,
+ const struct virtio_net_hdr *vhdr,
+ bool vhdr_is_little_endian);
bool __skb_flow_dissect(const struct net *net,
const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
void *target_container, const void *data,
- __be16 proto, int nhoff, int hlen, unsigned int flags);
+ __be16 proto, int nhoff, int hlen, unsigned int flags,
+ const struct virtio_net_hdr *vhdr,
+ bool vhdr_is_little_endian);
static inline bool skb_flow_dissect(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
void *target_container, unsigned int flags)
{
return __skb_flow_dissect(NULL, skb, flow_dissector,
- target_container, NULL, 0, 0, 0, flags);
+ target_container, NULL, 0, 0, 0, flags, NULL,
+ false);
}
static inline bool skb_flow_dissect_flow_keys(const struct sk_buff *skb,
@@ -1337,7 +1343,22 @@ static inline bool skb_flow_dissect_flow_keys(const struct sk_buff *skb,
{
memset(flow, 0, sizeof(*flow));
return __skb_flow_dissect(NULL, skb, &flow_keys_dissector,
- flow, NULL, 0, 0, 0, flags);
+ flow, NULL, 0, 0, 0, flags, NULL, false);
+}
+
+static inline bool
+__skb_flow_dissect_flow_keys_basic(const struct net *net,
+ const struct sk_buff *skb,
+ struct flow_keys_basic *flow,
+ const void *data, __be16 proto,
+ int nhoff, int hlen, unsigned int flags,
+ const struct virtio_net_hdr *vhdr,
+ bool vhdr_is_little_endian)
+{
+ memset(flow, 0, sizeof(*flow));
+ return __skb_flow_dissect(net, skb, &flow_keys_basic_dissector, flow,
+ data, proto, nhoff, hlen, flags, vhdr,
+ vhdr_is_little_endian);
}
static inline bool
@@ -1347,9 +1368,9 @@ skb_flow_dissect_flow_keys_basic(const struct net *net,
const void *data, __be16 proto,
int nhoff, int hlen, unsigned int flags)
{
- memset(flow, 0, sizeof(*flow));
- return __skb_flow_dissect(net, skb, &flow_keys_basic_dissector, flow,
- data, proto, nhoff, hlen, flags);
+ return __skb_flow_dissect_flow_keys_basic(net, skb, flow, data, proto,
+ nhoff, hlen, flags, NULL,
+ false);
}
void skb_flow_dissect_meta(const struct sk_buff *skb,
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 418b9b813d65..e1ac34548f9a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6017,6 +6017,8 @@ struct bpf_flow_keys {
};
__u32 flags;
__be32 flow_label;
+ __bpf_md_ptr(const struct virtio_net_hdr *, vhdr);
+ __u8 vhdr_is_little_endian;
};
struct bpf_func_info {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 331b170d9fcc..a037476954f5 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -22,6 +22,7 @@
#include <linux/error-injection.h>
#include <linux/bpf_lsm.h>
#include <linux/btf_ids.h>
+#include <linux/virtio_net.h>
#include "disasm.h"
@@ -3372,18 +3373,6 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
return -EACCES;
}
-static int check_flow_keys_access(struct bpf_verifier_env *env, int off,
- int size)
-{
- if (size < 0 || off < 0 ||
- (u64)off + size > sizeof(struct bpf_flow_keys)) {
- verbose(env, "invalid access to flow keys off=%d size=%d\n",
- off, size);
- return -EACCES;
- }
- return 0;
-}
-
static int check_sock_access(struct bpf_verifier_env *env, int insn_idx,
u32 regno, int off, int size,
enum bpf_access_type t)
@@ -4210,6 +4199,8 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown(env, regs, value_regno);
} else if (reg->type == PTR_TO_FLOW_KEYS) {
+ struct bpf_insn_access_aux info = {};
+
if (t == BPF_WRITE && value_regno >= 0 &&
is_pointer_value(env, value_regno)) {
verbose(env, "R%d leaks addr into flow keys\n",
@@ -4217,9 +4208,23 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
return -EACCES;
}
- err = check_flow_keys_access(env, off, size);
- if (!err && t == BPF_READ && value_regno >= 0)
- mark_reg_unknown(env, regs, value_regno);
+ err = check_flow_keys_access(off, size, t, &info);
+ if (err) {
+ verbose(env,
+ "invalid access to flow keys off=%d size=%d\n",
+ off, size);
+ } else if (t == BPF_READ && value_regno >= 0) {
+ if (off == offsetof(struct bpf_flow_keys, vhdr)) {
+ mark_reg_known_zero(env, regs, value_regno);
+ regs[value_regno].type = PTR_TO_BTF_ID_OR_NULL;
+ regs[value_regno].btf = btf_vmlinux;
+ regs[value_regno].btf_id = info.btf_id;
+ /* required for dropping or_null */
+ regs[value_regno].id = ++env->id_gen;
+ } else {
+ mark_reg_unknown(env, regs, value_regno);
+ }
+ }
} else if (type_is_sk_pointer(reg->type)) {
if (t == BPF_WRITE) {
verbose(env, "R%d cannot write into %s\n",
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index aa47af349ba8..a11c5ce99ccb 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -797,7 +797,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
bpf_test_timer_enter(&t);
do {
retval = bpf_flow_dissect(prog, &ctx, eth->h_proto, ETH_HLEN,
- size, flags);
+ size, flags, NULL, false);
} while (bpf_test_timer_continue(&t, repeat, &ret, &duration));
bpf_test_timer_leave(&t);
diff --git a/net/core/filter.c b/net/core/filter.c
index 239de1306de9..f5be14b947cd 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -8329,6 +8329,36 @@ static bool sk_msg_is_valid_access(int off, int size,
return true;
}
+BTF_ID_LIST_SINGLE(bpf_flow_dissector_btf_ids, struct, virtio_net_hdr);
+
+int check_flow_keys_access(int off, int size, enum bpf_access_type t,
+ struct bpf_insn_access_aux *info)
+{
+ if (size < 0 || off < 0 ||
+ (u64)off + size > sizeof(struct bpf_flow_keys))
+ return -EACCES;
+
+ switch (off) {
+ case bpf_ctx_range_ptr(struct bpf_flow_keys, vhdr):
+ if (t == BPF_WRITE || off % size != 0 || size != sizeof(__u64))
+ return -EACCES;
+
+ if (!bpf_flow_dissector_btf_ids[0])
+ return -EINVAL;
+
+ info->btf_id = bpf_flow_dissector_btf_ids[0];
+
+ break;
+ case offsetof(struct bpf_flow_keys, vhdr_is_little_endian):
+ if (t == BPF_WRITE)
+ return -EACCES;
+
+ break;
+ }
+
+ return 0;
+}
+
static bool flow_dissector_is_valid_access(int off, int size,
enum bpf_access_type type,
const struct bpf_prog *prog,
@@ -8358,6 +8388,8 @@ static bool flow_dissector_is_valid_access(int off, int size,
return false;
info->reg_type = PTR_TO_FLOW_KEYS;
return true;
+ case bpf_ctx_range(struct __sk_buff, len):
+ return size == size_default;
default:
return false;
}
@@ -8390,6 +8422,30 @@ static u32 flow_dissector_convert_ctx_access(enum bpf_access_type type,
si->dst_reg, si->src_reg,
offsetof(struct bpf_flow_dissector, flow_keys));
break;
+
+ case offsetof(struct __sk_buff, len):
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct bpf_flow_dissector, skb),
+ si->dst_reg, si->src_reg,
+ offsetof(struct bpf_flow_dissector, skb));
+ *insn++ = BPF_JMP_IMM(BPF_JNE, si->dst_reg, 0, 4);
+ /* bpf_flow_dissector->skb == NULL */
+ /* dst_reg = bpf_flow_dissector->data_end */
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct bpf_flow_dissector, data_end),
+ si->dst_reg, si->src_reg,
+ offsetof(struct bpf_flow_dissector, data_end));
+ /* AX = bpf_flow_dissector->data */
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct bpf_flow_dissector, data),
+ BPF_REG_AX, si->src_reg,
+ offsetof(struct bpf_flow_dissector, data));
+ /* dst_reg -= bpf_flow_dissector->data */
+ *insn++ = BPF_ALU64_REG(BPF_SUB, si->dst_reg, BPF_REG_AX);
+ *insn++ = BPF_JMP_A(1);
+ /* bpf_flow_dissector->skb != NULL */
+ /* bpf_flow_dissector->skb->len */
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct sk_buff, len),
+ si->dst_reg, si->dst_reg,
+ offsetof(struct sk_buff, len));
+ break;
}
return insn - insn_buf;
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 2aadbfc5193b..afbfef4402f5 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -28,6 +28,7 @@
#include <scsi/fc/fc_fcoe.h>
#include <uapi/linux/batadv_packet.h>
#include <linux/bpf.h>
+#include <linux/virtio_net.h>
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
#include <net/netfilter/nf_conntrack_core.h>
#include <net/netfilter/nf_conntrack_labels.h>
@@ -864,7 +865,9 @@ static void __skb_flow_bpf_to_target(const struct bpf_flow_keys *flow_keys,
}
bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
- __be16 proto, int nhoff, int hlen, unsigned int flags)
+ __be16 proto, int nhoff, int hlen, unsigned int flags,
+ const struct virtio_net_hdr *vhdr,
+ bool vhdr_is_little_endian)
{
struct bpf_flow_keys *flow_keys = ctx->flow_keys;
u32 result;
@@ -874,6 +877,8 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
flow_keys->n_proto = proto;
flow_keys->nhoff = nhoff;
flow_keys->thoff = flow_keys->nhoff;
+ flow_keys->vhdr = vhdr;
+ flow_keys->vhdr_is_little_endian = vhdr_is_little_endian;
BUILD_BUG_ON((int)BPF_FLOW_DISSECTOR_F_PARSE_1ST_FRAG !=
(int)FLOW_DISSECTOR_F_PARSE_1ST_FRAG);
@@ -904,6 +909,8 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
* @hlen: packet header length, if @data is NULL use skb_headlen(skb)
* @flags: flags that control the dissection process, e.g.
* FLOW_DISSECTOR_F_STOP_AT_ENCAP.
+ * @vhdr: virtio_net_header to include in kernel context for BPF flow dissector
+ * @vhdr_is_little_endian: whether virtio_net_hdr fields are little endian
*
* The function will try to retrieve individual keys into target specified
* by flow_dissector from either the skbuff or a raw buffer specified by the
@@ -915,7 +922,9 @@ bool __skb_flow_dissect(const struct net *net,
const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
void *target_container, const void *data,
- __be16 proto, int nhoff, int hlen, unsigned int flags)
+ __be16 proto, int nhoff, int hlen, unsigned int flags,
+ const struct virtio_net_hdr *vhdr,
+ bool vhdr_is_little_endian)
{
struct flow_dissector_key_control *key_control;
struct flow_dissector_key_basic *key_basic;
@@ -1012,7 +1021,8 @@ bool __skb_flow_dissect(const struct net *net,
prog = READ_ONCE(run_array->items[0].prog);
ret = bpf_flow_dissect(prog, &ctx, n_proto, nhoff,
- hlen, flags);
+ hlen, flags, vhdr,
+ vhdr_is_little_endian);
__skb_flow_bpf_to_target(&flow_keys, flow_dissector,
target_container);
rcu_read_unlock();
@@ -1610,7 +1620,7 @@ u32 __skb_get_hash_symmetric(const struct sk_buff *skb)
memset(&keys, 0, sizeof(keys));
__skb_flow_dissect(NULL, skb, &flow_keys_dissector_symmetric,
&keys, NULL, 0, 0, 0,
- FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
+ FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL, NULL, false);
return __flow_hash_from_keys(&keys, &hashrnd);
}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 418b9b813d65..e1ac34548f9a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6017,6 +6017,8 @@ struct bpf_flow_keys {
};
__u32 flags;
__be32 flow_label;
+ __bpf_md_ptr(const struct virtio_net_hdr *, vhdr);
+ __u8 vhdr_is_little_endian;
};
struct bpf_func_info {
--
2.32.0.272.g935e593368-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next v6 2/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb
2021-06-15 0:10 [PATCH net-next v6 0/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
2021-06-15 0:10 ` [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr Tanner Love
@ 2021-06-15 0:10 ` Tanner Love
2021-06-15 0:11 ` [PATCH net-next v6 3/3] selftests/net: amend bpf flow dissector prog to do vnet hdr validation Tanner Love
2 siblings, 0 replies; 11+ messages in thread
From: Tanner Love @ 2021-06-15 0:10 UTC (permalink / raw)
To: netdev
Cc: davem, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eric Dumazet, Willem de Bruijn, Petar Penkov, Jakub Kicinski,
Michael S . Tsirkin, Jason Wang, Martin KaFai Lau, Tanner Love
From: Tanner Love <tannerlove@google.com>
Syzkaller bugs have resulted from loose specification of
virtio_net_hdr[1]. Enable execution of a BPF flow dissector program
in virtio_net_hdr_to_skb to validate the vnet header and drop bad
input.
Introduce a new sysctl net.core.flow_dissect_vnet_hdr controlling a
static key to decide whether to perform flow dissection. When the key
is false, virtio_net_hdr_to_skb computes as before.
A permissive specification of vnet headers is part of the ABI. Some
applications now depend on it. Still, many of these packets are bogus.
Give admins the option to interpret behavior more strictly. For
instance, verifying that a VIRTIO_NET_HDR_GSO_TCPV6 header matches a
packet with unencapsulated IPv6/TCP without extension headers, with
payload length exceeding gso_size and hdr_len exactly at TCP payload
offset.
BPF flow dissection implements protocol parsing in an safe way. And is
configurable, so can be as pedantic as the workload allows (e.g.,
dropping UFO altogether).
Vnet_header flow dissection is *not* a substitute for fixing bugs when
reported. But even if not enabled continuously, offers a quick path to
mitigating vulnerabilities.
[1] https://syzkaller.appspot.com/bug?id=b419a5ca95062664fe1a60b764621eb4526e2cd0
Changes
v4:
- Expand commit message with rationale for bpf flow dissector based
implementation
v3:
- Move sysctl_flow_dissect_vnet_hdr_key definition to
flow_dissector.c to fix CONFIG_SYSCTL warning when building UML
Suggested-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Tanner Love <tannerlove@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
include/linux/virtio_net.h | 25 +++++++++++++++++++++----
net/core/flow_dissector.c | 3 +++
net/core/sysctl_net_core.c | 9 +++++++++
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index b465f8f3e554..b67b5413f2ce 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -25,10 +25,13 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
return 0;
}
+DECLARE_STATIC_KEY_FALSE(sysctl_flow_dissect_vnet_hdr_key);
+
static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
const struct virtio_net_hdr *hdr,
bool little_endian)
{
+ struct flow_keys_basic keys;
unsigned int gso_type = 0;
unsigned int thlen = 0;
unsigned int p_off = 0;
@@ -78,13 +81,24 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
p_off = skb_transport_offset(skb) + thlen;
if (!pskb_may_pull(skb, p_off))
return -EINVAL;
- } else {
+ }
+
+ /* BPF flow dissection for optional strict validation.
+ *
+ * Admins can define permitted packets more strictly, such as dropping
+ * deprecated UDP_UFO packets and requiring skb->protocol to be non-zero
+ * and matching packet headers.
+ */
+ if (static_branch_unlikely(&sysctl_flow_dissect_vnet_hdr_key) &&
+ !__skb_flow_dissect_flow_keys_basic(NULL, skb, &keys, NULL, 0, 0, 0,
+ 0, hdr, little_endian))
+ return -EINVAL;
+
+ if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) {
/* gso packets without NEEDS_CSUM do not set transport_offset.
* probe and drop if does not match one of the above types.
*/
if (gso_type && skb->network_header) {
- struct flow_keys_basic keys;
-
if (!skb->protocol) {
__be16 protocol = dev_parse_header_protocol(skb);
@@ -92,8 +106,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
if (protocol && protocol != skb->protocol)
return -EINVAL;
}
+
retry:
- if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
+ /* only if flow dissection not already done */
+ if (!static_branch_unlikely(&sysctl_flow_dissect_vnet_hdr_key) &&
+ !skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
NULL, 0, 0, 0,
0)) {
/* UFO does not specify ipv4 or 6: try both */
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index afbfef4402f5..2422166c2d8c 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -35,6 +35,9 @@
#endif
#include <linux/bpf-netns.h>
+DEFINE_STATIC_KEY_FALSE(sysctl_flow_dissect_vnet_hdr_key);
+EXPORT_SYMBOL(sysctl_flow_dissect_vnet_hdr_key);
+
static void dissector_set_key(struct flow_dissector *flow_dissector,
enum flow_dissector_key_id key_id)
{
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index c8496c1142c9..c01b9366bb75 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -36,6 +36,8 @@ static int net_msg_warn; /* Unused, but still a sysctl */
int sysctl_fb_tunnels_only_for_init_net __read_mostly = 0;
EXPORT_SYMBOL(sysctl_fb_tunnels_only_for_init_net);
+DECLARE_STATIC_KEY_FALSE(sysctl_flow_dissect_vnet_hdr_key);
+
/* 0 - Keep current behavior:
* IPv4: inherit all current settings from init_net
* IPv6: reset all settings to default
@@ -580,6 +582,13 @@ static struct ctl_table net_core_table[] = {
.extra1 = SYSCTL_ONE,
.extra2 = &int_3600,
},
+ {
+ .procname = "flow_dissect_vnet_hdr",
+ .data = &sysctl_flow_dissect_vnet_hdr_key.key,
+ .maxlen = sizeof(sysctl_flow_dissect_vnet_hdr_key),
+ .mode = 0644,
+ .proc_handler = proc_do_static_key,
+ },
{ }
};
--
2.32.0.272.g935e593368-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next v6 3/3] selftests/net: amend bpf flow dissector prog to do vnet hdr validation
2021-06-15 0:10 [PATCH net-next v6 0/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
2021-06-15 0:10 ` [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr Tanner Love
2021-06-15 0:10 ` [PATCH net-next v6 2/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
@ 2021-06-15 0:11 ` Tanner Love
2 siblings, 0 replies; 11+ messages in thread
From: Tanner Love @ 2021-06-15 0:11 UTC (permalink / raw)
To: netdev
Cc: davem, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eric Dumazet, Willem de Bruijn, Petar Penkov, Jakub Kicinski,
Michael S . Tsirkin, Jason Wang, Martin KaFai Lau, Tanner Love
From: Tanner Love <tannerlove@google.com>
Change the BPF flow dissector program to perform various checks on the
virtio_net_hdr fields after doing flow dissection.
Amend test_flow_dissector.(c|sh) to add test cases that inject packets
with reasonable or unreasonable virtio-net headers and assert that bad
packets are dropped and good packets are not. Do this via packet
socket; the kernel executes tpacket_snd, which enters
virtio_net_hdr_to_skb, where flow dissection / vnet header validation
occurs.
Changes
v4:
- Read virtio_net_hdr pointer from struct bpf_flow_keys
- Add vnet header endianness logic to BPF program
Signed-off-by: Tanner Love <tannerlove@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
tools/testing/selftests/bpf/progs/bpf_flow.c | 209 ++++++++++++++----
.../selftests/bpf/test_flow_dissector.c | 181 +++++++++++++--
.../selftests/bpf/test_flow_dissector.sh | 19 ++
3 files changed, 342 insertions(+), 67 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/bpf_flow.c b/tools/testing/selftests/bpf/progs/bpf_flow.c
index 95a5a0778ed7..a846ff7e6331 100644
--- a/tools/testing/selftests/bpf/progs/bpf_flow.c
+++ b/tools/testing/selftests/bpf/progs/bpf_flow.c
@@ -13,6 +13,7 @@
#include <linux/tcp.h>
#include <linux/udp.h>
#include <linux/if_packet.h>
+#include <linux/virtio_net.h>
#include <sys/socket.h>
#include <linux/if_tunnel.h>
#include <linux/mpls.h>
@@ -71,15 +72,140 @@ struct {
__type(value, struct bpf_flow_keys);
} last_dissection SEC(".maps");
-static __always_inline int export_flow_keys(struct bpf_flow_keys *keys,
- int ret)
+static __always_inline __u16 __virtio16_to_cpu(bool little_endian,
+ __virtio16 val)
{
+ if (little_endian)
+ return __le16_to_cpu((__le16)val);
+ else
+ return __be16_to_cpu((__be16)val);
+}
+
+/* Drops invalid virtio-net headers */
+static __always_inline int validate_vnet_hdr(const struct bpf_flow_keys *keys,
+ __u32 skb_len)
+{
+ __u16 gso_type, hdr_len, gso_size, csum_start, csum_offset;
+ const struct virtio_net_hdr *vhdr = keys->vhdr;
+
+ if (!vhdr)
+ return BPF_OK;
+
+ gso_type = __virtio16_to_cpu(keys->vhdr_is_little_endian,
+ vhdr->gso_type);
+ hdr_len = __virtio16_to_cpu(keys->vhdr_is_little_endian,
+ vhdr->hdr_len);
+ gso_size = __virtio16_to_cpu(keys->vhdr_is_little_endian,
+ vhdr->gso_size);
+ csum_start = __virtio16_to_cpu(keys->vhdr_is_little_endian,
+ vhdr->csum_start);
+ csum_offset = __virtio16_to_cpu(keys->vhdr_is_little_endian,
+ vhdr->csum_offset);
+
+ /* Check gso */
+ if (gso_type != VIRTIO_NET_HDR_GSO_NONE) {
+ if (!(vhdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
+ return BPF_DROP;
+
+ if (keys->is_encap)
+ return BPF_DROP;
+
+ switch (gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
+ case VIRTIO_NET_HDR_GSO_TCPV4:
+ if (keys->addr_proto != ETH_P_IP ||
+ keys->ip_proto != IPPROTO_TCP)
+ return BPF_DROP;
+
+ if (gso_size >= skb_len - keys->thoff -
+ sizeof(struct tcphdr))
+ return BPF_DROP;
+
+ break;
+ case VIRTIO_NET_HDR_GSO_TCPV6:
+ if (keys->addr_proto != ETH_P_IPV6 ||
+ keys->ip_proto != IPPROTO_TCP)
+ return BPF_DROP;
+
+ if (gso_size >= skb_len - keys->thoff -
+ sizeof(struct tcphdr))
+ return BPF_DROP;
+
+ break;
+ case VIRTIO_NET_HDR_GSO_UDP:
+ if (keys->ip_proto != IPPROTO_UDP)
+ return BPF_DROP;
+
+ if (gso_size >= skb_len - keys->thoff -
+ sizeof(struct udphdr))
+ return BPF_DROP;
+
+ break;
+ default:
+ return BPF_DROP;
+ }
+ }
+
+ /* Check hdr_len */
+ if (hdr_len) {
+ switch (keys->ip_proto) {
+ case IPPROTO_TCP:
+ if (hdr_len != keys->thoff + sizeof(struct tcphdr))
+ return BPF_DROP;
+
+ break;
+ case IPPROTO_UDP:
+ if (hdr_len != keys->thoff + sizeof(struct udphdr))
+ return BPF_DROP;
+
+ break;
+ }
+ }
+
+ /* Check csum */
+ if (vhdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+ if (keys->addr_proto != ETH_P_IP &&
+ keys->addr_proto != ETH_P_IPV6)
+ return BPF_DROP;
+
+ if (csum_start != keys->thoff)
+ return BPF_DROP;
+
+ switch (keys->ip_proto) {
+ case IPPROTO_TCP:
+ if (csum_offset != offsetof(struct tcphdr, check))
+ return BPF_DROP;
+
+ break;
+ case IPPROTO_UDP:
+ if (csum_offset != offsetof(struct udphdr, check))
+ return BPF_DROP;
+
+ break;
+ default:
+ return BPF_DROP;
+ }
+ }
+
+ return BPF_OK;
+}
+
+/* Common steps to perform regardless of where protocol parsing finishes:
+ * 1. store flow keys in map
+ * 2. if parse result is BPF_OK, parse the vnet hdr if present
+ * 3. return the parse result
+ */
+static __always_inline int parse_epilogue(struct __sk_buff *skb, int ret)
+{
+ const struct bpf_flow_keys *keys = skb->flow_keys;
__u32 key = (__u32)(keys->sport) << 16 | keys->dport;
struct bpf_flow_keys val;
memcpy(&val, keys, sizeof(val));
bpf_map_update_elem(&last_dissection, &key, &val, BPF_ANY);
- return ret;
+
+ if (ret != BPF_OK)
+ return ret;
+ return validate_vnet_hdr(keys, skb->len);
}
#define IPV6_FLOWLABEL_MASK __bpf_constant_htonl(0x000FFFFF)
@@ -114,8 +240,6 @@ static __always_inline void *bpf_flow_dissect_get_header(struct __sk_buff *skb,
/* Dispatches on ETHERTYPE */
static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
{
- struct bpf_flow_keys *keys = skb->flow_keys;
-
switch (proto) {
case bpf_htons(ETH_P_IP):
bpf_tail_call_static(skb, &jmp_table, IP);
@@ -131,12 +255,10 @@ static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
case bpf_htons(ETH_P_8021AD):
bpf_tail_call_static(skb, &jmp_table, VLAN);
break;
- default:
- /* Protocol not supported */
- return export_flow_keys(keys, BPF_DROP);
}
- return export_flow_keys(keys, BPF_DROP);
+ /* Protocol not supported */
+ return parse_epilogue(skb, BPF_DROP);
}
SEC("flow_dissector")
@@ -162,28 +284,28 @@ static __always_inline int parse_ip_proto(struct __sk_buff *skb, __u8 proto)
case IPPROTO_ICMP:
icmp = bpf_flow_dissect_get_header(skb, sizeof(*icmp), &_icmp);
if (!icmp)
- return export_flow_keys(keys, BPF_DROP);
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_DROP);
+ return parse_epilogue(skb, BPF_OK);
case IPPROTO_IPIP:
keys->is_encap = true;
if (keys->flags & BPF_FLOW_DISSECTOR_F_STOP_AT_ENCAP)
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
return parse_eth_proto(skb, bpf_htons(ETH_P_IP));
case IPPROTO_IPV6:
keys->is_encap = true;
if (keys->flags & BPF_FLOW_DISSECTOR_F_STOP_AT_ENCAP)
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
return parse_eth_proto(skb, bpf_htons(ETH_P_IPV6));
case IPPROTO_GRE:
gre = bpf_flow_dissect_get_header(skb, sizeof(*gre), &_gre);
if (!gre)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
if (bpf_htons(gre->flags & GRE_VERSION))
/* Only inspect standard GRE packets with version 0 */
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
keys->thoff += sizeof(*gre); /* Step over GRE Flags and Proto */
if (GRE_IS_CSUM(gre->flags))
@@ -195,13 +317,13 @@ static __always_inline int parse_ip_proto(struct __sk_buff *skb, __u8 proto)
keys->is_encap = true;
if (keys->flags & BPF_FLOW_DISSECTOR_F_STOP_AT_ENCAP)
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
if (gre->proto == bpf_htons(ETH_P_TEB)) {
eth = bpf_flow_dissect_get_header(skb, sizeof(*eth),
&_eth);
if (!eth)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->thoff += sizeof(*eth);
@@ -212,37 +334,35 @@ static __always_inline int parse_ip_proto(struct __sk_buff *skb, __u8 proto)
case IPPROTO_TCP:
tcp = bpf_flow_dissect_get_header(skb, sizeof(*tcp), &_tcp);
if (!tcp)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
if (tcp->doff < 5)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
if ((__u8 *)tcp + (tcp->doff << 2) > data_end)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->sport = tcp->source;
keys->dport = tcp->dest;
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
case IPPROTO_UDP:
case IPPROTO_UDPLITE:
udp = bpf_flow_dissect_get_header(skb, sizeof(*udp), &_udp);
if (!udp)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->sport = udp->source;
keys->dport = udp->dest;
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
default:
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
}
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
}
static __always_inline int parse_ipv6_proto(struct __sk_buff *skb, __u8 nexthdr)
{
- struct bpf_flow_keys *keys = skb->flow_keys;
-
switch (nexthdr) {
case IPPROTO_HOPOPTS:
case IPPROTO_DSTOPTS:
@@ -255,7 +375,7 @@ static __always_inline int parse_ipv6_proto(struct __sk_buff *skb, __u8 nexthdr)
return parse_ip_proto(skb, nexthdr);
}
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
}
PROG(IP)(struct __sk_buff *skb)
@@ -268,11 +388,11 @@ PROG(IP)(struct __sk_buff *skb)
iph = bpf_flow_dissect_get_header(skb, sizeof(*iph), &_iph);
if (!iph)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
/* IP header cannot be smaller than 20 bytes */
if (iph->ihl < 5)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->addr_proto = ETH_P_IP;
keys->ipv4_src = iph->saddr;
@@ -281,7 +401,7 @@ PROG(IP)(struct __sk_buff *skb)
keys->thoff += iph->ihl << 2;
if (data + keys->thoff > data_end)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
if (iph->frag_off & bpf_htons(IP_MF | IP_OFFSET)) {
keys->is_frag = true;
@@ -302,7 +422,7 @@ PROG(IP)(struct __sk_buff *skb)
}
if (done)
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
return parse_ip_proto(skb, iph->protocol);
}
@@ -314,7 +434,7 @@ PROG(IPV6)(struct __sk_buff *skb)
ip6h = bpf_flow_dissect_get_header(skb, sizeof(*ip6h), &_ip6h);
if (!ip6h)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->addr_proto = ETH_P_IPV6;
memcpy(&keys->ipv6_src, &ip6h->saddr, 2*sizeof(ip6h->saddr));
@@ -324,7 +444,7 @@ PROG(IPV6)(struct __sk_buff *skb)
keys->flow_label = ip6_flowlabel(ip6h);
if (keys->flags & BPF_FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL)
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
return parse_ipv6_proto(skb, ip6h->nexthdr);
}
@@ -336,7 +456,7 @@ PROG(IPV6OP)(struct __sk_buff *skb)
ip6h = bpf_flow_dissect_get_header(skb, sizeof(*ip6h), &_ip6h);
if (!ip6h)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
/* hlen is in 8-octets and does not include the first 8 bytes
* of the header
@@ -354,7 +474,7 @@ PROG(IPV6FR)(struct __sk_buff *skb)
fragh = bpf_flow_dissect_get_header(skb, sizeof(*fragh), &_fragh);
if (!fragh)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->thoff += sizeof(*fragh);
keys->is_frag = true;
@@ -367,9 +487,9 @@ PROG(IPV6FR)(struct __sk_buff *skb)
* explicitly asked for.
*/
if (!(keys->flags & BPF_FLOW_DISSECTOR_F_PARSE_1ST_FRAG))
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
} else {
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
}
return parse_ipv6_proto(skb, fragh->nexthdr);
@@ -377,14 +497,13 @@ PROG(IPV6FR)(struct __sk_buff *skb)
PROG(MPLS)(struct __sk_buff *skb)
{
- struct bpf_flow_keys *keys = skb->flow_keys;
struct mpls_label *mpls, _mpls;
mpls = bpf_flow_dissect_get_header(skb, sizeof(*mpls), &_mpls);
if (!mpls)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
- return export_flow_keys(keys, BPF_OK);
+ return parse_epilogue(skb, BPF_OK);
}
PROG(VLAN)(struct __sk_buff *skb)
@@ -396,10 +515,10 @@ PROG(VLAN)(struct __sk_buff *skb)
if (keys->n_proto == bpf_htons(ETH_P_8021AD)) {
vlan = bpf_flow_dissect_get_header(skb, sizeof(*vlan), &_vlan);
if (!vlan)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
if (vlan->h_vlan_encapsulated_proto != bpf_htons(ETH_P_8021Q))
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->nhoff += sizeof(*vlan);
keys->thoff += sizeof(*vlan);
@@ -407,14 +526,14 @@ PROG(VLAN)(struct __sk_buff *skb)
vlan = bpf_flow_dissect_get_header(skb, sizeof(*vlan), &_vlan);
if (!vlan)
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->nhoff += sizeof(*vlan);
keys->thoff += sizeof(*vlan);
/* Only allow 8021AD + 8021Q double tagging and no triple tagging.*/
if (vlan->h_vlan_encapsulated_proto == bpf_htons(ETH_P_8021AD) ||
vlan->h_vlan_encapsulated_proto == bpf_htons(ETH_P_8021Q))
- return export_flow_keys(keys, BPF_DROP);
+ return parse_epilogue(skb, BPF_DROP);
keys->n_proto = vlan->h_vlan_encapsulated_proto;
return parse_eth_proto(skb, vlan->h_vlan_encapsulated_proto);
diff --git a/tools/testing/selftests/bpf/test_flow_dissector.c b/tools/testing/selftests/bpf/test_flow_dissector.c
index 571cc076dd7d..aa80055a5518 100644
--- a/tools/testing/selftests/bpf/test_flow_dissector.c
+++ b/tools/testing/selftests/bpf/test_flow_dissector.c
@@ -17,6 +17,8 @@
#include <linux/if_packet.h>
#include <linux/if_ether.h>
#include <linux/ipv6.h>
+#include <linux/virtio_net.h>
+#include <net/if.h>
#include <netinet/ip.h>
#include <netinet/in.h>
#include <netinet/udp.h>
@@ -65,7 +67,8 @@ struct guehdr {
static uint8_t cfg_dsfield_inner;
static uint8_t cfg_dsfield_outer;
static uint8_t cfg_encap_proto;
-static bool cfg_expect_failure = false;
+static bool cfg_expect_norx;
+static bool cfg_expect_snd_failure;
static int cfg_l3_extra = AF_UNSPEC; /* optional SIT prefix */
static int cfg_l3_inner = AF_UNSPEC;
static int cfg_l3_outer = AF_UNSPEC;
@@ -77,8 +80,14 @@ static int cfg_port_gue = 6080;
static bool cfg_only_rx;
static bool cfg_only_tx;
static int cfg_src_port = 9;
+static bool cfg_tx_pf_packet;
+static bool cfg_use_vnet;
+static bool cfg_vnet_use_hdr_len_bad;
+static bool cfg_vnet_use_gso;
+static bool cfg_vnet_use_csum_off;
+static bool cfg_partial_udp_hdr;
-static char buf[ETH_DATA_LEN];
+static char buf[ETH_MAX_MTU];
#define INIT_ADDR4(name, addr4, port) \
static struct sockaddr_in name = { \
@@ -273,8 +282,48 @@ static int l3_length(int family)
return sizeof(struct ipv6hdr);
}
+static int build_vnet_header(void *header, int il3_len)
+{
+ struct virtio_net_hdr *vh = header;
+
+ vh->hdr_len = ETH_HLEN + il3_len + sizeof(struct udphdr);
+
+ if (cfg_partial_udp_hdr) {
+ vh->hdr_len -= (sizeof(struct udphdr) >> 1);
+ return sizeof(*vh);
+ }
+
+ /* Alteration must increase hdr_len; if not, kernel overwrites it */
+ if (cfg_vnet_use_hdr_len_bad)
+ vh->hdr_len++;
+
+ if (cfg_vnet_use_csum_off) {
+ vh->flags |= VIRTIO_NET_HDR_F_NEEDS_CSUM;
+ vh->csum_start = ETH_HLEN + il3_len;
+ vh->csum_offset = __builtin_offsetof(struct udphdr, check);
+ }
+
+ if (cfg_vnet_use_gso) {
+ vh->gso_type = VIRTIO_NET_HDR_GSO_UDP;
+ vh->gso_size = ETH_DATA_LEN - il3_len;
+ }
+
+ return sizeof(*vh);
+}
+
+static int build_eth_header(void *header)
+{
+ struct ethhdr *eth = header;
+ uint16_t proto = cfg_l3_inner == PF_INET ? ETH_P_IP : ETH_P_IPV6;
+
+ eth->h_proto = htons(proto);
+
+ return ETH_HLEN;
+}
+
static int build_packet(void)
{
+ int l2_len = 0;
int ol3_len = 0, ol4_len = 0, il3_len = 0, il4_len = 0;
int el3_len = 0;
@@ -294,23 +343,29 @@ static int build_packet(void)
il3_len = l3_length(cfg_l3_inner);
il4_len = sizeof(struct udphdr);
- if (el3_len + ol3_len + ol4_len + il3_len + il4_len + cfg_payload_len >=
- sizeof(buf))
+ if (cfg_use_vnet)
+ l2_len += build_vnet_header(buf, il3_len);
+ if (cfg_tx_pf_packet)
+ l2_len += build_eth_header(buf + l2_len);
+
+ if (l2_len + el3_len + ol3_len + ol4_len + il3_len + il4_len +
+ cfg_payload_len >= sizeof(buf))
error(1, 0, "packet too large\n");
/*
* Fill packet from inside out, to calculate correct checksums.
* But create ip before udp headers, as udp uses ip for pseudo-sum.
*/
- memset(buf + el3_len + ol3_len + ol4_len + il3_len + il4_len,
+ memset(buf + l2_len + el3_len + ol3_len + ol4_len + il3_len + il4_len,
cfg_payload_char, cfg_payload_len);
/* add zero byte for udp csum padding */
- buf[el3_len + ol3_len + ol4_len + il3_len + il4_len + cfg_payload_len] = 0;
+ buf[l2_len + el3_len + ol3_len + ol4_len + il3_len + il4_len +
+ cfg_payload_len] = 0;
switch (cfg_l3_inner) {
case PF_INET:
- build_ipv4_header(buf + el3_len + ol3_len + ol4_len,
+ build_ipv4_header(buf + l2_len + el3_len + ol3_len + ol4_len,
IPPROTO_UDP,
in_saddr4.sin_addr.s_addr,
in_daddr4.sin_addr.s_addr,
@@ -318,7 +373,7 @@ static int build_packet(void)
cfg_dsfield_inner);
break;
case PF_INET6:
- build_ipv6_header(buf + el3_len + ol3_len + ol4_len,
+ build_ipv6_header(buf + l2_len + el3_len + ol3_len + ol4_len,
IPPROTO_UDP,
&in_saddr6, &in_daddr6,
il4_len + cfg_payload_len,
@@ -326,22 +381,25 @@ static int build_packet(void)
break;
}
- build_udp_header(buf + el3_len + ol3_len + ol4_len + il3_len,
+ build_udp_header(buf + l2_len + el3_len + ol3_len + ol4_len + il3_len,
cfg_payload_len, CFG_PORT_INNER, cfg_l3_inner);
+ if (cfg_partial_udp_hdr)
+ return l2_len + il3_len + (il4_len >> 1);
+
if (!cfg_encap_proto)
- return il3_len + il4_len + cfg_payload_len;
+ return l2_len + il3_len + il4_len + cfg_payload_len;
switch (cfg_l3_outer) {
case PF_INET:
- build_ipv4_header(buf + el3_len, cfg_encap_proto,
+ build_ipv4_header(buf + l2_len + el3_len, cfg_encap_proto,
out_saddr4.sin_addr.s_addr,
out_daddr4.sin_addr.s_addr,
ol4_len + il3_len + il4_len + cfg_payload_len,
cfg_dsfield_outer);
break;
case PF_INET6:
- build_ipv6_header(buf + el3_len, cfg_encap_proto,
+ build_ipv6_header(buf + l2_len + el3_len, cfg_encap_proto,
&out_saddr6, &out_daddr6,
ol4_len + il3_len + il4_len + cfg_payload_len,
cfg_dsfield_outer);
@@ -350,17 +408,17 @@ static int build_packet(void)
switch (cfg_encap_proto) {
case IPPROTO_UDP:
- build_gue_header(buf + el3_len + ol3_len + ol4_len -
+ build_gue_header(buf + l2_len + el3_len + ol3_len + ol4_len -
sizeof(struct guehdr),
cfg_l3_inner == PF_INET ? IPPROTO_IPIP
: IPPROTO_IPV6);
- build_udp_header(buf + el3_len + ol3_len,
+ build_udp_header(buf + l2_len + el3_len + ol3_len,
sizeof(struct guehdr) + il3_len + il4_len +
cfg_payload_len,
cfg_port_gue, cfg_l3_outer);
break;
case IPPROTO_GRE:
- build_gre_header(buf + el3_len + ol3_len,
+ build_gre_header(buf + l2_len + el3_len + ol3_len,
cfg_l3_inner == PF_INET ? ETH_P_IP
: ETH_P_IPV6);
break;
@@ -368,7 +426,7 @@ static int build_packet(void)
switch (cfg_l3_extra) {
case PF_INET:
- build_ipv4_header(buf,
+ build_ipv4_header(buf + l2_len,
cfg_l3_outer == PF_INET ? IPPROTO_IPIP
: IPPROTO_IPV6,
extra_saddr4.sin_addr.s_addr,
@@ -377,7 +435,7 @@ static int build_packet(void)
cfg_payload_len, 0);
break;
case PF_INET6:
- build_ipv6_header(buf,
+ build_ipv6_header(buf + l2_len,
cfg_l3_outer == PF_INET ? IPPROTO_IPIP
: IPPROTO_IPV6,
&extra_saddr6, &extra_daddr6,
@@ -386,15 +444,46 @@ static int build_packet(void)
break;
}
- return el3_len + ol3_len + ol4_len + il3_len + il4_len +
+ return l2_len + el3_len + ol3_len + ol4_len + il3_len + il4_len +
cfg_payload_len;
}
+static int setup_tx_pfpacket(void)
+{
+ struct sockaddr_ll laddr = {0};
+ const int one = 1;
+ uint16_t proto;
+ int fd;
+
+ fd = socket(PF_PACKET, SOCK_RAW, 0);
+ if (fd == -1)
+ error(1, errno, "socket tx");
+
+ if (cfg_use_vnet &&
+ setsockopt(fd, SOL_PACKET, PACKET_VNET_HDR, &one, sizeof(one)))
+ error(1, errno, "setsockopt vnet");
+
+ proto = cfg_l3_inner == PF_INET ? ETH_P_IP : ETH_P_IPV6;
+ laddr.sll_family = AF_PACKET;
+ laddr.sll_protocol = htons(proto);
+ laddr.sll_ifindex = if_nametoindex("lo");
+ if (!laddr.sll_ifindex)
+ error(1, errno, "if_nametoindex");
+
+ if (bind(fd, (void *)&laddr, sizeof(laddr)))
+ error(1, errno, "bind");
+
+ return fd;
+}
+
/* sender transmits encapsulated over RAW or unencap'd over UDP */
static int setup_tx(void)
{
int family, fd, ret;
+ if (cfg_tx_pf_packet)
+ return setup_tx_pfpacket();
+
if (cfg_l3_extra)
family = cfg_l3_extra;
else if (cfg_l3_outer)
@@ -464,6 +553,13 @@ static int do_tx(int fd, const char *pkt, int len)
int ret;
ret = write(fd, pkt, len);
+
+ if (cfg_expect_snd_failure) {
+ if (ret == -1)
+ return 0;
+ error(1, 0, "expected tx to fail but it did not");
+ }
+
if (ret == -1)
error(1, errno, "send");
if (ret != len)
@@ -571,7 +667,7 @@ static int do_main(void)
* success (== 0) only if received all packets
* unless failure is expected, in which case none must arrive.
*/
- if (cfg_expect_failure)
+ if (cfg_expect_norx || cfg_expect_snd_failure)
return rx != 0;
else
return rx != tx;
@@ -623,8 +719,12 @@ static void parse_opts(int argc, char **argv)
{
int c;
- while ((c = getopt(argc, argv, "d:D:e:f:Fhi:l:n:o:O:Rs:S:t:Tx:X:")) != -1) {
+ while ((c = getopt(argc, argv,
+ "cd:D:e:Ef:FGghi:l:Ln:o:O:pRs:S:t:TUvx:X:")) != -1) {
switch (c) {
+ case 'c':
+ cfg_vnet_use_csum_off = true;
+ break;
case 'd':
if (cfg_l3_outer == AF_UNSPEC)
error(1, 0, "-d must be preceded by -o");
@@ -653,11 +753,17 @@ static void parse_opts(int argc, char **argv)
else
usage(argv[0]);
break;
+ case 'E':
+ cfg_expect_snd_failure = true;
+ break;
case 'f':
cfg_src_port = strtol(optarg, NULL, 0);
break;
case 'F':
- cfg_expect_failure = true;
+ cfg_expect_norx = true;
+ break;
+ case 'g':
+ cfg_vnet_use_gso = true;
break;
case 'h':
usage(argv[0]);
@@ -673,6 +779,9 @@ static void parse_opts(int argc, char **argv)
case 'l':
cfg_payload_len = strtol(optarg, NULL, 0);
break;
+ case 'L':
+ cfg_vnet_use_hdr_len_bad = true;
+ break;
case 'n':
cfg_num_pkt = strtol(optarg, NULL, 0);
break;
@@ -682,6 +791,9 @@ static void parse_opts(int argc, char **argv)
case 'O':
cfg_l3_extra = parse_protocol_family(argv[0], optarg);
break;
+ case 'p':
+ cfg_tx_pf_packet = true;
+ break;
case 'R':
cfg_only_rx = true;
break;
@@ -703,6 +815,12 @@ static void parse_opts(int argc, char **argv)
case 'T':
cfg_only_tx = true;
break;
+ case 'U':
+ cfg_partial_udp_hdr = true;
+ break;
+ case 'v':
+ cfg_use_vnet = true;
+ break;
case 'x':
cfg_dsfield_outer = strtol(optarg, NULL, 0);
break;
@@ -733,7 +851,26 @@ static void parse_opts(int argc, char **argv)
*/
if (((cfg_dsfield_outer & 0x3) == 0x3) &&
((cfg_dsfield_inner & 0x3) == 0x0))
- cfg_expect_failure = true;
+ cfg_expect_norx = true;
+
+ /* Don't wait around for packets that we expect to fail to send */
+ if (cfg_expect_snd_failure && !cfg_num_secs)
+ cfg_num_secs = 3;
+
+ if (cfg_partial_udp_hdr && cfg_encap_proto)
+ error(1, 0,
+ "ops: can't specify partial UDP hdr (-U) and encap (-e)");
+
+ if (cfg_use_vnet && cfg_encap_proto)
+ error(1, 0, "options: cannot specify encap (-e) with vnet (-v)");
+ if (cfg_use_vnet && !cfg_tx_pf_packet)
+ error(1, 0, "options: vnet (-v) requires psock for tx (-p)");
+ if (cfg_vnet_use_gso && !cfg_use_vnet)
+ error(1, 0, "options: gso (-g) requires vnet (-v)");
+ if (cfg_vnet_use_csum_off && !cfg_use_vnet)
+ error(1, 0, "options: vnet csum (-c) requires vnet (-v)");
+ if (cfg_vnet_use_hdr_len_bad && !cfg_use_vnet)
+ error(1, 0, "options: bad vnet hdrlen (-L) requires vnet (-v)");
}
static void print_opts(void)
diff --git a/tools/testing/selftests/bpf/test_flow_dissector.sh b/tools/testing/selftests/bpf/test_flow_dissector.sh
index 174b72a64a4c..5852cf815eeb 100755
--- a/tools/testing/selftests/bpf/test_flow_dissector.sh
+++ b/tools/testing/selftests/bpf/test_flow_dissector.sh
@@ -51,6 +51,9 @@ if [[ -z $(ip netns identify $$) ]]; then
echo "Skipping root flow dissector test, bpftool not found" >&2
fi
+ orig_flow_dissect_sysctl=$(</proc/sys/net/core/flow_dissect_vnet_hdr)
+ sysctl net.core.flow_dissect_vnet_hdr=1
+
# Run the rest of the tests in a net namespace.
../net/in_netns.sh "$0" "$@"
err=$(( $err + $? ))
@@ -61,6 +64,7 @@ if [[ -z $(ip netns identify $$) ]]; then
echo "selftests: $TESTNAME [FAILED]";
fi
+ sysctl net.core.flow_dissect_vnet_hdr=$orig_flow_dissect_sysctl
exit $err
fi
@@ -165,4 +169,19 @@ tc filter add dev lo parent ffff: protocol ipv6 pref 1337 flower ip_proto \
# Send 10 IPv6/UDP packets from port 10. Filter should not drop any.
./test_flow_dissector -i 6 -f 10
+
+echo "Testing virtio-net header validation..."
+echo "Testing valid vnet headers. Should *not* be dropped."
+./with_addr.sh ./test_flow_dissector -i 4 -D 192.168.0.1 -S 1.1.1.1 -p -v
+echo "Testing partial transport header. Should be dropped."
+./with_addr.sh ./test_flow_dissector -i 4 -D 192.168.0.1 -S 1.1.1.1 -p -v -U -E
+echo "Testing valid vnet gso spec. Should *not* be dropped."
+./with_addr.sh ./test_flow_dissector -i 4 -D 192.168.0.1 -S 1.1.1.1 -p -v -g -c -l 8000
+echo "Testing invalid vnet gso size. Should be dropped."
+./with_addr.sh ./test_flow_dissector -i 4 -D 192.168.0.1 -S 1.1.1.1 -p -v -g -c -l 100 -E
+echo "Testing invalid vnet header len. Should be dropped."
+./with_addr.sh ./test_flow_dissector -i 4 -D 192.168.0.1 -S 1.1.1.1 -p -v -L -E
+echo "Testing vnet gso without csum. Should be dropped."
+./with_addr.sh ./test_flow_dissector -i 4 -D 192.168.0.1 -S 1.1.1.1 -p -v -g -l 8000 -E
+
exit 0
--
2.32.0.272.g935e593368-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
2021-06-15 0:10 ` [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr Tanner Love
@ 2021-06-15 22:25 ` Martin KaFai Lau
2021-06-15 23:50 ` Tanner Love
2021-06-16 15:49 ` kernel test robot
2021-06-16 17:21 ` kernel test robot
2 siblings, 1 reply; 11+ messages in thread
From: Martin KaFai Lau @ 2021-06-15 22:25 UTC (permalink / raw)
To: Tanner Love
Cc: netdev, davem, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Eric Dumazet, Willem de Bruijn, Petar Penkov,
Jakub Kicinski, Michael S . Tsirkin, Jason Wang, Tanner Love,
Stanislav Fomichev
On Mon, Jun 14, 2021 at 08:10:58PM -0400, Tanner Love wrote:
> From: Tanner Love <tannerlove@google.com>
>
> Amend the bpf flow dissector program type to be able to process
> virtio-net headers. Do this to enable bpf flow dissector programs to
> perform virtio-net header validation. The next patch in this series
> will add a flow dissection hook in virtio_net_hdr_to_skb and make use
> of this extended functionality. That commit message has more
> background on the use case.
>
> Add two new members to struct bpf_flow_keys: a pointer to struct
> virtio_net_hdr, and vhdr_is_little_endian. The latter is required to
> inform the BPF program of the endianness of the virtio-net header
> fields, to handle the case of a version 1+ header on a big endian
> machine.
>
> Changes
> v6:
> - Move bpf_flow_dissector_btf_ids, check_flow_keys_access() to
> filter.c
> - Verify (off % size == 0) in check_flow_keys_access()
> - Check bpf_flow_dissector_btf_ids[0] is nonzero in
> check_flow_keys_access()
> v5:
> - Use PTR_TO_BTF_ID_OR_NULL instead of defining new
> PTR_TO_VNET_HDR_OR_NULL
> - Make check_flow_keys_access() disallow writes to keys->vhdr
> - Make check_flow_keys_access() check loading keys->vhdr is in
> sizeof(__u64)
> - Use BPF_REG_AX instead of BPF_REG_TMP as scratch reg
> - Describe parameter vhdr_is_little_endian in __skb_flow_dissect
> documentation
> v4:
> - Add virtio_net_hdr pointer to struct bpf_flow_keys
> - Add vhdr_is_little_endian to struct bpf_flow_keys
> v2:
> - Describe parameter vhdr in __skb_flow_dissect documentation
>
> Signed-off-by: Tanner Love <tannerlove@google.com>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> Reviewed-by: Petar Penkov <ppenkov@google.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> ---
> drivers/net/bonding/bond_main.c | 2 +-
> include/linux/bpf.h | 3 ++
> include/linux/skbuff.h | 35 ++++++++++++++++-----
> include/uapi/linux/bpf.h | 2 ++
> kernel/bpf/verifier.c | 35 ++++++++++++---------
> net/bpf/test_run.c | 2 +-
> net/core/filter.c | 56 +++++++++++++++++++++++++++++++++
> net/core/flow_dissector.c | 18 ++++++++---
> tools/include/uapi/linux/bpf.h | 2 ++
> 9 files changed, 127 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index eb79a9f05914..36993636d56d 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3554,7 +3554,7 @@ static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
> case BOND_XMIT_POLICY_ENCAP34:
> memset(fk, 0, sizeof(*fk));
> return __skb_flow_dissect(NULL, skb, &flow_keys_bonding,
> - fk, NULL, 0, 0, 0, 0);
> + fk, NULL, 0, 0, 0, 0, NULL, false);
> default:
> break;
> }
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 9dc44ba97584..f08dee59b099 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1515,6 +1515,9 @@ static inline int bpf_map_attr_numa_node(const union bpf_attr *attr)
> attr->numa_node : NUMA_NO_NODE;
> }
>
> +int check_flow_keys_access(int off, int size, enum bpf_access_type t,
> + struct bpf_insn_access_aux *info);
Thanks for moving it!
1. It needs to be put under CONFIG_NET. There is one earlier where
bpf_sock_is_valid_access() also resides.
2. nit. Rename it to xyz_is_valid_access(). xyz probably is
bpf_flow_keys here.
> void skb_flow_dissect_meta(const struct sk_buff *skb,
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 418b9b813d65..e1ac34548f9a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6017,6 +6017,8 @@ struct bpf_flow_keys {
> };
> __u32 flags;
> __be32 flow_label;
> + __bpf_md_ptr(const struct virtio_net_hdr *, vhdr);
> + __u8 vhdr_is_little_endian;
I am not familiar with virtio. A question on the "vhdr_is_little_endian" field.
The commit message said
"to handle the case of a version 1+ header on a big endian machine".
iiuc, version 1+ is always in little endian?
Does it mean most cases are in little endian?
and at least will eventually be moved to version 1+?
I wonder if this field will eventually be useless (because of always
true) and can it be avoided from the uapi now. The current uapi
fields (e.g. in bpf_sock) are always in one particular order.
If it is in big endian, can it be changed to little endian first
before calling the bpf prog?
> };
>
> struct bpf_func_info {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 331b170d9fcc..a037476954f5 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -22,6 +22,7 @@
> #include <linux/error-injection.h>
> #include <linux/bpf_lsm.h>
> #include <linux/btf_ids.h>
> +#include <linux/virtio_net.h>
>
> #include "disasm.h"
>
> @@ -3372,18 +3373,6 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
> return -EACCES;
> }
>
> -static int check_flow_keys_access(struct bpf_verifier_env *env, int off,
> - int size)
> -{
> - if (size < 0 || off < 0 ||
> - (u64)off + size > sizeof(struct bpf_flow_keys)) {
> - verbose(env, "invalid access to flow keys off=%d size=%d\n",
> - off, size);
> - return -EACCES;
> - }
> - return 0;
> -}
> -
> static int check_sock_access(struct bpf_verifier_env *env, int insn_idx,
> u32 regno, int off, int size,
> enum bpf_access_type t)
> @@ -4210,6 +4199,8 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> if (!err && t == BPF_READ && value_regno >= 0)
> mark_reg_unknown(env, regs, value_regno);
> } else if (reg->type == PTR_TO_FLOW_KEYS) {
> + struct bpf_insn_access_aux info = {};
> +
> if (t == BPF_WRITE && value_regno >= 0 &&
> is_pointer_value(env, value_regno)) {
> verbose(env, "R%d leaks addr into flow keys\n",
> @@ -4217,9 +4208,23 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> return -EACCES;
> }
>
> - err = check_flow_keys_access(env, off, size);
> - if (!err && t == BPF_READ && value_regno >= 0)
> - mark_reg_unknown(env, regs, value_regno);
> + err = check_flow_keys_access(off, size, t, &info);
> + if (err) {
> + verbose(env,
> + "invalid access to flow keys off=%d size=%d\n",
> + off, size);
> + } else if (t == BPF_READ && value_regno >= 0) {
> + if (off == offsetof(struct bpf_flow_keys, vhdr)) {
The logic below is very similar to the earlier PTR_TO_CTX case and
they can be refactored in the future.
It is better to check a generic PTR_TO_BTF_ID_OR_NULL value (and more on the
info.reg_type and info.btf later) instead of something specific to bpf_flow_keys
such that it will be easier to make sense with when refactoring with the
PTR_TO_CTX case in the future. Something like:
if (info.reg_type == PTR_TO_BTF_ID_OR_NULL) {
mark_reg_known_zero(env, regs, value_regno);
regs[value_regno].type = info.reg_type;
regs[value_regno].btf = info.btf;
regs[value_regno.btf_id = info.btf_id;
regs[value_regno].id = ++env->id_gen;
} else {
mark_reg_unknown(env, regs, value_regno);
}
> + mark_reg_known_zero(env, regs, value_regno);
> + regs[value_regno].type = PTR_TO_BTF_ID_OR_NULL;
> + regs[value_regno].btf = btf_vmlinux;
> + regs[value_regno].btf_id = info.btf_id;
> + /* required for dropping or_null */
> + regs[value_regno].id = ++env->id_gen;
> + } else {
> + mark_reg_unknown(env, regs, value_regno);
> + }
> + }
> } else if (type_is_sk_pointer(reg->type)) {
> if (t == BPF_WRITE) {
> verbose(env, "R%d cannot write into %s\n",
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index aa47af349ba8..a11c5ce99ccb 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -797,7 +797,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> bpf_test_timer_enter(&t);
> do {
> retval = bpf_flow_dissect(prog, &ctx, eth->h_proto, ETH_HLEN,
> - size, flags);
> + size, flags, NULL, false);
> } while (bpf_test_timer_continue(&t, repeat, &ret, &duration));
> bpf_test_timer_leave(&t);
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 239de1306de9..f5be14b947cd 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -8329,6 +8329,36 @@ static bool sk_msg_is_valid_access(int off, int size,
> return true;
> }
>
> +BTF_ID_LIST_SINGLE(bpf_flow_dissector_btf_ids, struct, virtio_net_hdr);
> +
> +int check_flow_keys_access(int off, int size, enum bpf_access_type t,
> + struct bpf_insn_access_aux *info)
> +{
> + if (size < 0 || off < 0 ||
> + (u64)off + size > sizeof(struct bpf_flow_keys))
"size" must be > 0 here or the verifier should have already rejected it,
so "size < 0" can be removed.
sizeof() is not enough now. There is end padding now beause of
the "__u8 vhdr_is_little_endian;". It is a good chance
to repleace it with offsetofend(struct bpf_flow_keys, whatever_last_member).
> + return -EACCES;
> +
> + switch (off) {
> + case bpf_ctx_range_ptr(struct bpf_flow_keys, vhdr):
> + if (t == BPF_WRITE || off % size != 0 || size != sizeof(__u64))
> + return -EACCES;
> +
> + if (!bpf_flow_dissector_btf_ids[0])
> + return -EINVAL;
> +
> + info->btf_id = bpf_flow_dissector_btf_ids[0];
It is setting the info->btf_id. Set the info->btf and info->reg_type
here also instead of having the caller to second guess.
info->btf = btf_vmlinux;
info->reg_type = PTR_TO_BTF_ID_OR_NULL;
others lgtm.
> +
> + break;
> + case offsetof(struct bpf_flow_keys, vhdr_is_little_endian):
> + if (t == BPF_WRITE)
> + return -EACCES;
> +
> + break;
> + }
> +
> + return 0;
> +}
> +
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
2021-06-15 22:25 ` Martin KaFai Lau
@ 2021-06-15 23:50 ` Tanner Love
2021-06-16 0:12 ` Martin KaFai Lau
0 siblings, 1 reply; 11+ messages in thread
From: Tanner Love @ 2021-06-15 23:50 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Network Development, David S . Miller, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eric Dumazet, Willem de Bruijn,
Petar Penkov, Jakub Kicinski, Michael S . Tsirkin, Jason Wang,
Tanner Love, Stanislav Fomichev
On Tue, Jun 15, 2021 at 3:25 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Mon, Jun 14, 2021 at 08:10:58PM -0400, Tanner Love wrote:
> > From: Tanner Love <tannerlove@google.com>
> >
> > Amend the bpf flow dissector program type to be able to process
> > virtio-net headers. Do this to enable bpf flow dissector programs to
> > perform virtio-net header validation. The next patch in this series
> > will add a flow dissection hook in virtio_net_hdr_to_skb and make use
> > of this extended functionality. That commit message has more
> > background on the use case.
> >
> > Add two new members to struct bpf_flow_keys: a pointer to struct
> > virtio_net_hdr, and vhdr_is_little_endian. The latter is required to
> > inform the BPF program of the endianness of the virtio-net header
> > fields, to handle the case of a version 1+ header on a big endian
> > machine.
> >
> > Changes
> > v6:
> > - Move bpf_flow_dissector_btf_ids, check_flow_keys_access() to
> > filter.c
> > - Verify (off % size == 0) in check_flow_keys_access()
> > - Check bpf_flow_dissector_btf_ids[0] is nonzero in
> > check_flow_keys_access()
> > v5:
> > - Use PTR_TO_BTF_ID_OR_NULL instead of defining new
> > PTR_TO_VNET_HDR_OR_NULL
> > - Make check_flow_keys_access() disallow writes to keys->vhdr
> > - Make check_flow_keys_access() check loading keys->vhdr is in
> > sizeof(__u64)
> > - Use BPF_REG_AX instead of BPF_REG_TMP as scratch reg
> > - Describe parameter vhdr_is_little_endian in __skb_flow_dissect
> > documentation
> > v4:
> > - Add virtio_net_hdr pointer to struct bpf_flow_keys
> > - Add vhdr_is_little_endian to struct bpf_flow_keys
> > v2:
> > - Describe parameter vhdr in __skb_flow_dissect documentation
> >
> > Signed-off-by: Tanner Love <tannerlove@google.com>
> > Reviewed-by: Willem de Bruijn <willemb@google.com>
> > Reviewed-by: Petar Penkov <ppenkov@google.com>
> > Reviewed-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > drivers/net/bonding/bond_main.c | 2 +-
> > include/linux/bpf.h | 3 ++
> > include/linux/skbuff.h | 35 ++++++++++++++++-----
> > include/uapi/linux/bpf.h | 2 ++
> > kernel/bpf/verifier.c | 35 ++++++++++++---------
> > net/bpf/test_run.c | 2 +-
> > net/core/filter.c | 56 +++++++++++++++++++++++++++++++++
> > net/core/flow_dissector.c | 18 ++++++++---
> > tools/include/uapi/linux/bpf.h | 2 ++
> > 9 files changed, 127 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index eb79a9f05914..36993636d56d 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -3554,7 +3554,7 @@ static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
> > case BOND_XMIT_POLICY_ENCAP34:
> > memset(fk, 0, sizeof(*fk));
> > return __skb_flow_dissect(NULL, skb, &flow_keys_bonding,
> > - fk, NULL, 0, 0, 0, 0);
> > + fk, NULL, 0, 0, 0, 0, NULL, false);
> > default:
> > break;
> > }
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 9dc44ba97584..f08dee59b099 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1515,6 +1515,9 @@ static inline int bpf_map_attr_numa_node(const union bpf_attr *attr)
> > attr->numa_node : NUMA_NO_NODE;
> > }
> >
> > +int check_flow_keys_access(int off, int size, enum bpf_access_type t,
> > + struct bpf_insn_access_aux *info);
> Thanks for moving it!
Thanks for all your helpful feedback!
>
> 1. It needs to be put under CONFIG_NET. There is one earlier where
> bpf_sock_is_valid_access() also resides.
> 2. nit. Rename it to xyz_is_valid_access(). xyz probably is
> bpf_flow_keys here.
>
> > void skb_flow_dissect_meta(const struct sk_buff *skb,
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 418b9b813d65..e1ac34548f9a 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -6017,6 +6017,8 @@ struct bpf_flow_keys {
> > };
> > __u32 flags;
> > __be32 flow_label;
> > + __bpf_md_ptr(const struct virtio_net_hdr *, vhdr);
> > + __u8 vhdr_is_little_endian;
> I am not familiar with virtio. A question on the "vhdr_is_little_endian" field.
> The commit message said
> "to handle the case of a version 1+ header on a big endian machine".
> iiuc, version 1+ is always in little endian?
That's right.
> Does it mean most cases are in little endian?
> and at least will eventually be moved to version 1+?
>
> I wonder if this field will eventually be useless (because of always
> true) and can it be avoided from the uapi now. The current uapi
> fields (e.g. in bpf_sock) are always in one particular order.
>
> If it is in big endian, can it be changed to little endian first
> before calling the bpf prog?
In fact, v1 of this patch set did the conversion prior to passing
the fields to the bpf prog, which meant that the bpf prog did not
have to do anything about endianness. I changed that, though,
at the suggestion of Alexei; Alexei suggested that we pass a
pointer to struct virtio_net_hdr, rather than copying the individual
virtio_net_hdr fields. V1 did the endianness conversion as part
of that copying process. If we go back to doing it like that, then
we lose the advantage that Alexei's suggestion aimed to achieve
(i.e. avoiding the cost of copying the fields).
I will address your other comments in the next version. Thanks
>
> > };
> >
> > struct bpf_func_info {
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 331b170d9fcc..a037476954f5 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -22,6 +22,7 @@
> > #include <linux/error-injection.h>
> > #include <linux/bpf_lsm.h>
> > #include <linux/btf_ids.h>
> > +#include <linux/virtio_net.h>
> >
> > #include "disasm.h"
> >
> > @@ -3372,18 +3373,6 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
> > return -EACCES;
> > }
> >
> > -static int check_flow_keys_access(struct bpf_verifier_env *env, int off,
> > - int size)
> > -{
> > - if (size < 0 || off < 0 ||
> > - (u64)off + size > sizeof(struct bpf_flow_keys)) {
> > - verbose(env, "invalid access to flow keys off=%d size=%d\n",
> > - off, size);
> > - return -EACCES;
> > - }
> > - return 0;
> > -}
> > -
> > static int check_sock_access(struct bpf_verifier_env *env, int insn_idx,
> > u32 regno, int off, int size,
> > enum bpf_access_type t)
> > @@ -4210,6 +4199,8 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> > if (!err && t == BPF_READ && value_regno >= 0)
> > mark_reg_unknown(env, regs, value_regno);
> > } else if (reg->type == PTR_TO_FLOW_KEYS) {
> > + struct bpf_insn_access_aux info = {};
> > +
> > if (t == BPF_WRITE && value_regno >= 0 &&
> > is_pointer_value(env, value_regno)) {
> > verbose(env, "R%d leaks addr into flow keys\n",
> > @@ -4217,9 +4208,23 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> > return -EACCES;
> > }
> >
> > - err = check_flow_keys_access(env, off, size);
> > - if (!err && t == BPF_READ && value_regno >= 0)
> > - mark_reg_unknown(env, regs, value_regno);
> > + err = check_flow_keys_access(off, size, t, &info);
> > + if (err) {
> > + verbose(env,
> > + "invalid access to flow keys off=%d size=%d\n",
> > + off, size);
> > + } else if (t == BPF_READ && value_regno >= 0) {
> > + if (off == offsetof(struct bpf_flow_keys, vhdr)) {
> The logic below is very similar to the earlier PTR_TO_CTX case and
> they can be refactored in the future.
>
> It is better to check a generic PTR_TO_BTF_ID_OR_NULL value (and more on the
> info.reg_type and info.btf later) instead of something specific to bpf_flow_keys
> such that it will be easier to make sense with when refactoring with the
> PTR_TO_CTX case in the future. Something like:
>
> if (info.reg_type == PTR_TO_BTF_ID_OR_NULL) {
> mark_reg_known_zero(env, regs, value_regno);
> regs[value_regno].type = info.reg_type;
> regs[value_regno].btf = info.btf;
> regs[value_regno.btf_id = info.btf_id;
> regs[value_regno].id = ++env->id_gen;
> } else {
> mark_reg_unknown(env, regs, value_regno);
> }
> > + mark_reg_known_zero(env, regs, value_regno);
> > + regs[value_regno].type = PTR_TO_BTF_ID_OR_NULL;
> > + regs[value_regno].btf = btf_vmlinux;
> > + regs[value_regno].btf_id = info.btf_id;
> > + /* required for dropping or_null */
> > + regs[value_regno].id = ++env->id_gen;
> > + } else {
> > + mark_reg_unknown(env, regs, value_regno);
> > + }
> > + }
> > } else if (type_is_sk_pointer(reg->type)) {
> > if (t == BPF_WRITE) {
> > verbose(env, "R%d cannot write into %s\n",
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index aa47af349ba8..a11c5ce99ccb 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -797,7 +797,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> > bpf_test_timer_enter(&t);
> > do {
> > retval = bpf_flow_dissect(prog, &ctx, eth->h_proto, ETH_HLEN,
> > - size, flags);
> > + size, flags, NULL, false);
> > } while (bpf_test_timer_continue(&t, repeat, &ret, &duration));
> > bpf_test_timer_leave(&t);
> >
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 239de1306de9..f5be14b947cd 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -8329,6 +8329,36 @@ static bool sk_msg_is_valid_access(int off, int size,
> > return true;
> > }
> >
> > +BTF_ID_LIST_SINGLE(bpf_flow_dissector_btf_ids, struct, virtio_net_hdr);
> > +
> > +int check_flow_keys_access(int off, int size, enum bpf_access_type t,
> > + struct bpf_insn_access_aux *info)
> > +{
> > + if (size < 0 || off < 0 ||
> > + (u64)off + size > sizeof(struct bpf_flow_keys))
> "size" must be > 0 here or the verifier should have already rejected it,
> so "size < 0" can be removed.
>
> sizeof() is not enough now. There is end padding now beause of
> the "__u8 vhdr_is_little_endian;". It is a good chance
> to repleace it with offsetofend(struct bpf_flow_keys, whatever_last_member).
>
> > + return -EACCES;
> > +
> > + switch (off) {
> > + case bpf_ctx_range_ptr(struct bpf_flow_keys, vhdr):
> > + if (t == BPF_WRITE || off % size != 0 || size != sizeof(__u64))
> > + return -EACCES;
> > +
> > + if (!bpf_flow_dissector_btf_ids[0])
> > + return -EINVAL;
> > +
> > + info->btf_id = bpf_flow_dissector_btf_ids[0];
> It is setting the info->btf_id. Set the info->btf and info->reg_type
> here also instead of having the caller to second guess.
>
> info->btf = btf_vmlinux;
> info->reg_type = PTR_TO_BTF_ID_OR_NULL;
>
> others lgtm.
>
> > +
> > + break;
> > + case offsetof(struct bpf_flow_keys, vhdr_is_little_endian):
> > + if (t == BPF_WRITE)
> > + return -EACCES;
> > +
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
2021-06-15 23:50 ` Tanner Love
@ 2021-06-16 0:12 ` Martin KaFai Lau
0 siblings, 0 replies; 11+ messages in thread
From: Martin KaFai Lau @ 2021-06-16 0:12 UTC (permalink / raw)
To: Tanner Love
Cc: Network Development, David S . Miller, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eric Dumazet, Willem de Bruijn,
Petar Penkov, Jakub Kicinski, Michael S . Tsirkin, Jason Wang,
Tanner Love, Stanislav Fomichev
On Tue, Jun 15, 2021 at 04:50:53PM -0700, Tanner Love wrote:
[ ... ]
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index 418b9b813d65..e1ac34548f9a 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -6017,6 +6017,8 @@ struct bpf_flow_keys {
> > > };
> > > __u32 flags;
> > > __be32 flow_label;
> > > + __bpf_md_ptr(const struct virtio_net_hdr *, vhdr);
> > > + __u8 vhdr_is_little_endian;
> > I am not familiar with virtio. A question on the "vhdr_is_little_endian" field.
> > The commit message said
> > "to handle the case of a version 1+ header on a big endian machine".
> > iiuc, version 1+ is always in little endian?
>
> That's right.
>
> > Does it mean most cases are in little endian?
> > and at least will eventually be moved to version 1+?
hmm..... so the common cases are little endian or
at least the remaining cases will eventually be moved to
version 1+ which is little endian?
> >
> > I wonder if this field will eventually be useless (because of always
> > true) and can it be avoided from the uapi now. The current uapi
> > fields (e.g. in bpf_sock) are always in one particular order.
> >
> > If it is in big endian, can it be changed to little endian first
> > before calling the bpf prog?
>
> In fact, v1 of this patch set did the conversion prior to passing
> the fields to the bpf prog, which meant that the bpf prog did not
> have to do anything about endianness. I changed that, though,
> at the suggestion of Alexei; Alexei suggested that we pass a
> pointer to struct virtio_net_hdr, rather than copying the individual
> virtio_net_hdr fields. V1 did the endianness conversion as part
> of that copying process. If we go back to doing it like that, then
> we lose the advantage that Alexei's suggestion aimed to achieve
> (i.e. avoiding the cost of copying the fields).
If the common case is little endian, then the vhdr pointer can still
be passed to bpf prog as-is directly for speed. A deep copy and
conversion are only needed for the less common big endian case which
eventually will be moved to version 1+?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
2021-06-15 0:10 ` [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr Tanner Love
@ 2021-06-16 15:49 ` kernel test robot
2021-06-16 15:49 ` kernel test robot
2021-06-16 17:21 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-06-16 15:49 UTC (permalink / raw)
To: Tanner Love, netdev
Cc: kbuild-all, davem, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Eric Dumazet, Willem de Bruijn, Petar Penkov,
Jakub Kicinski, Michael S . Tsirkin
[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]
Hi Tanner,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tanner-Love/virtio_net-add-optional-flow-dissection-in-virtio_net_hdr_to_skb/20210616-193224
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c7654495916e109f76a67fd3ae68f8fa70ab4faa
config: um-x86_64_defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/7d159f648961a7849f67e1c3d7ecb3d18bf5c7c2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tanner-Love/virtio_net-add-optional-flow-dissection-in-virtio_net_hdr_to_skb/20210616-193224
git checkout 7d159f648961a7849f67e1c3d7ecb3d18bf5c7c2
# save the attached .config to linux build tree
make W=1 ARCH=um SUBARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/core/filter.c:8334:5: warning: no previous prototype for 'check_flow_keys_access' [-Wmissing-prototypes]
8334 | int check_flow_keys_access(int off, int size, enum bpf_access_type t,
| ^~~~~~~~~~~~~~~~~~~~~~
vim +/check_flow_keys_access +8334 net/core/filter.c
8333
> 8334 int check_flow_keys_access(int off, int size, enum bpf_access_type t,
8335 struct bpf_insn_access_aux *info)
8336 {
8337 if (size < 0 || off < 0 ||
8338 (u64)off + size > sizeof(struct bpf_flow_keys))
8339 return -EACCES;
8340
8341 switch (off) {
8342 case bpf_ctx_range_ptr(struct bpf_flow_keys, vhdr):
8343 if (t == BPF_WRITE || off % size != 0 || size != sizeof(__u64))
8344 return -EACCES;
8345
8346 if (!bpf_flow_dissector_btf_ids[0])
8347 return -EINVAL;
8348
8349 info->btf_id = bpf_flow_dissector_btf_ids[0];
8350
8351 break;
8352 case offsetof(struct bpf_flow_keys, vhdr_is_little_endian):
8353 if (t == BPF_WRITE)
8354 return -EACCES;
8355
8356 break;
8357 }
8358
8359 return 0;
8360 }
8361
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8653 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
@ 2021-06-16 15:49 ` kernel test robot
0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-06-16 15:49 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2382 bytes --]
Hi Tanner,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tanner-Love/virtio_net-add-optional-flow-dissection-in-virtio_net_hdr_to_skb/20210616-193224
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c7654495916e109f76a67fd3ae68f8fa70ab4faa
config: um-x86_64_defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/7d159f648961a7849f67e1c3d7ecb3d18bf5c7c2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tanner-Love/virtio_net-add-optional-flow-dissection-in-virtio_net_hdr_to_skb/20210616-193224
git checkout 7d159f648961a7849f67e1c3d7ecb3d18bf5c7c2
# save the attached .config to linux build tree
make W=1 ARCH=um SUBARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/core/filter.c:8334:5: warning: no previous prototype for 'check_flow_keys_access' [-Wmissing-prototypes]
8334 | int check_flow_keys_access(int off, int size, enum bpf_access_type t,
| ^~~~~~~~~~~~~~~~~~~~~~
vim +/check_flow_keys_access +8334 net/core/filter.c
8333
> 8334 int check_flow_keys_access(int off, int size, enum bpf_access_type t,
8335 struct bpf_insn_access_aux *info)
8336 {
8337 if (size < 0 || off < 0 ||
8338 (u64)off + size > sizeof(struct bpf_flow_keys))
8339 return -EACCES;
8340
8341 switch (off) {
8342 case bpf_ctx_range_ptr(struct bpf_flow_keys, vhdr):
8343 if (t == BPF_WRITE || off % size != 0 || size != sizeof(__u64))
8344 return -EACCES;
8345
8346 if (!bpf_flow_dissector_btf_ids[0])
8347 return -EINVAL;
8348
8349 info->btf_id = bpf_flow_dissector_btf_ids[0];
8350
8351 break;
8352 case offsetof(struct bpf_flow_keys, vhdr_is_little_endian):
8353 if (t == BPF_WRITE)
8354 return -EACCES;
8355
8356 break;
8357 }
8358
8359 return 0;
8360 }
8361
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 8653 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
2021-06-15 0:10 ` [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr Tanner Love
@ 2021-06-16 17:21 ` kernel test robot
2021-06-16 15:49 ` kernel test robot
2021-06-16 17:21 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-06-16 17:21 UTC (permalink / raw)
To: Tanner Love, netdev
Cc: kbuild-all, clang-built-linux, davem, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eric Dumazet, Willem de Bruijn,
Petar Penkov, Jakub Kicinski, Michael S . Tsirkin
[-- Attachment #1: Type: text/plain, Size: 3274 bytes --]
Hi Tanner,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tanner-Love/virtio_net-add-optional-flow-dissection-in-virtio_net_hdr_to_skb/20210616-193224
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c7654495916e109f76a67fd3ae68f8fa70ab4faa
config: riscv-randconfig-r024-20210615 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/7d159f648961a7849f67e1c3d7ecb3d18bf5c7c2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tanner-Love/virtio_net-add-optional-flow-dissection-in-virtio_net_hdr_to_skb/20210616-193224
git checkout 7d159f648961a7849f67e1c3d7ecb3d18bf5c7c2
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/core/filter.c:8334:5: warning: no previous prototype for function 'check_flow_keys_access' [-Wmissing-prototypes]
int check_flow_keys_access(int off, int size, enum bpf_access_type t,
^
net/core/filter.c:8334:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int check_flow_keys_access(int off, int size, enum bpf_access_type t,
^
static
1 warning generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for LOCKDEP
Depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
Selected by
- PROVE_LOCKING && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
- DEBUG_LOCK_ALLOC && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
vim +/check_flow_keys_access +8334 net/core/filter.c
8333
> 8334 int check_flow_keys_access(int off, int size, enum bpf_access_type t,
8335 struct bpf_insn_access_aux *info)
8336 {
8337 if (size < 0 || off < 0 ||
8338 (u64)off + size > sizeof(struct bpf_flow_keys))
8339 return -EACCES;
8340
8341 switch (off) {
8342 case bpf_ctx_range_ptr(struct bpf_flow_keys, vhdr):
8343 if (t == BPF_WRITE || off % size != 0 || size != sizeof(__u64))
8344 return -EACCES;
8345
8346 if (!bpf_flow_dissector_btf_ids[0])
8347 return -EINVAL;
8348
8349 info->btf_id = bpf_flow_dissector_btf_ids[0];
8350
8351 break;
8352 case offsetof(struct bpf_flow_keys, vhdr_is_little_endian):
8353 if (t == BPF_WRITE)
8354 return -EACCES;
8355
8356 break;
8357 }
8358
8359 return 0;
8360 }
8361
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43679 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
@ 2021-06-16 17:21 ` kernel test robot
0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-06-16 17:21 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3355 bytes --]
Hi Tanner,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tanner-Love/virtio_net-add-optional-flow-dissection-in-virtio_net_hdr_to_skb/20210616-193224
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c7654495916e109f76a67fd3ae68f8fa70ab4faa
config: riscv-randconfig-r024-20210615 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/7d159f648961a7849f67e1c3d7ecb3d18bf5c7c2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tanner-Love/virtio_net-add-optional-flow-dissection-in-virtio_net_hdr_to_skb/20210616-193224
git checkout 7d159f648961a7849f67e1c3d7ecb3d18bf5c7c2
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/core/filter.c:8334:5: warning: no previous prototype for function 'check_flow_keys_access' [-Wmissing-prototypes]
int check_flow_keys_access(int off, int size, enum bpf_access_type t,
^
net/core/filter.c:8334:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int check_flow_keys_access(int off, int size, enum bpf_access_type t,
^
static
1 warning generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for LOCKDEP
Depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
Selected by
- PROVE_LOCKING && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
- DEBUG_LOCK_ALLOC && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
vim +/check_flow_keys_access +8334 net/core/filter.c
8333
> 8334 int check_flow_keys_access(int off, int size, enum bpf_access_type t,
8335 struct bpf_insn_access_aux *info)
8336 {
8337 if (size < 0 || off < 0 ||
8338 (u64)off + size > sizeof(struct bpf_flow_keys))
8339 return -EACCES;
8340
8341 switch (off) {
8342 case bpf_ctx_range_ptr(struct bpf_flow_keys, vhdr):
8343 if (t == BPF_WRITE || off % size != 0 || size != sizeof(__u64))
8344 return -EACCES;
8345
8346 if (!bpf_flow_dissector_btf_ids[0])
8347 return -EINVAL;
8348
8349 info->btf_id = bpf_flow_dissector_btf_ids[0];
8350
8351 break;
8352 case offsetof(struct bpf_flow_keys, vhdr_is_little_endian):
8353 if (t == BPF_WRITE)
8354 return -EACCES;
8355
8356 break;
8357 }
8358
8359 return 0;
8360 }
8361
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 43679 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-06-16 17:22 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 0:10 [PATCH net-next v6 0/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
2021-06-15 0:10 ` [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr Tanner Love
2021-06-15 22:25 ` Martin KaFai Lau
2021-06-15 23:50 ` Tanner Love
2021-06-16 0:12 ` Martin KaFai Lau
2021-06-16 15:49 ` kernel test robot
2021-06-16 15:49 ` kernel test robot
2021-06-16 17:21 ` kernel test robot
2021-06-16 17:21 ` kernel test robot
2021-06-15 0:10 ` [PATCH net-next v6 2/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
2021-06-15 0:11 ` [PATCH net-next v6 3/3] selftests/net: amend bpf flow dissector prog to do vnet hdr validation Tanner Love
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.