Skip to content

Commit

Permalink
net/tls: remove sock unlock/lock around strp_done()
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
John Fastabend authored and Daniel Borkmann committed Jul 22, 2019
1 parent f87e62d commit 313ab00
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 45 deletions.
7 changes: 4 additions & 3 deletions include/net/tls.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion net/tls/tls_device.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
61 changes: 30 additions & 31 deletions net/tls/tls_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -287,34 +272,48 @@ 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);
#endif
}

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);
}
Expand Down Expand Up @@ -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)
Expand Down
40 changes: 30 additions & 10 deletions net/tls/tls_sw.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand All @@ -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)
{
Expand Down

0 comments on commit 313ab00

Please sign in to comment.