Netdev Archive on lore.kernel.org
 help / color / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: John Fastabend <john.fastabend@gmail.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Sasha Levin <sashal@kernel.org>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: [PATCH AUTOSEL 5.2 025/123] bpf: sockmap, sock_map_delete needs to use xchg
Date: Tue, 13 Aug 2019 22:09:09 -0400
Message-ID: <20190814021047.14828-25-sashal@kernel.org> (raw)
In-Reply-To: <20190814021047.14828-1-sashal@kernel.org>

From: John Fastabend <john.fastabend@gmail.com>

[ Upstream commit 45a4521dcbd92e71c9e53031b40e34211d3b4feb ]

__sock_map_delete() may be called from a tcp event such as unhash or
close from the following trace,

  tcp_bpf_close()
    tcp_bpf_remove()
      sk_psock_unlink()
        sock_map_delete_from_link()
          __sock_map_delete()

In this case the sock lock is held but this only protects against
duplicate removals on the TCP side. If the map is free'd then we have
this trace,

  sock_map_free
    xchg()                  <- replaces map entry
    sock_map_unref()
      sk_psock_put()
        sock_map_del_link()

The __sock_map_delete() call however uses a read, test, null over the
map entry which can result in both paths trying to free the map
entry.

To fix use xchg in TCP paths as well so we avoid having two references
to the same map entry.

Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/core/sock_map.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index be6092ac69f8a..1d40e040320d2 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -281,16 +281,20 @@ static int __sock_map_delete(struct bpf_stab *stab, struct sock *sk_test,
 			     struct sock **psk)
 {
 	struct sock *sk;
+	int err = 0;
 
 	raw_spin_lock_bh(&stab->lock);
 	sk = *psk;
 	if (!sk_test || sk_test == sk)
-		*psk = NULL;
+		sk = xchg(psk, NULL);
+
+	if (likely(sk))
+		sock_map_unref(sk, psk);
+	else
+		err = -EINVAL;
+
 	raw_spin_unlock_bh(&stab->lock);
-	if (unlikely(!sk))
-		return -EINVAL;
-	sock_map_unref(sk, psk);
-	return 0;
+	return err;
 }
 
 static void sock_map_delete_from_link(struct bpf_map *map, struct sock *sk,
-- 
2.20.1


  parent reply index

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190814021047.14828-1-sashal@kernel.org>
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 020/123] libbpf: sanitize VAR to conservative 1-byte INT Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 021/123] netfilter: ebtables: fix a memory leak bug in compat Sasha Levin
2019-08-14  2:09 ` Sasha Levin [this message]
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 026/123] bpf: sockmap, synchronize_rcu before free'ing map Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 027/123] bpf: sockmap, only create entry if ulp is not already enabled Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 028/123] selftests/bpf: fix sendmsg6_prog on s390 Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 030/123] bonding: Force slave speed check after link state recovery for 802.3ad Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 031/123] net: mvpp2: Don't check for 3 consecutive Idle frames for 10G links Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 032/123] selftests: forwarding: gre_multipath: Enable IPv4 forwarding Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 033/123] selftests: forwarding: gre_multipath: Fix flower filters Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 034/123] bpf: fix access to skb_shared_info->gso_segs Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 035/123] selftests/bpf: add another gso_segs access Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 036/123] libbpf: fix using uninitialized ioctl results Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 037/123] can: dev: call netif_carrier_off() in register_candev() Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 038/123] can: mcp251x: add error check when wq alloc failed Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 039/123] can: gw: Fix error path of cgw_module_init Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 042/123] libbpf: silence GCC8 warning about string truncation Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 043/123] st21nfca_connectivity_event_received: null check the allocation Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 044/123] st_nci_hci_connectivity_event_received: " Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 045/123] {nl,mac}80211: fix interface combinations on crypto controlled devices Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 049/123] net: usb: qmi_wwan: Add the BroadMobi BM818 card Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 050/123] qed: RDMA - Fix the hw_ver returned in device attributes Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 051/123] isdn: mISDN: hfcsusb: Fix possible null-pointer dereferences in start_isoc_chain() Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 053/123] mac80211_hwsim: Fix possible null-pointer dereferences in hwsim_dump_radio_nl() Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 054/123] net: stmmac: manage errors returned by of_get_mac_address() Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 055/123] netfilter: ipset: Actually allow destination MAC address for hash:ip,mac sets too Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 056/123] netfilter: ipset: Copy the right MAC address in bitmap:ip,mac and hash:ip,mac sets Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 057/123] netfilter: ipset: Fix rename concurrency with listing Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 058/123] netfilter: ebtables: also count base chain policies Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 059/123] rxrpc: Fix potential deadlock Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 060/123] rxrpc: Fix the lack of notification when sendmsg() fails on a DATA packet Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 062/123] iwlwifi: mvm: avoid races in rate init and rate perform Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 063/123] iwlwifi: dbg_ini: move iwl_dbg_tlv_load_bin out of debug override ifdef Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 064/123] iwlwifi: dbg_ini: move iwl_dbg_tlv_free outside of debugfs ifdef Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 065/123] iwlwifi: fix locking in delayed GTK setting Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 066/123] iwlwifi: mvm: send LQ command always ASYNC Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 068/123] enetc: Fix build error without PHYLIB Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 069/123] isdn: hfcsusb: Fix mISDN driver crash caused by transfer buffer on the stack Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 070/123] net: phy: phy_led_triggers: Fix a possible null-pointer dereference in phy_led_trigger_change_speed() Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 073/123] net: usb: pegasus: fix improper read if get_registers() fail Sasha Levin
2019-08-14  2:09 ` [PATCH AUTOSEL 5.2 075/123] can: sja1000: force the string buffer NULL-terminated Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 076/123] can: peak_usb: " Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 078/123] net/ethernet/qlogic/qed: " Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 079/123] enetc: Select PHYLIB while CONFIG_FSL_ENETC_VF is set Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 102/123] net: cxgb3_main: Fix a resource leak in a error path in 'init_one()' Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 103/123] net: stmmac: Fix issues when number of Queues >= 4 Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 104/123] net: stmmac: tc: Do not return a fragment entry Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 106/123] net: hisilicon: make hip04_tx_reclaim non-reentrant Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 107/123] net: hisilicon: fix hip04-xmit never return TX_BUSY Sasha Levin
2019-08-14  2:10 ` [PATCH AUTOSEL 5.2 108/123] net: hisilicon: Fix dma_map_single failed on arm64 Sasha Levin

Reply instructions:

You may reply publically 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=20190814021047.14828-25-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.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

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox