net/tls: remove sock unlock/lock around strp_done()

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 <john.fastabend@gmail.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

diff --git a/include/net/tls.h b/include/net/tls.h
index d4276cb6de5354bfb11a98647640ebcced23823b..235508e35fd4538eef03a605e08584e57e09a0a5 100644 (file)
--- 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,14 +355,17 @@ 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);
 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_release_resources_tx(struct sock *sk);
+void tls_sw_free_ctx_tx(struct tls_context *tls_ctx);
 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 4d67d72f007c88ac86d9658d92af10e4b8af42dc..7c0b2b778703f7080c28d4919084390d4da05e7c 100644 (file)
--- 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 5c29b410cf7d1552513d627066ac8f514722d493..d152a00a7a27fd024251903218032d42242c7b78 100644 (file)
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -261,24 +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_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);
@@ -287,7 +272,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
        if (ctx->tx_conf == TLS_SW) {
                kfree(ctx->tx.rec_seq);
                kfree(ctx->tx.iv);
-               tls_sw_free_resources_tx(sk);
+               tls_sw_release_resources_tx(sk);
 #ifdef CONFIG_TLS_DEVICE
        } else if (ctx->tx_conf == TLS_HW) {
                tls_device_free_resources_tx(sk);
@@ -295,26 +280,40 @@ 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);
+       bool free_ctx;
+
+       if (ctx->tx_conf == TLS_SW)
+               tls_sw_cancel_work_tx(ctx);
+
+       lock_sock(sk);
+       free_ctx = ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW;
+       sk_proto_close = ctx->sk_proto_close;
+
+       if (ctx->tx_conf != TLS_BASE || ctx->rx_conf != TLS_BASE)
+               tls_sk_proto_cleanup(sk, ctx, timeo);
 
-skip_tx_cleanup:
        release_sock(sk);
+       if (ctx->tx_conf == TLS_SW)
+               tls_sw_free_ctx_tx(ctx);
+       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)
                tls_ctx_free(ctx);
 }
@@ -541,9 +540,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 38c0e53c727d408c2d9575a76d4dd2459688b6e9..91d21b048a9b245804ba800d7e9dbc1b5c07a868 100644 (file)
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2063,7 +2063,7 @@ void tls_sw_cancel_work_tx(struct tls_context *tls_ctx)
        cancel_delayed_work_sync(&ctx->tx_work.work);
 }
 
-void tls_sw_free_resources_tx(struct sock *sk)
+void tls_sw_release_resources_tx(struct sock *sk)
 {
        struct tls_context *tls_ctx = tls_get_ctx(sk);
        struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
@@ -2096,6 +2096,11 @@ void tls_sw_free_resources_tx(struct sock *sk)
 
        crypto_free_aead(ctx->aead_send);
        tls_free_open_rec(sk);
+}
+
+void tls_sw_free_ctx_tx(struct tls_context *tls_ctx)
+{
+       struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
 
        kfree(ctx);
 }
@@ -2114,25 +2119,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)
 {