From f13fe3e60c799e0fce643458b3bb336bf444cd25 Mon Sep 17 00:00:00 2001 From: Jakub Sitnicki Date: Tue, 17 Mar 2020 18:04:37 +0100 Subject: [PATCH 1/3] net/tls: Constify base proto ops used for building tls proto The helper that builds kTLS proto ops doesn't need to and should not modify the base proto ops. Annotate the parameter as read-only. Signed-off-by: Jakub Sitnicki Signed-off-by: David S. Miller --- net/tls/tls_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index 82225bcc1117c..ff08b2ff75979 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -63,7 +63,7 @@ static DEFINE_MUTEX(tcpv4_prot_mutex); static struct proto tls_prots[TLS_NUM_PROTS][TLS_NUM_CONFIG][TLS_NUM_CONFIG]; static struct proto_ops tls_sw_proto_ops; static void build_protos(struct proto prot[TLS_NUM_CONFIG][TLS_NUM_CONFIG], - struct proto *base); + const struct proto *base); void update_sk_prot(struct sock *sk, struct tls_context *ctx) { @@ -652,7 +652,7 @@ static void tls_build_proto(struct sock *sk) } static void build_protos(struct proto prot[TLS_NUM_CONFIG][TLS_NUM_CONFIG], - struct proto *base) + const struct proto *base) { prot[TLS_BASE][TLS_BASE] = *base; prot[TLS_BASE][TLS_BASE].setsockopt = tls_setsockopt; From 5bb4c45d466cb28dd0a7f15f80c4cb7768054e94 Mon Sep 17 00:00:00 2001 From: Jakub Sitnicki Date: Tue, 17 Mar 2020 18:04:38 +0100 Subject: [PATCH 2/3] net/tls: Read sk_prot once when building tls proto ops Apart from being a "tremendous" win when it comes to generated machine code (see bloat-o-meter output for x86-64 below) this mainly prepares ground for annotating access to sk_prot with READ_ONCE, so that we don't pepper the code with access annotations and needlessly repeat loads. add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-46 (-46) Function old new delta tls_init 851 805 -46 Total: Before=21063, After=21017, chg -0.22% Signed-off-by: Jakub Sitnicki Signed-off-by: David S. Miller --- net/tls/tls_main.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index ff08b2ff75979..e7de0306a7dfd 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -628,24 +628,25 @@ struct tls_context *tls_ctx_create(struct sock *sk) static void tls_build_proto(struct sock *sk) { int ip_ver = sk->sk_family == AF_INET6 ? TLSV6 : TLSV4; + const struct proto *prot = sk->sk_prot; /* Build IPv6 TLS whenever the address of tcpv6 _prot changes */ if (ip_ver == TLSV6 && - unlikely(sk->sk_prot != smp_load_acquire(&saved_tcpv6_prot))) { + unlikely(prot != smp_load_acquire(&saved_tcpv6_prot))) { mutex_lock(&tcpv6_prot_mutex); - if (likely(sk->sk_prot != saved_tcpv6_prot)) { - build_protos(tls_prots[TLSV6], sk->sk_prot); - smp_store_release(&saved_tcpv6_prot, sk->sk_prot); + if (likely(prot != saved_tcpv6_prot)) { + build_protos(tls_prots[TLSV6], prot); + smp_store_release(&saved_tcpv6_prot, prot); } mutex_unlock(&tcpv6_prot_mutex); } if (ip_ver == TLSV4 && - unlikely(sk->sk_prot != smp_load_acquire(&saved_tcpv4_prot))) { + unlikely(prot != smp_load_acquire(&saved_tcpv4_prot))) { mutex_lock(&tcpv4_prot_mutex); - if (likely(sk->sk_prot != saved_tcpv4_prot)) { - build_protos(tls_prots[TLSV4], sk->sk_prot); - smp_store_release(&saved_tcpv4_prot, sk->sk_prot); + if (likely(prot != saved_tcpv4_prot)) { + build_protos(tls_prots[TLSV4], prot); + smp_store_release(&saved_tcpv4_prot, prot); } mutex_unlock(&tcpv4_prot_mutex); } From d5bee7374b68de3c44586d46e9e61ffc97a1e886 Mon Sep 17 00:00:00 2001 From: Jakub Sitnicki Date: Tue, 17 Mar 2020 18:04:39 +0100 Subject: [PATCH 3/3] net/tls: Annotate access to sk_prot with READ_ONCE/WRITE_ONCE sockmap performs lockless writes to sk->sk_prot on the following paths: tcp_bpf_{recvmsg|sendmsg} / sock_map_unref sk_psock_put sk_psock_drop sk_psock_restore_proto WRITE_ONCE(sk->sk_prot, proto) To prevent load/store tearing [1], and to make tooling aware of intentional shared access [2], we need to annotate other sites that access sk_prot with READ_ONCE/WRITE_ONCE macros. Change done with Coccinelle with following semantic patch: @@ expression E; identifier I; struct sock *sk; identifier sk_prot =~ "^sk_prot$"; @@ ( E = -sk->sk_prot +READ_ONCE(sk->sk_prot) | -sk->sk_prot = E +WRITE_ONCE(sk->sk_prot, E) | -sk->sk_prot +READ_ONCE(sk->sk_prot) ->I ) Signed-off-by: Jakub Sitnicki Signed-off-by: David S. Miller --- net/tls/tls_device.c | 2 +- net/tls/tls_main.c | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c index 1c5574e2e0582..a562ebaaa33c3 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -366,7 +366,7 @@ static int tls_do_allocation(struct sock *sk, if (!offload_ctx->open_record) { if (unlikely(!skb_page_frag_refill(prepend_size, pfrag, sk->sk_allocation))) { - sk->sk_prot->enter_memory_pressure(sk); + READ_ONCE(sk->sk_prot)->enter_memory_pressure(sk); sk_stream_moderate_sndbuf(sk); return -ENOMEM; } diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index e7de0306a7dfd..156efce50dbd1 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -69,7 +69,8 @@ void update_sk_prot(struct sock *sk, struct tls_context *ctx) { int ip_ver = sk->sk_family == AF_INET6 ? TLSV6 : TLSV4; - sk->sk_prot = &tls_prots[ip_ver][ctx->tx_conf][ctx->rx_conf]; + WRITE_ONCE(sk->sk_prot, + &tls_prots[ip_ver][ctx->tx_conf][ctx->rx_conf]); } int wait_on_pending_writer(struct sock *sk, long *timeo) @@ -312,7 +313,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout) write_lock_bh(&sk->sk_callback_lock); if (free_ctx) rcu_assign_pointer(icsk->icsk_ulp_data, NULL); - sk->sk_prot = ctx->sk_proto; + WRITE_ONCE(sk->sk_prot, ctx->sk_proto); if (sk->sk_write_space == tls_write_space) sk->sk_write_space = ctx->sk_write_space; write_unlock_bh(&sk->sk_callback_lock); @@ -621,14 +622,14 @@ struct tls_context *tls_ctx_create(struct sock *sk) mutex_init(&ctx->tx_lock); rcu_assign_pointer(icsk->icsk_ulp_data, ctx); - ctx->sk_proto = sk->sk_prot; + ctx->sk_proto = READ_ONCE(sk->sk_prot); return ctx; } static void tls_build_proto(struct sock *sk) { int ip_ver = sk->sk_family == AF_INET6 ? TLSV6 : TLSV4; - const struct proto *prot = sk->sk_prot; + const struct proto *prot = READ_ONCE(sk->sk_prot); /* Build IPv6 TLS whenever the address of tcpv6 _prot changes */ if (ip_ver == TLSV6 &&