5.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: Dae R. Jeong threeearcat@gmail.com
[ Upstream commit 91e61dd7a0af660408e87372d8330ceb218be302 ]
In tls_init(), a write memory barrier is missing, and store-store reordering may cause NULL dereference in tls_{setsockopt,getsockopt}.
CPU0 CPU1 ----- ----- // In tls_init() // In tls_ctx_create() ctx = kzalloc() ctx->sk_proto = READ_ONCE(sk->sk_prot) -(1)
// In update_sk_prot() WRITE_ONCE(sk->sk_prot, tls_prots) -(2)
// In sock_common_setsockopt() READ_ONCE(sk->sk_prot)->setsockopt()
// In tls_{setsockopt,getsockopt}() ctx->sk_proto->setsockopt() -(3)
In the above scenario, when (1) and (2) are reordered, (3) can observe the NULL value of ctx->sk_proto, causing NULL dereference.
To fix it, we rely on rcu_assign_pointer() which implies the release barrier semantic. By moving rcu_assign_pointer() after ctx->sk_proto is initialized, we can ensure that ctx->sk_proto are visible when changing sk->sk_prot.
Fixes: d5bee7374b68 ("net/tls: Annotate access to sk_prot with READ_ONCE/WRITE_ONCE") Signed-off-by: Yewon Choi woni9911@gmail.com Signed-off-by: Dae R. Jeong threeearcat@gmail.com Link: https://lore.kernel.org/netdev/ZU4OJG56g2V9z_H7@dragonet/T/ Link: https://lore.kernel.org/r/Zkx4vjSFp0mfpjQ2@libra05 Signed-off-by: Paolo Abeni pabeni@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- net/tls/tls_main.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index 2bbacd9b97e56..ebf856cf821da 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -633,9 +633,17 @@ struct tls_context *tls_ctx_create(struct sock *sk) return NULL;
mutex_init(&ctx->tx_lock); - rcu_assign_pointer(icsk->icsk_ulp_data, ctx); ctx->sk_proto = READ_ONCE(sk->sk_prot); ctx->sk = sk; + /* Release semantic of rcu_assign_pointer() ensures that + * ctx->sk_proto is visible before changing sk->sk_prot in + * update_sk_prot(), and prevents reading uninitialized value in + * tls_{getsockopt, setsockopt}. Note that we do not need a + * read barrier in tls_{getsockopt,setsockopt} as there is an + * address dependency between sk->sk_proto->{getsockopt,setsockopt} + * and ctx->sk_proto. + */ + rcu_assign_pointer(icsk->icsk_ulp_data, ctx); return ctx; }