From 0b8c707ddf37171413fe67350263e5b6ffeedf7c Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Thu, 19 Mar 2015 19:38:27 +0100 Subject: [PATCH] ebpf, filter: do not convert skb->protocol to host endianess during runtime Commit c24973957975 ("bpf: allow BPF programs access 'protocol' and 'vlan_tci' fields") has added support for accessing protocol, vlan_present and vlan_tci into the skb offset map. As referenced in the below discussion, accessing skb->protocol from an eBPF program should be converted without handling endianess. The reason for this is that an eBPF program could simply do a check more naturally, by f.e. testing skb->protocol == htons(ETH_P_IP), where the LLVM compiler resolves htons() against a constant automatically during compilation time, as opposed to an otherwise needed run time conversion. After all, the way of programming both from a user perspective differs quite a lot, i.e. bpf_asm ["ld proto"] versus a C subset/LLVM. Reference: https://patchwork.ozlabs.org/patch/450819/ Signed-off-by: Daniel Borkmann Acked-by: Alexei Starovoitov Signed-off-by: David S. Miller --- net/core/filter.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/net/core/filter.c b/net/core/filter.c index b95ae7fe7e4f..bdaac5895def 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -178,16 +178,6 @@ static u32 convert_skb_access(int skb_field, int dst_reg, int src_reg, offsetof(struct sk_buff, queue_mapping)); break; - case SKF_AD_PROTOCOL: - BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, protocol) != 2); - - /* dst_reg = *(u16 *) (src_reg + offsetof(protocol)) */ - *insn++ = BPF_LDX_MEM(BPF_H, dst_reg, src_reg, - offsetof(struct sk_buff, protocol)); - /* dst_reg = ntohs(dst_reg) [emitting a nop or swap16] */ - *insn++ = BPF_ENDIAN(BPF_FROM_BE, dst_reg, 16); - break; - case SKF_AD_VLAN_TAG: case SKF_AD_VLAN_TAG_PRESENT: BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, vlan_tci) != 2); @@ -219,8 +209,13 @@ static bool convert_bpf_extensions(struct sock_filter *fp, switch (fp->k) { case SKF_AD_OFF + SKF_AD_PROTOCOL: - cnt = convert_skb_access(SKF_AD_PROTOCOL, BPF_REG_A, BPF_REG_CTX, insn); - insn += cnt - 1; + BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, protocol) != 2); + + /* A = *(u16 *) (CTX + offsetof(protocol)) */ + *insn++ = BPF_LDX_MEM(BPF_H, BPF_REG_A, BPF_REG_CTX, + offsetof(struct sk_buff, protocol)); + /* A = ntohs(A) [emitting a nop or swap16] */ + *insn = BPF_ENDIAN(BPF_FROM_BE, BPF_REG_A, 16); break; case SKF_AD_OFF + SKF_AD_PKTTYPE: @@ -1224,6 +1219,13 @@ static u32 sk_filter_convert_ctx_access(int dst_reg, int src_reg, int ctx_off, offsetof(struct sk_buff, len)); break; + case offsetof(struct __sk_buff, protocol): + BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, protocol) != 2); + + *insn++ = BPF_LDX_MEM(BPF_H, dst_reg, src_reg, + offsetof(struct sk_buff, protocol)); + break; + case offsetof(struct __sk_buff, mark): return convert_skb_access(SKF_AD_MARK, dst_reg, src_reg, insn); @@ -1233,9 +1235,6 @@ static u32 sk_filter_convert_ctx_access(int dst_reg, int src_reg, int ctx_off, case offsetof(struct __sk_buff, queue_mapping): return convert_skb_access(SKF_AD_QUEUE, dst_reg, src_reg, insn); - case offsetof(struct __sk_buff, protocol): - return convert_skb_access(SKF_AD_PROTOCOL, dst_reg, src_reg, insn); - case offsetof(struct __sk_buff, vlan_present): return convert_skb_access(SKF_AD_VLAN_TAG_PRESENT, dst_reg, src_reg, insn); -- 2.20.1