netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Martin KaFai Lau <kafai@fb.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Sasha Levin <sashal@kernel.org>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: [PATCH AUTOSEL 4.20 49/81] bpf: Fix syscall's stackmap lookup potential deadlock
Date: Thu, 28 Feb 2019 10:07:41 -0500	[thread overview]
Message-ID: <20190228150813.10256-49-sashal@kernel.org> (raw)
In-Reply-To: <20190228150813.10256-1-sashal@kernel.org>

From: Martin KaFai Lau <kafai@fb.com>

[ Upstream commit 7c4cd051add3d00bbff008a133c936c515eaa8fe ]

The map_lookup_elem used to not acquiring spinlock
in order to optimize the reader.

It was true until commit 557c0c6e7df8 ("bpf: convert stackmap to pre-allocation")
The syscall's map_lookup_elem(stackmap) calls bpf_stackmap_copy().
bpf_stackmap_copy() may find the elem no longer needed after the copy is done.
If that is the case, pcpu_freelist_push() saves this elem for reuse later.
This push requires a spinlock.

If a tracing bpf_prog got run in the middle of the syscall's
map_lookup_elem(stackmap) and this tracing bpf_prog is calling
bpf_get_stackid(stackmap) which also requires the same pcpu_freelist's
spinlock, it may end up with a dead lock situation as reported by
Eric Dumazet in https://patchwork.ozlabs.org/patch/1030266/

The situation is the same as the syscall's map_update_elem() which
needs to acquire the pcpu_freelist's spinlock and could race
with tracing bpf_prog.  Hence, this patch fixes it by protecting
bpf_stackmap_copy() with this_cpu_inc(bpf_prog_active)
to prevent tracing bpf_prog from running.

A later syscall's map_lookup_elem commit f1a2e44a3aec ("bpf: add queue and stack maps")
also acquires a spinlock and races with tracing bpf_prog similarly.
Hence, this patch is forward looking and protects the majority
of the map lookups.  bpf_map_offload_lookup_elem() is the exception
since it is for network bpf_prog only (i.e. never called by tracing
bpf_prog).

Fixes: 557c0c6e7df8 ("bpf: convert stackmap to pre-allocation")
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 kernel/bpf/syscall.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cf5040fd54344..5f03ddf7b6152 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -712,8 +712,13 @@ static int map_lookup_elem(union bpf_attr *attr)
 
 	if (bpf_map_is_dev_bound(map)) {
 		err = bpf_map_offload_lookup_elem(map, key, value);
-	} else if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
-		   map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH) {
+		goto done;
+	}
+
+	preempt_disable();
+	this_cpu_inc(bpf_prog_active);
+	if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
+	    map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH) {
 		err = bpf_percpu_hash_copy(map, key, value);
 	} else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
 		err = bpf_percpu_array_copy(map, key, value);
@@ -743,7 +748,10 @@ static int map_lookup_elem(union bpf_attr *attr)
 		}
 		rcu_read_unlock();
 	}
+	this_cpu_dec(bpf_prog_active);
+	preempt_enable();
 
+done:
 	if (err)
 		goto free_value;
 
-- 
2.19.1


  parent reply	other threads:[~2019-02-28 15:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190228150813.10256-1-sashal@kernel.org>
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 14/81] wlcore: sdio: Fixup power on/off sequence Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 15/81] bpftool: Fix prog dump by tag Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 16/81] bpftool: fix percpu maps updating Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 17/81] bpf: sock recvbuff must be limited by rmem_max in bpf_setsockopt() Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 23/81] batman-adv: release station info tidstats Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 29/81] sk_msg: Always cancel strp work before freeing the psock Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 38/81] net: stmmac: Fallback to Platform Data clock in Watchdog conversion Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 39/81] net: stmmac: Send TSO packets always from Queue 0 Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 40/81] net: stmmac: Disable EEE mode earlier in XMIT callback Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 44/81] ath10k: correct bus type for WCN3990 Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 45/81] bpf, selftests: fix handling of sparse CPU allocations Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 46/81] bpf: run bpf programs with preemption disabled Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 47/81] bpf: fix lockdep false positive in percpu_freelist Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 48/81] bpf: fix potential deadlock in bpf_prog_register Sasha Levin
2019-02-28 15:07 ` Sasha Levin [this message]
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 54/81] vsock/virtio: fix kernel panic after device hot-unplug Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 55/81] vsock/virtio: reset connected sockets on device removal Sasha Levin
2019-02-28 15:07 ` [PATCH AUTOSEL 4.20 59/81] netfilter: nf_nat: skip nat clash resolution for same-origin entries Sasha Levin
2019-02-28 15:08 ` [PATCH AUTOSEL 4.20 78/81] qed: Fix EQ full firmware assert Sasha Levin
2019-02-28 15:08 ` [PATCH AUTOSEL 4.20 79/81] qed: Consider TX tcs while deriving the max num_queues for PF Sasha Levin
2019-02-28 15:08 ` [PATCH AUTOSEL 4.20 80/81] qede: Fix system crash on configuring channels Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190228150813.10256-49-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kafai@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).