From patchwork Mon Jul 8 19:13:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1129209 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: incoming-bpf@patchwork.ozlabs.org Delivered-To: patchwork-incoming-bpf@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=bpf-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="qoDiqNov"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45jFVc6883z9sNF for ; Tue, 9 Jul 2019 05:14:00 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731584AbfGHTOA (ORCPT ); Mon, 8 Jul 2019 15:14:00 -0400 Received: from mail-io1-f68.google.com ([209.85.166.68]:43626 "EHLO mail-io1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728827AbfGHTOA (ORCPT ); Mon, 8 Jul 2019 15:14:00 -0400 Received: by mail-io1-f68.google.com with SMTP id k20so37738549ios.10; Mon, 08 Jul 2019 12:13:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=OkBt5UDkvteL6zmVGFv0ClYVAZZcXsFBdk1jOy9OQfg=; b=qoDiqNovyJm9oUZmEkXMWQRQPntJpG3eyaSkeRLsk1CC2+QJI6teWKbpwjZ1f3yI/v RxhVGPxCw8gOnq3T0bCcOO5yHqjgxG2jP4rvrZv0kT96pAssyobP6opZ5fQij/PBUFG4 xWXt77ER7jRWCfwlcMFCCFZ8NggKN3Xl9v8R8pE1aW2I/QfK31bfItwbEOJ1wn8e4cvh 9qwVFyJVBE9FRR3ie6koCjSV8ruwA1tFgWlyN3JOgy7qlGoSgYqux/l9WJIUycQs2MAz opk809quzD60GavPuLAImcUN+6dco0wnyP1t0Qn1wW14hwbEMgGpAvdCQosjHiv6ecIc k0Sw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=OkBt5UDkvteL6zmVGFv0ClYVAZZcXsFBdk1jOy9OQfg=; b=CGCzWmO95vRrKLDna6c+aUqj6leZSacpADWBvIw9lmDSHTO2PiIW9YJJNY1a7DSk3E vu7I5kY8TsPOfAvXCmgpwz7sBumJxUcZ35TyA3+QZwY/0WrlyT8DmkPjzIK2EZKhopak eAepWVMayHfIOjs5uhFhkp9HojqACY10kswscCArQyM4ivyXaaGyq3zLFge1J5N9l/i2 KMr9fasTbQOh4ZH84+RHZR0HzpU+6Y9sX1ZyjBGG+3gsZ0YndNi1OXY0PZ+5/uBkUxMf ecnOn9xxs6mtqvHgFhQNQJpE9gpl12PIgBOAS3tzJX++DeOjR8uxVhnErmZCFkJ9OgKM VsEA== X-Gm-Message-State: APjAAAWxqcijZeWVWBNeq7afEs6gSSX2XdUZ0MRlmKzlp192ThAX8+gK YpopS9cFj6tJfzjHz3mZ2bs= X-Google-Smtp-Source: APXvYqwA8U7ZwZ92WXuvKM4M1uPu17rs5y7Vmsjj7iRvtjYgjXg01k2pnri0ORVGN3yxYE95PFwjoA== X-Received: by 2002:a02:3904:: with SMTP id l4mr540217jaa.81.1562613239387; Mon, 08 Jul 2019 12:13:59 -0700 (PDT) Received: from [127.0.1.1] ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id s10sm8804137iod.46.2019.07.08.12.13.52 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 08 Jul 2019 12:13:58 -0700 (PDT) Subject: [bpf PATCH v2 1/6] tls: remove close callback sock unlock/lock and flush_sync From: John Fastabend To: jakub.kicinski@netronome.com, ast@kernel.org, daniel@iogearbox.net Cc: netdev@vger.kernel.org, edumazet@google.com, john.fastabend@gmail.com, bpf@vger.kernel.org Date: Mon, 08 Jul 2019 19:13:47 +0000 Message-ID: <156261322680.31108.7327349397950347146.stgit@ubuntu3-kvm1> In-Reply-To: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> References: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org The tls close() callback currently drops the sock lock, makes a cancel_delayed_work_sync() call, and then relocks the sock. The same pattern is used again to call strp_done(). By restructuring the code we can avoid droping lock and then reclaiming it. To simplify this we do the following, tls_sk_proto_close set_bit(CLOSING) set_bit(SCHEDULE) cancel_delay_work_sync() <- cancel workqueue lock_sock(sk) ... release_sock(sk) strp_done() Setting the CLOSING bit prevents the SCHEDULE bit from being cleared by any workqueue items e.g. if one happens to be scheduled and run between when we set SCHEDULE bit and cancel work. Then because SCHEDULE bit is set now no new work will be scheduled. Then strp_done() is called after the sk work is completed. Any outstanding work is sync'd and finally ctx is free'd. Tested with net selftests and bpf selftests. Signed-off-by: John Fastabend --- include/net/tls.h | 4 ++-- net/tls/tls_main.c | 56 ++++++++++++++++++++++++++-------------------------- net/tls/tls_sw.c | 38 ++++++++++++++++++++++------------- 3 files changed, 54 insertions(+), 44 deletions(-) diff --git a/include/net/tls.h b/include/net/tls.h index 63e473420b00..0a3540a6304d 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -107,9 +107,7 @@ struct tls_device { enum { TLS_BASE, TLS_SW, -#ifdef CONFIG_TLS_DEVICE TLS_HW, -#endif TLS_HW_RECORD, TLS_NUM_CONFIG, }; @@ -162,6 +160,7 @@ struct tls_sw_context_tx { int async_capable; #define BIT_TX_SCHEDULED 0 +#define BIT_TX_CLOSING 1 unsigned long tx_bitmask; }; @@ -361,6 +360,7 @@ void tls_sw_close(struct sock *sk, long timeout); void tls_sw_free_resources_tx(struct sock *sk); void tls_sw_free_resources_rx(struct sock *sk); void tls_sw_release_resources_rx(struct sock *sk); +void tls_sw_release_strp_rx(struct tls_context *tls_ctx); int tls_sw_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int nonblock, int flags, int *addr_len); bool tls_sw_stream_read(const struct sock *sk); diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index fc81ae18cc44..d63c3583d2f7 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -261,24 +261,9 @@ static void tls_ctx_free(struct tls_context *ctx) kfree(ctx); } -static void tls_sk_proto_close(struct sock *sk, long timeout) +static void tls_sk_proto_cleanup(struct sock *sk, + struct tls_context *ctx, long timeo) { - struct tls_context *ctx = tls_get_ctx(sk); - long timeo = sock_sndtimeo(sk, 0); - void (*sk_proto_close)(struct sock *sk, long timeout); - bool free_ctx = false; - - lock_sock(sk); - sk_proto_close = ctx->sk_proto_close; - - if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD) - goto skip_tx_cleanup; - - if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) { - free_ctx = true; - goto skip_tx_cleanup; - } - if (!tls_complete_pending_work(sk, ctx, 0, &timeo)) tls_handle_open_record(sk, 0); @@ -299,22 +284,37 @@ static void tls_sk_proto_close(struct sock *sk, long timeout) #ifdef CONFIG_TLS_DEVICE if (ctx->rx_conf == TLS_HW) tls_device_offload_cleanup_rx(sk); - - if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW) { -#else - { #endif - tls_ctx_free(ctx); - ctx = NULL; - } +} + +static void tls_sk_proto_close(struct sock *sk, long timeout) +{ + struct tls_context *ctx = tls_get_ctx(sk); + long timeo = sock_sndtimeo(sk, 0); + void (*sk_proto_close)(struct sock *sk, long timeout); + + if (ctx->tx_conf == TLS_SW) + tls_sw_cancel_work_tx(ctx); + + lock_sock(sk); + sk_proto_close = ctx->sk_proto_close; + + if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD) + goto skip_tx_cleanup; + + if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) + goto skip_tx_cleanup; + + tls_sk_proto_cleanup(sk, ctx, timeo); skip_tx_cleanup: release_sock(sk); + if (ctx->rx_conf == TLS_SW) + tls_sw_release_strp_rx(ctx); sk_proto_close(sk, timeout); - /* free ctx for TLS_HW_RECORD, used by tcp_set_state - * for sk->sk_prot->unhash [tls_hw_unhash] - */ - if (free_ctx) + + if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW && + ctx->tx_conf != TLS_HW_RECORD && ctx->rx_conf != TLS_HW_RECORD) tls_ctx_free(ctx); } diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index db585964b52b..3b01cd72ca6c 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2053,6 +2053,15 @@ static void tls_data_ready(struct sock *sk) } } +void tls_sw_cancel_work_tx(struct tls_context *tls_ctx) +{ + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx); + + set_bit(BIT_TX_CLOSING, &ctx->tx_bitmask); + set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask); + cancel_delayed_work_sync(&ctx->tx_work.work); +} + void tls_sw_free_resources_tx(struct sock *sk) { struct tls_context *tls_ctx = tls_get_ctx(sk); @@ -2064,11 +2073,6 @@ void tls_sw_free_resources_tx(struct sock *sk) if (atomic_read(&ctx->encrypt_pending)) crypto_wait_req(-EINPROGRESS, &ctx->async_wait); - release_sock(sk); - cancel_delayed_work_sync(&ctx->tx_work.work); - lock_sock(sk); - - /* Tx whatever records we can transmit and abandon the rest */ tls_tx_records(sk, -1); /* Free up un-sent records in tx_list. First, free @@ -2112,22 +2116,22 @@ void tls_sw_release_resources_rx(struct sock *sk) write_lock_bh(&sk->sk_callback_lock); sk->sk_data_ready = ctx->saved_data_ready; write_unlock_bh(&sk->sk_callback_lock); - release_sock(sk); - strp_done(&ctx->strp); - lock_sock(sk); } } -void tls_sw_free_resources_rx(struct sock *sk) +void tls_sw_release_strp_rx(struct tls_context *tls_ctx) { - struct tls_context *tls_ctx = tls_get_ctx(sk); struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx); - tls_sw_release_resources_rx(sk); - + strp_done(&ctx->strp); kfree(ctx); } +void tls_sw_free_resources_rx(struct sock *sk) +{ + tls_sw_release_resources_rx(sk); +} + /* The work handler to transmitt the encrypted records in tx_list */ static void tx_work_handler(struct work_struct *work) { @@ -2136,11 +2140,17 @@ static void tx_work_handler(struct work_struct *work) struct tx_work, work); struct sock *sk = tx_work->sk; struct tls_context *tls_ctx = tls_get_ctx(sk); - struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx); + struct tls_sw_context_tx *ctx; - if (!test_and_clear_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask)) + if (unlikely(!tls_ctx)) + return; + + ctx = tls_sw_ctx_tx(tls_ctx); + if (test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask)) return; + if (!test_and_clear_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask)) + return; lock_sock(sk); tls_tx_records(sk, -1); release_sock(sk); From patchwork Mon Jul 8 19:14:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1129211 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="LZVUkVTj"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45jFVz2S1mz9sNF for ; Tue, 9 Jul 2019 05:14:19 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391503AbfGHTOS (ORCPT ); Mon, 8 Jul 2019 15:14:18 -0400 Received: from mail-io1-f67.google.com ([209.85.166.67]:43697 "EHLO mail-io1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728827AbfGHTOS (ORCPT ); Mon, 8 Jul 2019 15:14:18 -0400 Received: by mail-io1-f67.google.com with SMTP id k20so37740378ios.10; Mon, 08 Jul 2019 12:14:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=hXIgA+39NNFcF6s8vWmNwPku9P6vco1srrmA0dk8IiQ=; b=LZVUkVTjXxiLLVo4olzoO2KCaynPTs+KBAmkmRw90vnekLx8GCNhN21rPPYCuS+1Rq 9cipQgCBTNPAHYXu3ZBUPAlj8nfs9Y5w2ALd3RR6dOmBxb1C/DS10x7lTgM/6isOVCzm 0mn9sBLG9hrJZ2kIOgxgx3ngUd5LIKLt6ETdPFjVg/aO/cCqjmKeu2znWqpFd+PtuRi4 1M8+wpHFIuu4gJKZ/v7fNMQVsrD7Y8pLTfbDWg+0NBAS7LMYMKrlbgy3LhiVe7BQmDAv YuK5usjyYPspILrpWX3U0A10Q+cfhgG2FB+DYpI7QYVJBkucgh7j/05rUYoScvyfJR0u EhCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=hXIgA+39NNFcF6s8vWmNwPku9P6vco1srrmA0dk8IiQ=; b=MUaZmvyji3kLunPJ+la4TFQro86HlF1wk47DwpOyfS6w8VacM5qzTYnvZzXgtPcXg+ YPrLcbfDMkRrBpXQM88zi0AnObVmdfRe92hce7Ia/MRyn7xKbd33NbR1G0rVI3WCoWja o7u2LxFAIkNUW1ls72WD/FCSJz3uT2HWLeD8a70ZZjyTa/LERN2gdKfeBLv6ZX3YdCkN nDPDt0ttVK91pAdS4HDFLX/qx4vC6TX5rZv20XWpJijhnJBA9DA9rys19MKc5cmvFmm3 MszLAYdhLlDTXHowTlTZDbFzu+kbN3Uxe7sM8iTo6+ijzckX6O62SzyC6JasLX1hVbqB PcFw== X-Gm-Message-State: APjAAAWxsrQYR2fTfJc3NrxImlR0w89J6prv4SBnwWNaEnlRc+oARH9u rt3JPGKAx5CBOuztLBrDSc8= X-Google-Smtp-Source: APXvYqxavi6I2MhtcfpvFdaPFH1fV2WxDLS1yOqRVTBz/gjL+DAeAEZq2KNB8Dc0ezMxJ+kjUi+xqg== X-Received: by 2002:a05:6602:2289:: with SMTP id d9mr21319341iod.47.1562613257267; Mon, 08 Jul 2019 12:14:17 -0700 (PDT) Received: from [127.0.1.1] ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id n22sm10030848iob.37.2019.07.08.12.14.10 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 08 Jul 2019 12:14:16 -0700 (PDT) Subject: [bpf PATCH v2 2/6] bpf: tls fix transition through disconnect with close From: John Fastabend To: jakub.kicinski@netronome.com, ast@kernel.org, daniel@iogearbox.net Cc: netdev@vger.kernel.org, edumazet@google.com, john.fastabend@gmail.com, bpf@vger.kernel.org Date: Mon, 08 Jul 2019 19:14:05 +0000 Message-ID: <156261324561.31108.14410711674221391677.stgit@ubuntu3-kvm1> In-Reply-To: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> References: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org It is possible (via shutdown()) for TCP socks to go through TCP_CLOSE state via tcp_dosconnect() without actually calling tcp_close which would then call the tls close callback. Because of this a user could disconnect a socket then put it in a LISTEN state which would break our assumptions about sockets always being ESTABLISHED state. More directly because close() can call unhash() and unhash is implemented by sockmap if a sockmap socket has TLS enabled we can incorrectly destroy the psock from unhash() and then call its close handler again. But because the psock (sockmap socket representation) is already destroyed we call close handler in sk->prot. However, in some cases (TLS BASE/BASE case) this will still point at the sockmap close handler resulting in a circular call and crash reported by syzbot. To fix both above issues implement the unhash() routine for TLS. Fixes: 3c4d7559159bf ("tls: kernel TLS support") Reported-by: Eric Dumazet Signed-off-by: John Fastabend --- include/net/tls.h | 6 +++++- net/tls/tls_main.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/include/net/tls.h b/include/net/tls.h index 0a3540a6304d..2a98d0584999 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -251,6 +251,8 @@ struct tls_context { u8 tx_conf:3; u8 rx_conf:3; + struct proto *sk_proto; + int (*push_pending_record)(struct sock *sk, int flags); void (*sk_write_space)(struct sock *sk); @@ -288,6 +290,8 @@ struct tls_context { struct list_head list; refcount_t refcount; + + struct work_struct gc; }; enum tls_offload_ctx_dir { @@ -356,7 +360,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx); int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size); int tls_sw_sendpage(struct sock *sk, struct page *page, int offset, size_t size, int flags); -void tls_sw_close(struct sock *sk, long timeout); +void tls_sw_cancel_work_tx(struct tls_context *tls_ctx); void tls_sw_free_resources_tx(struct sock *sk); void tls_sw_free_resources_rx(struct sock *sk); void tls_sw_release_resources_rx(struct sock *sk); diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index d63c3583d2f7..e8418456ee24 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -251,6 +251,32 @@ static void tls_write_space(struct sock *sk) ctx->sk_write_space(sk); } +static void tls_ctx_free_deferred(struct work_struct *gc) +{ + struct tls_context *ctx = container_of(gc, struct tls_context, gc); + + if (ctx->rx_conf == TLS_SW) + tls_sw_release_strp_rx(ctx); + + /* Ensure any remaining work items are completed. The sk will + * already have lost its tls_ctx reference by the time we get + * here so no xmit operation will actually be performed. + */ + tls_sw_cancel_work_tx(ctx); + kfree(ctx); +} + +static void tls_ctx_free_wq(struct tls_context *ctx) +{ + if (!ctx) + return; + + memzero_explicit(&ctx->crypto_send, sizeof(ctx->crypto_send)); + memzero_explicit(&ctx->crypto_recv, sizeof(ctx->crypto_recv)); + INIT_WORK(&ctx->gc, tls_ctx_free_deferred); + schedule_work(&ctx->gc); +} + static void tls_ctx_free(struct tls_context *ctx) { if (!ctx) @@ -287,6 +313,27 @@ static void tls_sk_proto_cleanup(struct sock *sk, #endif } +static void tls_sk_proto_unhash(struct sock *sk) +{ + struct inet_connection_sock *icsk = inet_csk(sk); + long timeo = sock_sndtimeo(sk, 0); + struct tls_context *ctx; + + if (unlikely(!icsk->icsk_ulp_data)) { + if (sk->sk_prot->unhash) + sk->sk_prot->unhash(sk); + } + + ctx = tls_get_ctx(sk); + if (ctx->tx_conf == TLS_SW || ctx->rx_conf == TLS_SW) + tls_sk_proto_cleanup(sk, ctx, timeo); + icsk->icsk_ulp_data = NULL; + tls_ctx_free_wq(ctx); + + if (ctx->unhash) + ctx->unhash(sk); +} + static void tls_sk_proto_close(struct sock *sk, long timeout) { struct tls_context *ctx = tls_get_ctx(sk); @@ -305,6 +352,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout) if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) goto skip_tx_cleanup; + sk->sk_prot = ctx->sk_proto; tls_sk_proto_cleanup(sk, ctx, timeo); skip_tx_cleanup: @@ -606,6 +654,7 @@ static struct tls_context *create_ctx(struct sock *sk) ctx->setsockopt = sk->sk_prot->setsockopt; ctx->getsockopt = sk->sk_prot->getsockopt; ctx->sk_proto_close = sk->sk_prot->close; + ctx->unhash = sk->sk_prot->unhash; return ctx; } @@ -729,20 +778,24 @@ static void build_protos(struct proto prot[TLS_NUM_CONFIG][TLS_NUM_CONFIG], prot[TLS_BASE][TLS_BASE].setsockopt = tls_setsockopt; prot[TLS_BASE][TLS_BASE].getsockopt = tls_getsockopt; prot[TLS_BASE][TLS_BASE].close = tls_sk_proto_close; + prot[TLS_BASE][TLS_BASE].unhash = tls_sk_proto_unhash; prot[TLS_SW][TLS_BASE] = prot[TLS_BASE][TLS_BASE]; prot[TLS_SW][TLS_BASE].sendmsg = tls_sw_sendmsg; prot[TLS_SW][TLS_BASE].sendpage = tls_sw_sendpage; + prot[TLS_SW][TLS_BASE].unhash = tls_sk_proto_unhash; prot[TLS_BASE][TLS_SW] = prot[TLS_BASE][TLS_BASE]; prot[TLS_BASE][TLS_SW].recvmsg = tls_sw_recvmsg; prot[TLS_BASE][TLS_SW].stream_memory_read = tls_sw_stream_read; prot[TLS_BASE][TLS_SW].close = tls_sk_proto_close; + prot[TLS_BASE][TLS_SW].unhash = tls_sk_proto_unhash; prot[TLS_SW][TLS_SW] = prot[TLS_SW][TLS_BASE]; prot[TLS_SW][TLS_SW].recvmsg = tls_sw_recvmsg; prot[TLS_SW][TLS_SW].stream_memory_read = tls_sw_stream_read; prot[TLS_SW][TLS_SW].close = tls_sk_proto_close; + prot[TLS_SW][TLS_SW].unhash = tls_sk_proto_unhash; #ifdef CONFIG_TLS_DEVICE prot[TLS_HW][TLS_BASE] = prot[TLS_BASE][TLS_BASE]; @@ -793,6 +846,7 @@ static int tls_init(struct sock *sk) tls_build_proto(sk); ctx->tx_conf = TLS_BASE; ctx->rx_conf = TLS_BASE; + ctx->sk_proto = sk->sk_prot; update_sk_prot(sk, ctx); out: return rc; From patchwork Mon Jul 8 19:14:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1129213 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: incoming-bpf@patchwork.ozlabs.org Delivered-To: patchwork-incoming-bpf@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=bpf-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="Muvb234S"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45jFWL0knlz9sNH for ; Tue, 9 Jul 2019 05:14:38 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731664AbfGHTOh (ORCPT ); Mon, 8 Jul 2019 15:14:37 -0400 Received: from mail-io1-f65.google.com ([209.85.166.65]:42972 "EHLO mail-io1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728827AbfGHTOh (ORCPT ); Mon, 8 Jul 2019 15:14:37 -0400 Received: by mail-io1-f65.google.com with SMTP id u19so37757794ior.9; Mon, 08 Jul 2019 12:14:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=ZHj7FzG6KKRySBDBc7TLYiAyMZ+JKkfDT3AI7gRaHYQ=; b=Muvb234SOfI2dkAM481zW/DsSPQRjV4pTwlWdKOCxo5jtzZc3QbpcYfS3lp2DtzKgW SCbhZO1+ctHSXwhQhViRgjpaQI/ty0bOIzECsc2WID5Rjmz9MN1L+BIaxcmN56kT4Y5f pNYCofDHeCgphlpp0bqkZed3/tfi/YF8NhT10VFceJtcjGVOJkWpbENx/Xc+omeKbtY+ HVSwnO9zTJIXwRg0lURkgt2vDRfVbWFN+79jXUHT8kp9AStjjCMQz39rLTuQUc6UsPCw 4TGLePgDeSH9E6n/Vf051gWhU6C1S94m/Vt20lKlM81507FUsEs9bPUC0jrJ7h1xYcwg M9mw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=ZHj7FzG6KKRySBDBc7TLYiAyMZ+JKkfDT3AI7gRaHYQ=; b=MJ30eqmgJAsZ+4Ybo/CqPHU/wf/B0AeuLjYulbQQoDonL86aPSC09QqX3vVgJmHcDL 6PiN1WLAJgGtf7Il8WJ+M1OgTF8+1zQYzYXMcYG8kMUxJwlTtaR76IIsU81tIB6/Cisr AxEIPbGhg8ebYPz44GCwRBphR7i3HLVz3VnJU2DOQC9LLPWpGtpOnQ/Y7U8h8oooCrNP fJwYMKtiwuOGSfEMDi6y/dig6Vu0uGV7OYNOgMo5yLnv8nlD6k4zzQY3Y7zfKk9Ls1G5 I8qzjm5cDV27/eTHMnxgesmVoZWayoRVWg7VL+iVhCUbwcVdusOkPPb6Yo4s3oO5X+7/ Alkw== X-Gm-Message-State: APjAAAUZQtAj6ZAWrn72P5eoUIgCyIRo5F8g7y+LDRivrNrIrQwOrFOE lKsIEmRbOe3nBl9xIUBaaok= X-Google-Smtp-Source: APXvYqz3ev/yMDnkXUqLxZeVICHbSM9FMfCk+G9X4V1kQN2o+fn+NB8T6b4Ay+2n7NDSloC2wr5flg== X-Received: by 2002:a02:3f1d:: with SMTP id d29mr24516554jaa.116.1562613276243; Mon, 08 Jul 2019 12:14:36 -0700 (PDT) Received: from [127.0.1.1] ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id c17sm15350637ioo.82.2019.07.08.12.14.29 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 08 Jul 2019 12:14:35 -0700 (PDT) Subject: [bpf PATCH v2 3/6] bpf: sockmap, sock_map_delete needs to use xchg From: John Fastabend To: jakub.kicinski@netronome.com, ast@kernel.org, daniel@iogearbox.net Cc: netdev@vger.kernel.org, edumazet@google.com, john.fastabend@gmail.com, bpf@vger.kernel.org Date: Mon, 08 Jul 2019 19:14:23 +0000 Message-ID: <156261326334.31108.17728262375224434415.stgit@ubuntu3-kvm1> In-Reply-To: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> References: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org __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 --- 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 52d4faeee18b..28702f2e9a4a 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -276,16 +276,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, From patchwork Mon Jul 8 19:14:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1129216 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="YyuIAZE6"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45jFWh4mWbz9sNk for ; Tue, 9 Jul 2019 05:14:56 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2403817AbfGHTO4 (ORCPT ); Mon, 8 Jul 2019 15:14:56 -0400 Received: from mail-io1-f65.google.com ([209.85.166.65]:40886 "EHLO mail-io1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731582AbfGHTOz (ORCPT ); Mon, 8 Jul 2019 15:14:55 -0400 Received: by mail-io1-f65.google.com with SMTP id h6so29575592iom.7; Mon, 08 Jul 2019 12:14:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=tuW20oRn6IJ0xhfycOnDw/Ym3bbk01yn3xu2K8ZvEKA=; b=YyuIAZE6F7vdehmPor1V1IKvHWhRBkr3jio+Nt6WPbSbmYBXB1mK5RqPxywmyRrkOg w19/AQhLLMJolZvfgRBA30GXUNjJVA9sIs9Fj8mQx0kp28OVs6dWCGPWbo20ya5xv/kH 7wwcSh2nUxLW328QECKtorSWNIsvAxP0FUkLIvZv5GBC6tKeSXENEcyNT1A8XXTOfbHL 1C/k1EG2E8QeFIYC1hX1B39ofvtFdEWtw7a6LwQOUj1x8seSiS5f55uIOki2u5zrwMjt 3qSPf7JwVTJs/Ey1uUxAJFIu04GWsuSFOfOMqYi0asYLvMeQULySNklVRR+8worQk4oC Xn5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=tuW20oRn6IJ0xhfycOnDw/Ym3bbk01yn3xu2K8ZvEKA=; b=U6YsG5OL3KDsZaVPY7m2AZDh7sVezlIdXGB8BADD/geqCOWrDN6M+rDl13R4u6PgeE X6JU0sJ/QBDAtyPh6TWdswvxrbTA2FNW1RnXbS03ud/oJkg092w2sCjS3pIRFXKehxyA L1EGuWov0+/UVnSzEyctjcKRO+nLiTNwhpXWM8St41/YML/L3Wcp4N2FwPO3HE//uFte VoyLxZM7U4wcxQB/JNDvbFbE/yPOfDKwGK4c/3XjjaHo8COJqVprkXcwgBFXRwoPWPwE Me4E70T1m4yQ4v+ZjS/xy6BQs9WYSNqaP5Z3Iawhctk69Wj/3c1qGzQ2BjyIMcjUtJ/p bXWQ== X-Gm-Message-State: APjAAAWDzuk0ABPjrqYQUzEoScktSRHVL/3s1jVYW1RppE/FOJ/FfT8E BSkBdd51bzDG/mL5vbWKT0g= X-Google-Smtp-Source: APXvYqyzaAqv4RbFPI+87Dq2DfsHMDKN5tetE4s+OJXg00ui1KBSjMiA3Ncj31rgWhlCc0gOAu8DPw== X-Received: by 2002:a6b:6310:: with SMTP id p16mr20888179iog.118.1562613295102; Mon, 08 Jul 2019 12:14:55 -0700 (PDT) Received: from [127.0.1.1] ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id o7sm15117478ioo.81.2019.07.08.12.14.47 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 08 Jul 2019 12:14:54 -0700 (PDT) Subject: [bpf PATCH v2 4/6] bpf: sockmap, synchronize_rcu before free'ing map From: John Fastabend To: jakub.kicinski@netronome.com, ast@kernel.org, daniel@iogearbox.net Cc: netdev@vger.kernel.org, edumazet@google.com, john.fastabend@gmail.com, bpf@vger.kernel.org Date: Mon, 08 Jul 2019 19:14:42 +0000 Message-ID: <156261328226.31108.15349135352631297716.stgit@ubuntu3-kvm1> In-Reply-To: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> References: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org We need to have a synchronize_rcu before free'ing the sockmap because any outstanding psock references will have a pointer to the map and when they use this could trigger a use after free. Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") Signed-off-by: John Fastabend --- net/core/sock_map.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/core/sock_map.c b/net/core/sock_map.c index 28702f2e9a4a..56bcabe7c2f2 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -247,6 +247,8 @@ static void sock_map_free(struct bpf_map *map) raw_spin_unlock_bh(&stab->lock); rcu_read_unlock(); + synchronize_rcu(); + bpf_map_area_free(stab->sks); kfree(stab); } From patchwork Mon Jul 8 19:15:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1129217 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: incoming-bpf@patchwork.ozlabs.org Delivered-To: patchwork-incoming-bpf@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=bpf-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="Yutek5f2"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45jFX20Sb7z9sDB for ; Tue, 9 Jul 2019 05:15:14 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2403826AbfGHTPN (ORCPT ); Mon, 8 Jul 2019 15:15:13 -0400 Received: from mail-io1-f65.google.com ([209.85.166.65]:36077 "EHLO mail-io1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728827AbfGHTPN (ORCPT ); Mon, 8 Jul 2019 15:15:13 -0400 Received: by mail-io1-f65.google.com with SMTP id o9so22218058iom.3; Mon, 08 Jul 2019 12:15:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=lV8etYsEZTqwvPfp5XBIG2UlKxfnHXO0yLVdtiun7Hk=; b=Yutek5f2mygHV6NeordcHOHStjSVRsVYT3PxfDEE/7T4Xc7j4Vkyib7w3V9C4vN2SA VmBxBpEeB9Tvwg90kyxQS9QCql8AWCjD7VANvJ3ZrU67zRPqqXv9ihQDcZ0nSrkVcj0y 8IVhMLDlrHWx/bs6+DhxHqX1DFbeH1QYCNMSad+7223lqq5NvwdxR1cjHELL2TOaJLN7 RNBaIrjNj7kXQkLTrA4mBwIxOFfWW/nmKdsEqWBP6sX6UJyhj2qUYQvnYpzz/365RvaQ wEmzKJdf10Z0xlGfxWyl6wKPBxG3gvo+lWmieIzcCKo+/0evMhWhOR6rcG0mN/GjOABM Rwkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=lV8etYsEZTqwvPfp5XBIG2UlKxfnHXO0yLVdtiun7Hk=; b=sOWEiUsBss4sQoi4EJxOEgLvl6MetRqjFw8WorRTTEpEIshg/BDYjJbmPSTy69URAE FRoW1VMM/ULMAKPfTP3t2ky8w3QQmXsDQrMD9Pboi9Duiw9k/g19t+CqUtif5Klx5iPt fUCiPHJ1vmaTN3tLmGMuU6zOkMwn+HBN6YAuHeGJK/JUrLe19flJI07y6PccUCfpfkp9 qiC7B4WFhPBii5WvpeCqI+30VhtHeIFXJvbqgAernKQfgKqUV4x8x4BxVNUM1o8xGiur UPwxZydU122n4f9DC/gXoCaMykGd6BLrRA9c1K63hiLGfmVpGKcxbpMTtMO9UUrgPL1O /yiA== X-Gm-Message-State: APjAAAVngyyLLSbX8Uv+VONaR+RT9VFOjBTvY30Z+4NvDDuEdbq+To+j SIz4V5YsBQO+1rDvCt+ufsw= X-Google-Smtp-Source: APXvYqz8J9EN6QDIuDm8wthUnw8EGfCb9XqvgK4uf7rm4DSx3A2kso8fPfVR2gPuq3ZjkVUP4GifWg== X-Received: by 2002:a6b:8b8b:: with SMTP id n133mr1367677iod.183.1562613312685; Mon, 08 Jul 2019 12:15:12 -0700 (PDT) Received: from [127.0.1.1] ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id v10sm20828239iob.43.2019.07.08.12.15.05 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 08 Jul 2019 12:15:12 -0700 (PDT) Subject: [bpf PATCH v2 5/6] bpf: sockmap, only create entry if ulp is not already enabled From: John Fastabend To: jakub.kicinski@netronome.com, ast@kernel.org, daniel@iogearbox.net Cc: netdev@vger.kernel.org, edumazet@google.com, john.fastabend@gmail.com, bpf@vger.kernel.org Date: Mon, 08 Jul 2019 19:15:01 +0000 Message-ID: <156261330105.31108.1927837989210591684.stgit@ubuntu3-kvm1> In-Reply-To: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> References: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org Sockmap does not currently support adding sockets after TLS has been enabled. There never was a real use case for this so it was never added. But, we lost the test for ULP at some point so add it here and fail the socket insert if TLS is enabled. Future work could make sockmap support this use case but fixup the bug here. Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") Signed-off-by: John Fastabend --- net/core/sock_map.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/core/sock_map.c b/net/core/sock_map.c index 56bcabe7c2f2..1330a7442e5b 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -334,6 +334,7 @@ static int sock_map_update_common(struct bpf_map *map, u32 idx, struct sock *sk, u64 flags) { struct bpf_stab *stab = container_of(map, struct bpf_stab, map); + struct inet_connection_sock *icsk = inet_csk(sk); struct sk_psock_link *link; struct sk_psock *psock; struct sock *osk; @@ -344,6 +345,8 @@ static int sock_map_update_common(struct bpf_map *map, u32 idx, return -EINVAL; if (unlikely(idx >= map->max_entries)) return -E2BIG; + if (unlikely(icsk->icsk_ulp_data)) + return -EINVAL; link = sk_psock_init_link(); if (!link) From patchwork Mon Jul 8 19:15:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1129219 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: incoming-bpf@patchwork.ozlabs.org Delivered-To: patchwork-incoming-bpf@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=bpf-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="eCAOBsM3"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45jFXQ736Jz9sDB for ; Tue, 9 Jul 2019 05:15:34 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2403846AbfGHTPe (ORCPT ); Mon, 8 Jul 2019 15:15:34 -0400 Received: from mail-io1-f65.google.com ([209.85.166.65]:34897 "EHLO mail-io1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728827AbfGHTPd (ORCPT ); Mon, 8 Jul 2019 15:15:33 -0400 Received: by mail-io1-f65.google.com with SMTP id m24so28331336ioo.2; Mon, 08 Jul 2019 12:15:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=QP2BQLcDax4oZj4bSFvh8EyG6vL9pp9ZKdbj2H2DM00=; b=eCAOBsM34g1rAjGofOPcZnsTLxwSEKDrWSRNkD1rnDVJ5B6VybqOeVDJxcBCNHU79x m2YN7DxKrgD0kdfKexYWKolUE4r90H6+kOCEaFaGTx1hMBSow28SOgd9YeOPU2ibOuIy f0lEoxhB8T97JkNRk76xJcKRmgR1/IO4sWj+g96OxxRZbYd+DWsbsfmXrQ/nycJPOzpt TAKDQcqjaBBNmFtzZg6WsG/pOwOBqkjl2J0bhwZD5zm5Ihspwh+nq4idx+oynQDNTLSI PjurtbvRnMjyGDc0FnHi2Gr3GT4yDboMOVmnnBaaUjKd5DC14GPZ0Dt/+UtInWsrIW49 1WWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=QP2BQLcDax4oZj4bSFvh8EyG6vL9pp9ZKdbj2H2DM00=; b=rMFJPGDkoFmYzhPL/AuMWLaqs2HEcAs4+q+OvP+uGDtV90tu44a+0ZB1sP14XDsvxs PDc5ttpcLXp/gjTPvx6PhFUGNPzABNjpX6K250Lp9qdUykGcjHG6qEgMsSx3sUaRuU3N qOKG5zDZfEqG+cLXWX2llY1WbYkTICtdJLFHLor9WPnIKqVDh1vh0wUqzu9zjU1mPcI5 0q5DWJ4G06UOI2tJXJ9yl/k7FVTlNs2WJlSMzcpPMvIAfMn84I2kiOjNko9Gsc273dY3 cp3TKNccDJd9qkKJy9iQgfCldaQTMPL7rTXpbF+2B9t+6+mMXSVylBKBBI4mDXAvXylr 6A/w== X-Gm-Message-State: APjAAAXwbExI1ln/65mzmBSs54Srs0WnpZ+FsYb6NZ8ZR2BBtvqCcrlM +GonV+3RiRMrmGOL6YVs8tc= X-Google-Smtp-Source: APXvYqyBrhPyBUUvj+cE1ZyGeWfTlVkncjlSoabTSCZNQaU5A3YOR5JTS/dYB/TFR/eO47LKDoD1RQ== X-Received: by 2002:a02:5ec3:: with SMTP id h186mr17005079jab.110.1562613332141; Mon, 08 Jul 2019 12:15:32 -0700 (PDT) Received: from [127.0.1.1] ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id u4sm23675520iol.59.2019.07.08.12.15.24 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 08 Jul 2019 12:15:31 -0700 (PDT) Subject: [bpf PATCH v2 6/6] bpf: sockmap/tls, close can race with map free From: John Fastabend To: jakub.kicinski@netronome.com, ast@kernel.org, daniel@iogearbox.net Cc: netdev@vger.kernel.org, edumazet@google.com, john.fastabend@gmail.com, bpf@vger.kernel.org Date: Mon, 08 Jul 2019 19:15:18 +0000 Message-ID: <156261331866.31108.6405316261950259075.stgit@ubuntu3-kvm1> In-Reply-To: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> References: <156261310104.31108.4569969631798277807.stgit@ubuntu3-kvm1> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org When a map free is called and in parallel a socket is closed we have two paths that can potentially reset the socket prot ops, the bpf close() path and the map free path. This creates a problem with which prot ops should be used from the socket closed side. If the map_free side completes first then we want to call the original lowest level ops. However, if the tls path runs first we want to call the sockmap ops. Additionally there was no locking around prot updates in TLS code paths so the prot ops could be changed multiple times once from TLS path and again from sockmap side potentially leaving ops pointed at either TLS or sockmap when psock and/or tls context have already been destroyed. To fix this race first only update ops inside callback lock so that TLS, sockmap and lowest level all agree on prot state. Second and a ULP callback update() so that lower layers can inform the upper layer when they are being removed allowing the upper layer to reset prot ops. This gets us close to allowing sockmap and tls to be stacked in arbitrary order but will save that patch for *next trees. Reported-by: syzbot+06537213db7ba2745c4a@syzkaller.appspotmail.com Fixes: 02c558b2d5d6 ("bpf: sockmap, support for msg_peek in sk_msg with redirect ingress") Signed-off-by: John Fastabend --- include/linux/skmsg.h | 8 +++++++- include/net/tcp.h | 3 +++ net/core/skmsg.c | 4 ++-- net/ipv4/tcp_ulp.c | 13 +++++++++++++ net/tls/tls_main.c | 35 +++++++++++++++++++++++++++++------ 5 files changed, 54 insertions(+), 9 deletions(-) diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h index 50ced8aba9db..e4b3fb4bb77c 100644 --- a/include/linux/skmsg.h +++ b/include/linux/skmsg.h @@ -354,7 +354,13 @@ static inline void sk_psock_restore_proto(struct sock *sk, sk->sk_write_space = psock->saved_write_space; if (psock->sk_proto) { - sk->sk_prot = psock->sk_proto; + struct inet_connection_sock *icsk = inet_csk(sk); + bool has_ulp = !!icsk->icsk_ulp_data; + + if (has_ulp) + tcp_update_ulp(sk, psock->sk_proto); + else + sk->sk_prot = psock->sk_proto; psock->sk_proto = NULL; } } diff --git a/include/net/tcp.h b/include/net/tcp.h index 9d36cc88d043..123cac4c96f2 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -2102,6 +2102,8 @@ struct tcp_ulp_ops { /* initialize ulp */ int (*init)(struct sock *sk); + /* update ulp */ + void (*update)(struct sock *sk, struct proto *p); /* cleanup ulp */ void (*release)(struct sock *sk); @@ -2113,6 +2115,7 @@ void tcp_unregister_ulp(struct tcp_ulp_ops *type); int tcp_set_ulp(struct sock *sk, const char *name); void tcp_get_available_ulp(char *buf, size_t len); void tcp_cleanup_ulp(struct sock *sk); +void tcp_update_ulp(struct sock *sk, struct proto *p); #define MODULE_ALIAS_TCP_ULP(name) \ __MODULE_INFO(alias, alias_userspace, name); \ diff --git a/net/core/skmsg.c b/net/core/skmsg.c index 93bffaad2135..6832eeb4b785 100644 --- a/net/core/skmsg.c +++ b/net/core/skmsg.c @@ -585,12 +585,12 @@ EXPORT_SYMBOL_GPL(sk_psock_destroy); void sk_psock_drop(struct sock *sk, struct sk_psock *psock) { - rcu_assign_sk_user_data(sk, NULL); sk_psock_cork_free(psock); sk_psock_zap_ingress(psock); - sk_psock_restore_proto(sk, psock); write_lock_bh(&sk->sk_callback_lock); + sk_psock_restore_proto(sk, psock); + rcu_assign_sk_user_data(sk, NULL); if (psock->progs.skb_parser) sk_psock_stop_strp(sk, psock); write_unlock_bh(&sk->sk_callback_lock); diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c index 3d8a1d835471..4849edb62d52 100644 --- a/net/ipv4/tcp_ulp.c +++ b/net/ipv4/tcp_ulp.c @@ -96,6 +96,19 @@ void tcp_get_available_ulp(char *buf, size_t maxlen) rcu_read_unlock(); } +void tcp_update_ulp(struct sock *sk, struct proto *proto) +{ + struct inet_connection_sock *icsk = inet_csk(sk); + + if (!icsk->icsk_ulp_ops) { + sk->sk_prot = proto; + return; + } + + if (icsk->icsk_ulp_ops->update) + icsk->icsk_ulp_ops->update(sk, proto); +} + void tcp_cleanup_ulp(struct sock *sk) { struct inet_connection_sock *icsk = inet_csk(sk); diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index e8418456ee24..4ba5476fbc5f 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -336,15 +336,17 @@ static void tls_sk_proto_unhash(struct sock *sk) static void tls_sk_proto_close(struct sock *sk, long timeout) { + struct inet_connection_sock *icsk = inet_csk(sk); struct tls_context *ctx = tls_get_ctx(sk); long timeo = sock_sndtimeo(sk, 0); - void (*sk_proto_close)(struct sock *sk, long timeout); + + if (unlikely(!ctx)) + return; if (ctx->tx_conf == TLS_SW) tls_sw_cancel_work_tx(ctx); lock_sock(sk); - sk_proto_close = ctx->sk_proto_close; if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD) goto skip_tx_cleanup; @@ -352,15 +354,18 @@ static void tls_sk_proto_close(struct sock *sk, long timeout) if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) goto skip_tx_cleanup; - sk->sk_prot = ctx->sk_proto; tls_sk_proto_cleanup(sk, ctx, timeo); skip_tx_cleanup: + write_lock_bh(&sk->sk_callback_lock); + icsk->icsk_ulp_data = NULL; + if (sk->sk_prot->close == tls_sk_proto_close) + sk->sk_prot = ctx->sk_proto; + write_unlock_bh(&sk->sk_callback_lock); release_sock(sk); if (ctx->rx_conf == TLS_SW) tls_sw_release_strp_rx(ctx); - sk_proto_close(sk, timeout); - + ctx->sk_proto_close(sk, timeout); if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW && ctx->tx_conf != TLS_HW_RECORD && ctx->rx_conf != TLS_HW_RECORD) tls_ctx_free(ctx); @@ -836,22 +841,39 @@ static int tls_init(struct sock *sk) if (sk->sk_state != TCP_ESTABLISHED) return -ENOTSUPP; + tls_build_proto(sk); + /* allocate tls context */ + write_lock_bh(&sk->sk_callback_lock); ctx = create_ctx(sk); if (!ctx) { rc = -ENOMEM; goto out; } - tls_build_proto(sk); ctx->tx_conf = TLS_BASE; ctx->rx_conf = TLS_BASE; ctx->sk_proto = sk->sk_prot; update_sk_prot(sk, ctx); out: + write_unlock_bh(&sk->sk_callback_lock); return rc; } +static void tls_update(struct sock *sk, struct proto *p) +{ + struct tls_context *ctx; + + ctx = tls_get_ctx(sk); + if (likely(ctx)) { + ctx->sk_proto_close = p->close; + ctx->unhash = p->unhash; + ctx->sk_proto = p; + } else { + sk->sk_prot = p; + } +} + void tls_register_device(struct tls_device *device) { spin_lock_bh(&device_spinlock); @@ -872,6 +894,7 @@ static struct tcp_ulp_ops tcp_tls_ulp_ops __read_mostly = { .name = "tls", .owner = THIS_MODULE, .init = tls_init, + .update = tls_update, }; static int __init tls_register(void)