From patchwork Mon Jul 15 20:49:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1132279 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="EDVZnwuf"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45nbHC6Y5Zz9s3l for ; Tue, 16 Jul 2019 06:49:11 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730275AbfGOUtL (ORCPT ); Mon, 15 Jul 2019 16:49:11 -0400 Received: from mail-oi1-f195.google.com ([209.85.167.195]:46021 "EHLO mail-oi1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729505AbfGOUtL (ORCPT ); Mon, 15 Jul 2019 16:49:11 -0400 Received: by mail-oi1-f195.google.com with SMTP id m206so13766540oib.12; Mon, 15 Jul 2019 13:49:10 -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=nSxP56LyPb7uTkprbb9AoFVApYUjT+yRCfgItnmRVaY=; b=EDVZnwufWLPKvZS0rBXbtuOw9zeeVp9qakaGl3i7eLow21u290qrYQyOiut8GjnCNa aFRacGR35cQ5UCZQprR5G6qt/za0fnt0we7Pu1OYJ6bwgPj6c09c5BRtXfmo+jsB/dXj J/Ma7KOATRGzcvRRg3J8QukvVGDP2J29FI5Iav8Oqqb0fCHv9Yyf4Hsyy2AoGNsDrQ65 m4iOyrWNe3H+c3C0SeyXumOVhjcvvBH1a9mnbU6ER++AqxIwM+uAFi9+DHQ/JCMoJMi4 e6lDasv0vHJYDIs04mic1Ci01qHI2IV0De+VHl9vs9wLza9ujCdMLoOh3vtsqi4GjCDN ioeQ== 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=nSxP56LyPb7uTkprbb9AoFVApYUjT+yRCfgItnmRVaY=; b=WSFCUeaEkHunJdEIeZgajzdom9VrtHYYPWp7eA9J6s39E4Z+VdsJMaKjy0f4VBrtT5 IMgJtJd+cR6wrBg8hRKLeAQeLmsbABfnusdBtWLJxlY3gqE10wj1qi0Zk96cwmuUpDug vo4d9DxzaVU0dHko18SjLGz1tZfkOOUu+Qc7B4WMKSWmz/q6eo8tWRLxarFFuMUnpk56 pP3bdYXk/iycetEjKAHT2Cs/Ul2OSLELFaLsQz5PpSnma2idvNK4IIem56lDp+UO5GbN 2RLkVMCrUohpL5nAv+kjfOBAKvzVKfLcCQnOFpLSda01MdFHgIDS5BYtAMKnIVxdP73I xhbQ== X-Gm-Message-State: APjAAAUEzw6Npv9den9bsSBlD3Tz/I8uB2NbJtKpmgEY6TqkpyqV1Wsh wv7oHQCxpxiJGcD89+K/uA8= X-Google-Smtp-Source: APXvYqwaxu1O0DA8Sdd85cZC/Qwryb6pCpgNVymtAKSzzSF9Av+aUUq0KOZVaI2mV8K3gpYI0zTcRA== X-Received: by 2002:aca:2119:: with SMTP id 25mr12776336oiz.48.1563223750585; Mon, 15 Jul 2019 13:49:10 -0700 (PDT) Received: from [127.0.1.1] ([99.0.85.34]) by smtp.gmail.com with ESMTPSA id g93sm7261088otb.39.2019.07.15.13.49.09 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 15 Jul 2019 13:49:10 -0700 (PDT) Subject: [bpf PATCH v3 1/8] net/tls: don't arm strparser immediately in tls_set_sw_offload() 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, 15 Jul 2019 13:49:09 -0700 Message-ID: <156322374898.18678.12650354854448590855.stgit@john-XPS-13-9370> In-Reply-To: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> References: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> 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 From: Jakub Kicinski In tls_set_device_offload_rx() we prepare the software context for RX fallback and proceed to add the connection to the device. Unfortunately, software context prep includes arming strparser so in case of a later error we have to release the socket lock to call strp_done(). In preparation for not releasing the socket lock half way through callbacks move arming strparser into a separate function. Following patches will make use of that. Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe --- include/net/tls.h | 1 + net/tls/tls_device.c | 1 + net/tls/tls_main.c | 8 +++++--- net/tls/tls_sw.c | 19 ++++++++++++------- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/include/net/tls.h b/include/net/tls.h index 584609174fe0..43f551cd508b 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -355,6 +355,7 @@ int tls_sk_attach(struct sock *sk, int optname, char __user *optval, unsigned int optlen); int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx); +void tls_sw_strparser_arm(struct sock *sk, struct tls_context *ctx); 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); diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c index 7c0b2b778703..4d67d72f007c 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -1045,6 +1045,7 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx) rc = tls_set_sw_offload(sk, ctx, 0); if (rc) goto release_ctx; + tls_sw_strparser_arm(sk, ctx); rc = netdev->tlsdev_ops->tls_dev_add(netdev, sk, TLS_OFFLOAD_CTX_DIR_RX, &ctx->crypto_recv.info, diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index 4674e57e66b0..85a9d7d57b32 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -526,6 +526,8 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval, { #endif rc = tls_set_sw_offload(sk, ctx, 1); + if (rc) + goto err_crypto_info; conf = TLS_SW; } } else { @@ -537,13 +539,13 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval, { #endif rc = tls_set_sw_offload(sk, ctx, 0); + if (rc) + goto err_crypto_info; + tls_sw_strparser_arm(sk, ctx); conf = TLS_SW; } } - if (rc) - goto err_crypto_info; - if (tx) ctx->tx_conf = conf; else diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 53b4ad94e74a..f58a8ffc2a9c 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2160,6 +2160,18 @@ void tls_sw_write_space(struct sock *sk, struct tls_context *ctx) } } +void tls_sw_strparser_arm(struct sock *sk, struct tls_context *tls_ctx) +{ + struct tls_sw_context_rx *rx_ctx = tls_sw_ctx_rx(tls_ctx); + + write_lock_bh(&sk->sk_callback_lock); + rx_ctx->saved_data_ready = sk->sk_data_ready; + sk->sk_data_ready = tls_data_ready; + write_unlock_bh(&sk->sk_callback_lock); + + strp_check_rcv(&rx_ctx->strp); +} + int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx) { struct tls_context *tls_ctx = tls_get_ctx(sk); @@ -2357,13 +2369,6 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx) cb.parse_msg = tls_read_size; strp_init(&sw_ctx_rx->strp, sk, &cb); - - write_lock_bh(&sk->sk_callback_lock); - sw_ctx_rx->saved_data_ready = sk->sk_data_ready; - sk->sk_data_ready = tls_data_ready; - write_unlock_bh(&sk->sk_callback_lock); - - strp_check_rcv(&sw_ctx_rx->strp); } goto out; From patchwork Mon Jul 15 20:49:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1132281 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="gZV8Tu17"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45nbHM0zrhz9s3l for ; Tue, 16 Jul 2019 06:49:19 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731449AbfGOUtS (ORCPT ); Mon, 15 Jul 2019 16:49:18 -0400 Received: from mail-ot1-f65.google.com ([209.85.210.65]:37932 "EHLO mail-ot1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729505AbfGOUtS (ORCPT ); Mon, 15 Jul 2019 16:49:18 -0400 Received: by mail-ot1-f65.google.com with SMTP id d17so18560046oth.5; Mon, 15 Jul 2019 13:49: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=B2ogZ6mjovzkCR5HJmoGraNTud1gYT/+smTPhr8188g=; b=gZV8Tu17SHlNYM/fQS8Qttmu66ly4GyykrFHxK3/HViOrYp/5uIFGwKcI7vcrqVyUm IsoIoX9q/WbYvWAlRORz5g4F0sAKjVZKij7INGet3e+GL0MnFfIGiIVBrU/DHCKE5MfU fbNCuLVNbaAktiJVgqiNfIpCnRO/rCmOXUNeS0WBf8XIMPGqCn1uzx/siNN7r7eRepsr TQEKwf1LWiH2yawrPjG2I51udj9+lVznir0zzV9+jhO5zp8UCas8Xq+Gx3pdznh7WYTj y6vayC4pImFcbz0nEj3Hc/fysNFJ4IdurCkJYS68QajXOUcDEdcT2rOBesPILdwOMCpi yi0Q== 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=B2ogZ6mjovzkCR5HJmoGraNTud1gYT/+smTPhr8188g=; b=UizqqdI2dMA64TJnTxXhhSPUzJsmrqyV3QEkXKPT0H+HQg2ex7zuj5qaZu0JtyegxL RTwS7pTpv9C8DYyJkpRGfo7rLyVkrpftha8od0aOqRkDKzF39BcQPt9XK1VcYGvLbZoZ xNGuhjaTM3olYe4+tDB0es3r37zo98CLAcSaulXHrNpLuGaJbl6X9SdEJCfKJW5oCLGw OrDaOA9Wr1dSUa4/gu0ySW37q0ttLZnC7WSPWlfDRHtS2sx/U/lGamVbDB+fb3RTI8Pb 0Eg46mZjzXortIX/4SGo1GyOQC77xe7jLzT4ZG3N888OkW/Wc0vOTBXeRlkAxR4gGbMq BW6Q== X-Gm-Message-State: APjAAAU1FPz0+VEjhI1jvHrNP5AERQe1GoVaJHvJ1NWJPPK29rADbMOu tNYFQHwtzfrmhXsGx9lkTCM= X-Google-Smtp-Source: APXvYqw1PS6SbWzw9iSPEYeNXt0KdiZdhA+n6zitIL/4QyzAouOcX0uBfmn4kmOu5Ba96N/hZM57GA== X-Received: by 2002:a9d:470f:: with SMTP id a15mr20072874otf.235.1563223757350; Mon, 15 Jul 2019 13:49:17 -0700 (PDT) Received: from [127.0.1.1] ([99.0.85.34]) by smtp.gmail.com with ESMTPSA id 132sm6414818oid.47.2019.07.15.13.49.16 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 15 Jul 2019 13:49:16 -0700 (PDT) Subject: [bpf PATCH v3 2/8] tls: remove close callback sock unlock/lock around TX work flush 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, 15 Jul 2019 13:49:15 -0700 Message-ID: <156322375571.18678.11939499603669901288.stgit@john-XPS-13-9370> In-Reply-To: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> References: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> 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. 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. Tested with net selftests and bpf selftests. Signed-off-by: John Fastabend Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe --- include/net/tls.h | 2 ++ net/tls/tls_main.c | 3 +++ net/tls/tls_sw.c | 24 +++++++++++++++++------- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/include/net/tls.h b/include/net/tls.h index 43f551cd508b..d4276cb6de53 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -162,6 +162,7 @@ struct tls_sw_context_tx { int async_capable; #define BIT_TX_SCHEDULED 0 +#define BIT_TX_CLOSING 1 unsigned long tx_bitmask; }; @@ -360,6 +361,7 @@ 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 85a9d7d57b32..ddda422498aa 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -268,6 +268,9 @@ static void tls_sk_proto_close(struct sock *sk, long timeout) void (*sk_proto_close)(struct sock *sk, long timeout); bool free_ctx = false; + if (ctx->tx_conf == TLS_SW) + tls_sw_cancel_work_tx(ctx); + lock_sock(sk); sk_proto_close = ctx->sk_proto_close; diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index f58a8ffc2a9c..38c0e53c727d 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2054,6 +2054,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); @@ -2065,11 +2074,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 @@ -2137,11 +2141,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 15 20:49:22 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1132283 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="OZNR7DQ0"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45nbHT6CWbz9s3l for ; Tue, 16 Jul 2019 06:49:25 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732014AbfGOUtZ (ORCPT ); Mon, 15 Jul 2019 16:49:25 -0400 Received: from mail-ot1-f67.google.com ([209.85.210.67]:45677 "EHLO mail-ot1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729505AbfGOUtZ (ORCPT ); Mon, 15 Jul 2019 16:49:25 -0400 Received: by mail-ot1-f67.google.com with SMTP id x21so18506303otq.12; Mon, 15 Jul 2019 13:49:24 -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=UiKaE+sa/JTtPfu66ECQVi+EOsevEAKzkG+J0EsB2cI=; b=OZNR7DQ0gkcJBaoCDGjC3/4y6j6rKMfEpn+X4u5wlWMn6GyYahj1yxDToa5fZi2Z2I PUo77rOZ/iAL/VF3cXcfx1hnWqOERtkDcqMwtcJ2L2rLMtH59kBUveCftVUQQDRpvnuV ZCWg3oJ2tEnuboHGXm4iExL1gE4gIxbwgclsr52z8DTH0dNJpkdiYJrGARqRDwfrxKIX cBemLZQAJuGFkRA0Kunvwalomj9x/5pJAsPW8M9qhhyr7O4bUw8ef/hC3pi/0Gsllsxa qcPMwpO1RYqj/ucbS9UVvQe6JeoMCBKcXvRfppOQG9zL/9ihp5brTOhGpHVJP5sGrCHi 8mGg== 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=UiKaE+sa/JTtPfu66ECQVi+EOsevEAKzkG+J0EsB2cI=; b=PX5Yn4AJj1pVGGbV9Jq5vImFUWhtrkjh1JWgztOq9h8JvFT+gC+I8Lca/1nXmcv2ex Uod5szKM034+M938V0t4CtMC3pKDYCRY3mE2V8Qma28dv6t8CaSQfAcaWq3Au2xTbM2d 3WbAhMxta/3XtvnOYw6lmfMjpVfP84/WvjDyXp92i8o1XXZrRuYo30YM6J2uUdJ6rWnD UEpU+iupEUxdTdG/XeJH/e67lWppdg8v9IsXYaWakt9ViEZhk23mUoF5pFbjNju25eja zQM9UTl9/n6d1HHLl0eXWpMKU3W5eXGiEG7ZmRNeO5iAES9A9qlL6nHBZiA2xnTUffkL 5JdA== X-Gm-Message-State: APjAAAWE6ozHpy6JHA+4wLrJ+1I1MraiLt3TBaw1OJGSff8z54Qv2h+5 RteDgxNnuWZxZrBZVCFttHM= X-Google-Smtp-Source: APXvYqyaIr6KH1VdHwm2SPwcp1uLiJQ7gW0tLAbbKd908tfGXryV3v7UkL+o5HSHcoNuorlvLZ3uuA== X-Received: by 2002:a9d:7741:: with SMTP id t1mr5739795otl.178.1563223763952; Mon, 15 Jul 2019 13:49:23 -0700 (PDT) Received: from [127.0.1.1] ([99.0.85.34]) by smtp.gmail.com with ESMTPSA id z20sm6088307oic.31.2019.07.15.13.49.23 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 15 Jul 2019 13:49:23 -0700 (PDT) Subject: [bpf PATCH v3 3/8] tls: remove sock unlock/lock around strp_done() 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, 15 Jul 2019 13:49:22 -0700 Message-ID: <156322376245.18678.2590668444515934979.stgit@john-XPS-13-9370> In-Reply-To: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> References: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> 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 to call strp_done(). Split up the RX cleanup into stopping the strparser and releasing most resources, syncing strparser and finally freeing the context. To avoid the need for a strp_done() call on the cleanup path of device offload make sure we don't arm the strparser until we are sure init will be successful. Signed-off-by: John Fastabend Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe --- include/net/tls.h | 4 ++- net/tls/tls_device.c | 1 - net/tls/tls_main.c | 65 +++++++++++++++++++++++++------------------------- net/tls/tls_sw.c | 33 ++++++++++++++++++------- 4 files changed, 58 insertions(+), 45 deletions(-) diff --git a/include/net/tls.h b/include/net/tls.h index d4276cb6de53..72ddd16de056 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, }; @@ -357,6 +355,7 @@ int tls_sk_attach(struct sock *sk, int optname, char __user *optval, int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx); void tls_sw_strparser_arm(struct sock *sk, struct tls_context *ctx); +void tls_sw_strparser_done(struct tls_context *tls_ctx); 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); @@ -365,6 +364,7 @@ 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); +void tls_sw_free_ctx_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_device.c b/net/tls/tls_device.c index 4d67d72f007c..7c0b2b778703 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -1045,7 +1045,6 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx) rc = tls_set_sw_offload(sk, ctx, 0); if (rc) goto release_ctx; - tls_sw_strparser_arm(sk, ctx); rc = netdev->tlsdev_ops->tls_dev_add(netdev, sk, TLS_OFFLOAD_CTX_DIR_RX, &ctx->crypto_recv.info, diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index ddda422498aa..9f4a9da182ae 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -261,27 +261,9 @@ 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; - - 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) { - free_ctx = true; - goto skip_tx_cleanup; - } - if (unlikely(sk->sk_write_pending) && !wait_on_pending_writer(sk, &timeo)) tls_handle_open_record(sk, 0); @@ -298,27 +280,44 @@ static void tls_sk_proto_close(struct sock *sk, long timeout) } if (ctx->rx_conf == TLS_SW) - tls_sw_free_resources_rx(sk); + tls_sw_release_resources_rx(sk); #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) +{ + void (*sk_proto_close)(struct sock *sk, long timeout); + struct tls_context *ctx = tls_get_ctx(sk); + long timeo = sock_sndtimeo(sk, 0); + + 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 || ctx->rx_conf == TLS_HW) + tls_sw_strparser_done(ctx); + if (ctx->rx_conf == TLS_SW) + tls_sw_free_ctx_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); } @@ -544,9 +543,9 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval, rc = tls_set_sw_offload(sk, ctx, 0); if (rc) goto err_crypto_info; - tls_sw_strparser_arm(sk, ctx); conf = TLS_SW; } + tls_sw_strparser_arm(sk, ctx); } if (tx) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 38c0e53c727d..ee8fef312475 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2114,25 +2114,40 @@ void tls_sw_release_resources_rx(struct sock *sk) skb_queue_purge(&ctx->rx_list); crypto_free_aead(ctx->aead_recv); strp_stop(&ctx->strp); - 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); + /* If tls_sw_strparser_arm() was not called (cleanup paths) + * we still want to strp_stop(), but sk->sk_data_ready was + * never swapped. + */ + if (ctx->saved_data_ready) { + write_lock_bh(&sk->sk_callback_lock); + sk->sk_data_ready = ctx->saved_data_ready; + write_unlock_bh(&sk->sk_callback_lock); + } } } -void tls_sw_free_resources_rx(struct sock *sk) +void tls_sw_strparser_done(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); +} + +void tls_sw_free_ctx_rx(struct tls_context *tls_ctx) +{ + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx); kfree(ctx); } +void tls_sw_free_resources_rx(struct sock *sk) +{ + struct tls_context *tls_ctx = tls_get_ctx(sk); + + tls_sw_release_resources_rx(sk); + tls_sw_free_ctx_rx(tls_ctx); +} + /* The work handler to transmitt the encrypted records in tx_list */ static void tx_work_handler(struct work_struct *work) { From patchwork Mon Jul 15 20:49:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1132285 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="NQFF6mr6"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45nbHc1kTBz9sBt for ; Tue, 16 Jul 2019 06:49:32 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731213AbfGOUtb (ORCPT ); Mon, 15 Jul 2019 16:49:31 -0400 Received: from mail-ot1-f68.google.com ([209.85.210.68]:41639 "EHLO mail-ot1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729505AbfGOUtb (ORCPT ); Mon, 15 Jul 2019 16:49:31 -0400 Received: by mail-ot1-f68.google.com with SMTP id o101so18533062ota.8; Mon, 15 Jul 2019 13:49:31 -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=vmW6M38859vActXSBpqLHrDceWezbmKov04rtR+CpkU=; b=NQFF6mr6UoRsUaQEJmUehwrHUvePKXsc1oSGcn0KoDkwGFZ5Qo3DFimZxWIW9EkaH5 NF9iJvE5iSZHAQrag8VeC85EwqRio6qXKkBZ3w6pL8q9oHgL5XyHBged77omwIr5LO9e upk6pp+blH5VsFb9Stqm4/DYVr6u6x0Fgdw3DmaiH6U+OQVbS4Qo9GIk8rwNziz/un9X lHGMGGR2hppG6IolQt52Wn4D1pu9z1/i8N8olOgaGddY1lxvWjqzAq09KtL5OxBt4Sjg otI2vqngR2y/Iw4p43AyIkxUHx1FslS2KdlwCfTukpaJPO+UNaTtlvZi/rocVmtYcNt3 rRLQ== 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=vmW6M38859vActXSBpqLHrDceWezbmKov04rtR+CpkU=; b=hly4e4jD9l1xTYRDfEhVp224DVT0HlcI+YFKp07E8Vcu/kU/BqXPUQzBEGAPiHx+K9 XgHMB8vCpjO71bW8KDjWMZHM+K0nU1aqBMdoDXajByWoTsMz6iaL+CCsYPSpTsMo2GCq KHupprBc2Bw4kwWziCiPLY27HYsTZ6VeWwQDFF8gw8y+sdtCGNNwAbfV2wxSIzCn2aYY IwISUqidvQA6g8DeNaO+Zpt9BY5cCKTR+81p7pAJbgaLdsmIsoGtA/3J42RP7s7jIG58 n9g4PFfU/2c4GzX7/j2HWd+rx+kS/KxhWS/kYm4YWnwCMYbVuqEzo4TfLXlSQKJ/7XNl cu3Q== X-Gm-Message-State: APjAAAVGRdZ/ewvvu8PWpmyHwTqM3R10vSBCbJmmIjT3EdhaOvderNeH tdTIhb4KdLa/X6LR94KEv1g= X-Google-Smtp-Source: APXvYqzjl3rAa+33Rdb6bsu3pGPhRZdI3en8kwNMYx7Pz2Hv6Q9Viet+vL1VxPicbJIrs0jkp3dNPw== X-Received: by 2002:a05:6830:154e:: with SMTP id l14mr20206506otp.365.1563223770653; Mon, 15 Jul 2019 13:49:30 -0700 (PDT) Received: from [127.0.1.1] ([99.0.85.34]) by smtp.gmail.com with ESMTPSA id u17sm6232175oif.11.2019.07.15.13.49.29 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 15 Jul 2019 13:49:30 -0700 (PDT) Subject: [bpf PATCH v3 4/8] 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, 15 Jul 2019 13:49:29 -0700 Message-ID: <156322376920.18678.2076367069324125104.stgit@john-XPS-13-9370> In-Reply-To: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> References: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> 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 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 | 5 ++++- net/tls/tls_main.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/include/net/tls.h b/include/net/tls.h index 72ddd16de056..79ef7049375d 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 { @@ -359,7 +363,6 @@ void tls_sw_strparser_done(struct tls_context *tls_ctx); 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); diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index 9f4a9da182ae..f4cb0522fa95 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -251,6 +251,35 @@ 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 || ctx->rx_conf == TLS_HW) + tls_sw_strparser_done(ctx); + + if (ctx->rx_conf == TLS_SW) + tls_sw_free_ctx_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); +} + void tls_ctx_free(struct tls_context *ctx) { if (!ctx) @@ -288,6 +317,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) { void (*sk_proto_close)(struct sock *sk, long timeout); @@ -306,6 +356,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: @@ -611,6 +662,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; } @@ -734,20 +786,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]; @@ -798,6 +854,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 15 20:49:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1132287 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="Mt//Z6oj"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45nbHk6t5Mz9sBt for ; Tue, 16 Jul 2019 06:49:38 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732382AbfGOUti (ORCPT ); Mon, 15 Jul 2019 16:49:38 -0400 Received: from mail-ot1-f68.google.com ([209.85.210.68]:40949 "EHLO mail-ot1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729505AbfGOUti (ORCPT ); Mon, 15 Jul 2019 16:49:38 -0400 Received: by mail-ot1-f68.google.com with SMTP id y20so2595937otk.7; Mon, 15 Jul 2019 13:49:37 -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=Mt//Z6ojehdLRVZBQxlCutK6O9L5omSoMegkQTdhEZP6Q7s04Y4PvQAXrgrc3BCawd hGJMZAakxhxLUJhPoHp/lTI4GECdjUZIEObeTU5MJruCYURri6QNjWphmKgNmgU5ZzQA VFyyhfirSahfcBVF8LzNx3kY+8GJb4qgbaMt42u5Jp8/FJCOB+JKC0y3DAzBlsK9/r7E jDqGBxru+pQr3RFu7faSR95PA51KraZhDVPrc7/lvaOFE1y5izXUpcIIRsl3OFx5Rfi0 CXipNfseLMejqrLdmgKXnr7RPuIEqCeHhMTmzKxbHVmQVi1/U8XlmOB6mzgagzCMoCns fOjg== 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=mWihltdDWo4xYboR/JI7V0ppjTKGTgMfVeFkSI0UPxGpwPFHJnm3bJTIq+rGfoW5c/ TzKY64TNXhdLiaF0WHL/MA798rrUe+6uN4vcJkncA3SUdyh82D6EMqqh8x6K6PY+X0tl FA0KU76VyEnHVsyvNs/PHRTxWCFh9AYgi28WRaGOeCwvyp3jJf54C01XTIF0bUo8L8/+ bmPFntIuMk1FJAer6XzjIcL9t6WlnO7bYqfO3745IlFGxuIJ8TNoPYqbFByJtaRMd7eB uBW/hrVgSNSrsE7qQe4mYFjOumuMH+i5hX3nkFtoyLZfkis3qvY9/urv8w8Vx40g1VmL CsaA== X-Gm-Message-State: APjAAAUXsw9At+DwyL24jj24NDGT4qy1Gv45kKX+0P7GuStHiTnO2aoy BWjn5wFVqruygavvGKPXwvA= X-Google-Smtp-Source: APXvYqygiZIHfhM2/ndiQzWwnORwOI00rpzymuG4yHtWBM8FxSllf08V/erRJpNnB8kaGf3ElvX0ow== X-Received: by 2002:a9d:7e6:: with SMTP id 93mr22157851oto.143.1563223777358; Mon, 15 Jul 2019 13:49:37 -0700 (PDT) Received: from [127.0.1.1] ([99.0.85.34]) by smtp.gmail.com with ESMTPSA id d22sm6381356oig.38.2019.07.15.13.49.36 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 15 Jul 2019 13:49:36 -0700 (PDT) Subject: [bpf PATCH v3 5/8] 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, 15 Jul 2019 13:49:35 -0700 Message-ID: <156322377575.18678.9230942769643387694.stgit@john-XPS-13-9370> In-Reply-To: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> References: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> 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 15 20:49: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: 1132289 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="rHuAoW6r"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45nbHs4DRKz9sBt for ; Tue, 16 Jul 2019 06:49:45 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732543AbfGOUtp (ORCPT ); Mon, 15 Jul 2019 16:49:45 -0400 Received: from mail-ot1-f67.google.com ([209.85.210.67]:46340 "EHLO mail-ot1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729505AbfGOUtp (ORCPT ); Mon, 15 Jul 2019 16:49:45 -0400 Received: by mail-ot1-f67.google.com with SMTP id z23so18502901ote.13; Mon, 15 Jul 2019 13:49:44 -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=rHuAoW6rz7p2nDeRv0XfGUjc4uQp5JemfeAPFkJMtUaQnD5wcBfiPye1IMqk9OiJPY I6c2/cXrsd85CQskR03FNaGnDORbctFY7Ke2/gbAcnqnX2wtNhSOyi6Ofl4U3emr0znZ ootuiYWWyUodz1VezolmXG4MmchCIpJ0bvRV31qmRTow+qh1MAS7qPpM7j06MMl0ZDh7 CiaQh/lKzHm1zfzFFLY343h66IOCN7r00vKggGl/6oMI9VWT/LLmMhBSNtduULbuBzqD Bs2JB2eNOp9iuy+DJfvDOVoAvfjTCIIGdg3RkJEc5lx8edw/vBEYgqEvLUiDlD+ykZgT 0YFw== 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=nbs1kMiAGJ+ye62FDf1k7hJkulzfM46l71fvolQp8nnckSpANYB4u3KJ1Bkc5okwaS fmvg92RqLMOOQTCUTAkgwNKKJyasAQaizDLD1rgG7/u957dkbxlJPIYO6SsRZZYevWxG gkTQT5RG+ID0EM408TyN4vpcKM/EXPmCipZiMRlW4VnPbf3FL44019ILedUX8Y3jOiEL j8fjOLA/JJUXPwX0mTdnwRiKfnpEgZT2CTR0GNN0yue/JndcCM8PTc7ulf8JYqAq2+0I 8z50fTDZYX8gcLppo3Ox1JZ4UOFkGas0kL2G1NoXqto8Bk7CFwcgvIFlmxUrokNWQlYS 41og== X-Gm-Message-State: APjAAAXXmIPeKGAt0FtY17vKGX8d3Jv+oI++LHoGbppQqDiV2aRnSmGg GTLjSJGoaSP6kOmsRlXo9FM= X-Google-Smtp-Source: APXvYqxs72/143Cs1oCHNdncKNJa58y29RlhSdtJbJKVT4e2MKR0130YylWHfpP2p9uuAHrasvWZ+g== X-Received: by 2002:a05:6830:193:: with SMTP id q19mr22293791ota.187.1563223784174; Mon, 15 Jul 2019 13:49:44 -0700 (PDT) Received: from [127.0.1.1] ([99.0.85.34]) by smtp.gmail.com with ESMTPSA id w3sm6923532otb.55.2019.07.15.13.49.43 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 15 Jul 2019 13:49:43 -0700 (PDT) Subject: [bpf PATCH v3 6/8] 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, 15 Jul 2019 13:49:42 -0700 Message-ID: <156322378251.18678.18296242426680141239.stgit@john-XPS-13-9370> In-Reply-To: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> References: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> 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 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 15 20:49:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1132291 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="A6oDPw/P"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45nbJ06Z7Tz9sBt for ; Tue, 16 Jul 2019 06:49:52 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732734AbfGOUtw (ORCPT ); Mon, 15 Jul 2019 16:49:52 -0400 Received: from mail-oi1-f193.google.com ([209.85.167.193]:36521 "EHLO mail-oi1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732649AbfGOUtv (ORCPT ); Mon, 15 Jul 2019 16:49:51 -0400 Received: by mail-oi1-f193.google.com with SMTP id w7so13769180oic.3; Mon, 15 Jul 2019 13:49:51 -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=A6oDPw/P5DaD/KV31i8qd/1mEavsEstipFzcCzGzu0sJzWuXKw9nPZTkZJDsuFTI+K +Yw2K0GpcCzHYgMRjwGcoDYWU9oAFF6a7qP8EAjxZIAXCnR2arpXjDQAoCnRoglxzc3j sw7Vnf1P824ybfZ5D2g2iICbhzzo68Z9LkGklGI9MgQw0L+NZmLfcbzSHdXw1KbgcQxK o7WIaXJhasJQ0VlDwLgRTh9Y143AUZr7uqLC69N61xyfS1uThf5Mg5jKnRz1cVUOOfV6 WNStly7n4k7poRNf4+tvfVgiY9vOmnx+pw56590ewW+uDndaHlWcOWpsfgpORRY6I40G S13w== 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=Thbjje2yXFUrwwZCJgUr/lXjRs3zzxjXxV/aucCV19kytbDgnWHWbNusYGPiYXd2Ge KkNgZ9mFBmEqhtOHbWb88pKDzrGPXs3Fn6Smn45ek382Z1GMsIUDcGgigWvL+Mpc7v5L dztqsjG+4U+Jx5eM0OHhls1jzjBK/S2KOABaUfgTO/Xws6Cemmc8nDrBxPHdP+2DTRt8 q8bNQfBtRd0Hw005KjUsYI/PSFXa0Xa4ap3en+RlVYycoMz8J+QyQZcYsr5DrmA5tLq1 G1n2HDVK1rWpeg8I/On8sioj52xsT5sjng8vvQIsca7a4lmZ96c6SPigxWTLGeS2HMX5 BBBw== X-Gm-Message-State: APjAAAUEvQhQGPZy/FH+mbfPZkj4CnEYFj5OmXfrfQvpENpNdbt8VEOv 3T9Eds3YPeu7+opXmGgXvLA= X-Google-Smtp-Source: APXvYqxreGOSOOAh7PHOq4M2LAgBr8ThKgY4J1T50izt/wa85YyiyU5U5VZALXG9S7ZTevrr/3Bq3g== X-Received: by 2002:a05:6808:293:: with SMTP id z19mr14235591oic.13.1563223790790; Mon, 15 Jul 2019 13:49:50 -0700 (PDT) Received: from [127.0.1.1] ([99.0.85.34]) by smtp.gmail.com with ESMTPSA id i11sm6506126oia.9.2019.07.15.13.49.49 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 15 Jul 2019 13:49:50 -0700 (PDT) Subject: [bpf PATCH v3 7/8] 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, 15 Jul 2019 13:49:49 -0700 Message-ID: <156322378927.18678.10640333359699805145.stgit@john-XPS-13-9370> In-Reply-To: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> References: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> 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 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 15 20:49:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1132293 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="E7rGehBd"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45nbJ80jBSz9sNC for ; Tue, 16 Jul 2019 06:50:00 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732768AbfGOUt7 (ORCPT ); Mon, 15 Jul 2019 16:49:59 -0400 Received: from mail-ot1-f66.google.com ([209.85.210.66]:38001 "EHLO mail-ot1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731609AbfGOUt7 (ORCPT ); Mon, 15 Jul 2019 16:49:59 -0400 Received: by mail-ot1-f66.google.com with SMTP id d17so18562109oth.5; Mon, 15 Jul 2019 13:49:57 -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=9UvQoo287w1iW6qT7DydtKV+4jTrc/vB0yeHkxw35KI=; b=E7rGehBdseKUpnNuR+ll7UhuZw0TrFmXXTbEwbssCa8570fBycnpo5sYa8AuRL7HeI 1MF52ChCdPvQ4AXDU1NjDHfTtdmHcOPLDvmpUpWqYYkDvlLZIQQrTV7OwRIs8eTPxiSv CqBMOl5Kx8VTY46uOq8rUJC2Qpf3sBQHh7a+Za+SEVO1vxNkPlBeXO4hXHAmYiWSRPf8 mxYQA6pfYJS4r/MFTpzU/0Os+DVZxo0bIVYykLJyx8umiU/M6D1+YB5NEZR7BAWglVq1 ++70xSTsPm/WZrkBKfxgqlSxIF9lKYFibGUjyyTFIX9ckSJsIMsb7i+r8BWkX9+MfMpr jv0g== 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=9UvQoo287w1iW6qT7DydtKV+4jTrc/vB0yeHkxw35KI=; b=L8WzdZzBuxKfqvAyYHCriGKUXuYsXTQANNz2EurSPBn38HbkRw5CfNyIlLy5kQDALJ 2GgMOIAovuwUtbxsRb7fesYnrBXT9138hnPU3CE7rIKdwvn+lN80ksFoMMC7aioDVrh1 L677DfNMZDTWgLm0jNRN8wDdk+XY671Cr67NE3hg51/1LVAUusvH8a+zlu/+r+i5bOuF R1NYVBPSGI224xhHYw1VeGj2Mbt12JxIe0/uMlLerMSlst9Gdxoskdmyh5Mc3xlpLiKp VK7g/YwX0D+Ks9XGUBkrPKjq6amG9wgDJTeDENCBYltSCgjKqUk5e153KED6uaZMpHMe D40A== X-Gm-Message-State: APjAAAWRSgyuugLoGHoBwucGoHumKiVmNTa/krvsHxrqCLVyC4SU2FYy TqyBkPMvdos0XCz9hAmvThw= X-Google-Smtp-Source: APXvYqwqF3Ph/Fob6FXf+ShI5iIePe/5KAa6pV/aZArpBMxKFL2kN+2s3xm+w9ZROkCkHZdn62hGgQ== X-Received: by 2002:a9d:69cd:: with SMTP id v13mr581553oto.89.1563223797552; Mon, 15 Jul 2019 13:49:57 -0700 (PDT) Received: from [127.0.1.1] ([99.0.85.34]) by smtp.gmail.com with ESMTPSA id i11sm6506202oia.9.2019.07.15.13.49.56 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 15 Jul 2019 13:49:57 -0700 (PDT) Subject: [bpf PATCH v3 8/8] 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, 15 Jul 2019 13:49:56 -0700 Message-ID: <156322379603.18678.372458877012213332.stgit@john-XPS-13-9370> In-Reply-To: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> References: <156322373173.18678.6003379631139659856.stgit@john-XPS-13-9370> 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 | 48 ++++++++++++++++++++++++++++++++++++------------ 5 files changed, 61 insertions(+), 15 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 cca3c59b98bf..f4702c8b9b8c 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 f4cb0522fa95..e67e687f79a2 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -323,15 +323,16 @@ static void tls_sk_proto_unhash(struct sock *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); + + write_lock_bh(&sk->sk_callback_lock); icsk->icsk_ulp_data = NULL; + if (sk->sk_prot->unhash == tls_sk_proto_unhash) + sk->sk_prot = ctx->sk_proto; + write_unlock_bh(&sk->sk_callback_lock); + tls_ctx_free_wq(ctx); if (ctx->unhash) @@ -340,15 +341,17 @@ static void tls_sk_proto_unhash(struct sock *sk) static void tls_sk_proto_close(struct sock *sk, long timeout) { - void (*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); + 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; @@ -356,17 +359,20 @@ 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 || ctx->rx_conf == TLS_HW) tls_sw_strparser_done(ctx); if (ctx->rx_conf == TLS_SW) tls_sw_free_ctx_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); @@ -833,7 +839,7 @@ static int tls_init(struct sock *sk) int rc = 0; if (tls_hw_prot(sk)) - goto out; + return 0; /* The TLS ulp is currently supported only for TCP sockets * in ESTABLISHED state. @@ -844,22 +850,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); @@ -880,6 +903,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)