From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8A6D2C3B1B9 for ; Fri, 14 Feb 2020 18:15:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6000E2187F for ; Fri, 14 Feb 2020 18:15:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1581704139; bh=SsqKa/ZjaIyluXId/sG0bPe98/YPUtYaCn6TggH3Gso=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=CKZec7lulA0hCYlZE3LZCdLCWsKp/hyk8GtkoSZzRvGvmjNzLRWePPF1B8v8+rmuU m7Jc3+aGS9SD7q5ileWKItXbE6iyKJL/l48/f05mT+4m+JAv7EdpFETGoWifEuMwTG 3ygXhPeME1B2Pt8dO4Wb3wiCl+k+IdPpZRmD/KFw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730880AbgBNPvw (ORCPT ); Fri, 14 Feb 2020 10:51:52 -0500 Received: from mail.kernel.org ([198.145.29.99]:57086 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730868AbgBNPvv (ORCPT ); Fri, 14 Feb 2020 10:51:51 -0500 Received: from sasha-vm.mshome.net (c-73-47-72-35.hsd1.nh.comcast.net [73.47.72.35]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B28BF24649; Fri, 14 Feb 2020 15:51:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1581695510; bh=SsqKa/ZjaIyluXId/sG0bPe98/YPUtYaCn6TggH3Gso=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AtvGi7v5fOYZ29toUQ85qH3sIJ6hzq387qDqf/LRxus4gE0CbS7j1izda2BVLPnE+ 7hTds4TURcIlNHY5iGEl2BVfhPV50MtyGAj91N01qlsBAyDvbZhMSm3DvZFieQS8oL /IFACnDS3l1RD5MkwfLu1ubNoJS/hHvE4MQMmtp8= From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Lorenz Bauer , Daniel Borkmann , Jakub Sitnicki , Sasha Levin , netdev@vger.kernel.org, bpf@vger.kernel.org Subject: [PATCH AUTOSEL 5.5 135/542] bpf, sockmap: Check update requirements after locking Date: Fri, 14 Feb 2020 10:42:07 -0500 Message-Id: <20200214154854.6746-135-sashal@kernel.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200214154854.6746-1-sashal@kernel.org> References: <20200214154854.6746-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Lorenz Bauer [ Upstream commit 85b8ac01a421791d66c3a458a7f83cfd173fe3fa ] It's currently possible to insert sockets in unexpected states into a sockmap, due to a TOCTTOU when updating the map from a syscall. sock_map_update_elem checks that sk->sk_state == TCP_ESTABLISHED, locks the socket and then calls sock_map_update_common. At this point, the socket may have transitioned into another state, and the earlier assumptions don't hold anymore. Crucially, it's conceivable (though very unlikely) that a socket has become unhashed. This breaks the sockmap's assumption that it will get a callback via sk->sk_prot->unhash. Fix this by checking the (fixed) sk_type and sk_protocol without the lock, followed by a locked check of sk_state. Unfortunately it's not possible to push the check down into sock_(map|hash)_update_common, since BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB run before the socket has transitioned from TCP_SYN_RECV into TCP_ESTABLISHED. Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface") Signed-off-by: Lorenz Bauer Signed-off-by: Daniel Borkmann Reviewed-by: Jakub Sitnicki Link: https://lore.kernel.org/bpf/20200207103713.28175-1-lmb@cloudflare.com Signed-off-by: Sasha Levin --- net/core/sock_map.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/net/core/sock_map.c b/net/core/sock_map.c index 058422b932607..b16ff3b8c6503 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -417,14 +417,16 @@ static int sock_map_update_elem(struct bpf_map *map, void *key, ret = -EINVAL; goto out; } - if (!sock_map_sk_is_suitable(sk) || - sk->sk_state != TCP_ESTABLISHED) { + if (!sock_map_sk_is_suitable(sk)) { ret = -EOPNOTSUPP; goto out; } sock_map_sk_acquire(sk); - ret = sock_map_update_common(map, idx, sk, flags); + if (sk->sk_state != TCP_ESTABLISHED) + ret = -EOPNOTSUPP; + else + ret = sock_map_update_common(map, idx, sk, flags); sock_map_sk_release(sk); out: fput(sock->file); @@ -740,14 +742,16 @@ static int sock_hash_update_elem(struct bpf_map *map, void *key, ret = -EINVAL; goto out; } - if (!sock_map_sk_is_suitable(sk) || - sk->sk_state != TCP_ESTABLISHED) { + if (!sock_map_sk_is_suitable(sk)) { ret = -EOPNOTSUPP; goto out; } sock_map_sk_acquire(sk); - ret = sock_hash_update_common(map, key, sk, flags); + if (sk->sk_state != TCP_ESTABLISHED) + ret = -EOPNOTSUPP; + else + ret = sock_hash_update_common(map, key, sk, flags); sock_map_sk_release(sk); out: fput(sock->file); -- 2.20.1