--- /dev/null
+From 6bdf6abc56b53103324dfd270a86580306e1a232 Mon Sep 17 00:00:00 2001
+From: Daniel Borkmann <daniel@iogearbox.net>
+Date: Thu, 29 Jun 2017 03:04:59 +0200
+Subject: bpf: prevent leaking pointer via xadd on unpriviledged
+
+From: Daniel Borkmann <daniel@iogearbox.net>
+
+commit 6bdf6abc56b53103324dfd270a86580306e1a232 upstream.
+
+Leaking kernel addresses on unpriviledged is generally disallowed,
+for example, verifier rejects the following:
+
+ 0: (b7) r0 = 0
+ 1: (18) r2 = 0xffff897e82304400
+ 3: (7b) *(u64 *)(r1 +48) = r2
+ R2 leaks addr into ctx
+
+Doing pointer arithmetic on them is also forbidden, so that they
+don't turn into unknown value and then get leaked out. However,
+there's xadd as a special case, where we don't check the src reg
+for being a pointer register, e.g. the following will pass:
+
+ 0: (b7) r0 = 0
+ 1: (7b) *(u64 *)(r1 +48) = r0
+ 2: (18) r2 = 0xffff897e82304400 ; map
+ 4: (db) lock *(u64 *)(r1 +48) += r2
+ 5: (95) exit
+
+We could store the pointer into skb->cb, loose the type context,
+and then read it out from there again to leak it eventually out
+of a map value. Or more easily in a different variant, too:
+
+ 0: (bf) r6 = r1
+ 1: (7a) *(u64 *)(r10 -8) = 0
+ 2: (bf) r2 = r10
+ 3: (07) r2 += -8
+ 4: (18) r1 = 0x0
+ 6: (85) call bpf_map_lookup_elem#1
+ 7: (15) if r0 == 0x0 goto pc+3
+ R0=map_value(ks=8,vs=8,id=0),min_value=0,max_value=0 R6=ctx R10=fp
+ 8: (b7) r3 = 0
+ 9: (7b) *(u64 *)(r0 +0) = r3
+ 10: (db) lock *(u64 *)(r0 +0) += r6
+ 11: (b7) r0 = 0
+ 12: (95) exit
+
+ from 7 to 11: R0=inv,min_value=0,max_value=0 R6=ctx R10=fp
+ 11: (b7) r0 = 0
+ 12: (95) exit
+
+Prevent this by checking xadd src reg for pointer types. Also
+add a couple of test cases related to this.
+
+Fixes: 1be7f75d1668 ("bpf: enable non-root eBPF programs")
+Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
+Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
+Acked-by: Alexei Starovoitov <ast@kernel.org>
+Acked-by: Martin KaFai Lau <kafai@fb.com>
+Acked-by: Edward Cree <ecree@solarflare.com>
+Signed-off-by: David S. Miller <davem@davemloft.net>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ kernel/bpf/verifier.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+--- a/kernel/bpf/verifier.c
++++ b/kernel/bpf/verifier.c
+@@ -754,6 +754,11 @@ static int check_xadd(struct verifier_en
+ if (err)
+ return err;
+
++ if (is_pointer_value(env, insn->src_reg)) {
++ verbose("R%d leaks addr into mem\n", insn->src_reg);
++ return -EACCES;
++ }
++
+ /* check whether atomic_add can read the memory */
+ err = check_mem_access(env, insn->dst_reg, insn->off,
+ BPF_SIZE(insn->code), BPF_READ, -1);
--- /dev/null
+From e44699d2c28067f69698ccb68dd3ddeacfebc434 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Michal=20Kube=C4=8Dek?= <mkubecek@suse.cz>
+Date: Thu, 29 Jun 2017 11:13:36 +0200
+Subject: net: handle NAPI_GRO_FREE_STOLEN_HEAD case also in napi_frags_finish()
+
+From: Michal Kubeček <mkubecek@suse.cz>
+
+commit e44699d2c28067f69698ccb68dd3ddeacfebc434 upstream.
+
+Recently I started seeing warnings about pages with refcount -1. The
+problem was traced to packets being reused after their head was merged into
+a GRO packet by skb_gro_receive(). While bisecting the issue pointed to
+commit c21b48cc1bbf ("net: adjust skb->truesize in ___pskb_trim()") and
+I have never seen it on a kernel with it reverted, I believe the real
+problem appeared earlier when the option to merge head frag in GRO was
+implemented.
+
+Handling NAPI_GRO_FREE_STOLEN_HEAD state was only added to GRO_MERGED_FREE
+branch of napi_skb_finish() so that if the driver uses napi_gro_frags()
+and head is merged (which in my case happens after the skb_condense()
+call added by the commit mentioned above), the skb is reused including the
+head that has been merged. As a result, we release the page reference
+twice and eventually end up with negative page refcount.
+
+To fix the problem, handle NAPI_GRO_FREE_STOLEN_HEAD in napi_frags_finish()
+the same way it's done in napi_skb_finish().
+
+Fixes: d7e8883cfcf4 ("net: make GRO aware of skb->head_frag")
+Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
+Signed-off-by: David S. Miller <davem@davemloft.net>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ net/core/dev.c | 22 ++++++++++++++++------
+ 1 file changed, 16 insertions(+), 6 deletions(-)
+
+--- a/net/core/dev.c
++++ b/net/core/dev.c
+@@ -4375,6 +4375,12 @@ struct packet_offload *gro_find_complete
+ }
+ EXPORT_SYMBOL(gro_find_complete_by_type);
+
++static void napi_skb_free_stolen_head(struct sk_buff *skb)
++{
++ skb_dst_drop(skb);
++ kmem_cache_free(skbuff_head_cache, skb);
++}
++
+ static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
+ {
+ switch (ret) {
+@@ -4388,12 +4394,10 @@ static gro_result_t napi_skb_finish(gro_
+ break;
+
+ case GRO_MERGED_FREE:
+- if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD) {
+- skb_dst_drop(skb);
+- kmem_cache_free(skbuff_head_cache, skb);
+- } else {
++ if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD)
++ napi_skb_free_stolen_head(skb);
++ else
+ __kfree_skb(skb);
+- }
+ break;
+
+ case GRO_HELD:
+@@ -4459,10 +4463,16 @@ static gro_result_t napi_frags_finish(st
+ break;
+
+ case GRO_DROP:
+- case GRO_MERGED_FREE:
+ napi_reuse_skb(napi, skb);
+ break;
+
++ case GRO_MERGED_FREE:
++ if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD)
++ napi_skb_free_stolen_head(skb);
++ else
++ napi_reuse_skb(napi, skb);
++ break;
++
+ case GRO_MERGED:
+ break;
+ }
projects / thirdparty / kernel / stable-queue.git / commitdiff
