From patchwork Thu Feb 13 15:20:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kroah-Hartman X-Patchwork-Id: 231461 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=-6.8 required=3.0 tests=DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, 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 1CBA1C2BA83 for ; Thu, 13 Feb 2020 15:42:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E5C0020661 for ; Thu, 13 Feb 2020 15:42:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1581608526; bh=0Wyzm8prTXdxcC5qKMuthwEvTdxOmhJ2ohgnlYe8qGk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=2NeKAbW3iAgqK5x5agw92WOeZykms1OFDTn7eZdgK28SR6kip6UDqseGRs+cyUFnE 5M8KLcS3Ppwtv950lPLV6IQoLi3vpa4MvbyoH3/8ZvYDvuZb3cDCq0x7XrFqENmKYM dvd63zNKVa6bjf02zRTHOK9X+l6iZK7eBwXiHP8A= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387472AbgBMP2U (ORCPT ); Thu, 13 Feb 2020 10:28:20 -0500 Received: from mail.kernel.org ([198.145.29.99]:55238 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728591AbgBMP2T (ORCPT ); Thu, 13 Feb 2020 10:28:19 -0500 Received: from localhost (unknown [104.132.1.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id A39862168B; Thu, 13 Feb 2020 15:28:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1581607698; bh=0Wyzm8prTXdxcC5qKMuthwEvTdxOmhJ2ohgnlYe8qGk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZHmv6xJtHmJXDBhsSEtUU6lvlBC6SYDgYQFVPEvjM132H1ugyN74w6Tg7wkD2D8zU POyMl6fDyzCGsaQ7OsZFOsmzATK2FMlM03s9D9HXaTGdw+KG3nvP5kYXfvZnf0Qc7P cZ/1xAJnbFzQcWi/KTAHUBjNwlMsG1oZNt7dNiWc= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Lorenz Bauer , Daniel Borkmann , Jakub Sitnicki Subject: [PATCH 5.5 041/120] bpf, sockmap: Check update requirements after locking Date: Thu, 13 Feb 2020 07:20:37 -0800 Message-Id: <20200213151915.720570765@linuxfoundation.org> X-Mailer: git-send-email 2.25.0 In-Reply-To: <20200213151901.039700531@linuxfoundation.org> References: <20200213151901.039700531@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Lorenz Bauer commit 85b8ac01a421791d66c3a458a7f83cfd173fe3fa upstream. 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: Greg Kroah-Hartman --- net/core/sock_map.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -417,14 +417,16 @@ static int sock_map_update_elem(struct b 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 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);