Skip to content

Commit 91e61dd

Browse files
threeearcatPaolo Abeni
authored andcommitted
tls: fix missing memory barrier in tls_init
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: d5bee73 ("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>
1 parent 3b1c92f commit 91e61dd

1 file changed

Lines changed: 9 additions & 1 deletion

File tree

net/tls/tls_main.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,9 +816,17 @@ struct tls_context *tls_ctx_create(struct sock *sk)
816816
return NULL;
817817

818818
mutex_init(&ctx->tx_lock);
819-
rcu_assign_pointer(icsk->icsk_ulp_data, ctx);
820819
ctx->sk_proto = READ_ONCE(sk->sk_prot);
821820
ctx->sk = sk;
821+
/* Release semantic of rcu_assign_pointer() ensures that
822+
* ctx->sk_proto is visible before changing sk->sk_prot in
823+
* update_sk_prot(), and prevents reading uninitialized value in
824+
* tls_{getsockopt, setsockopt}. Note that we do not need a
825+
* read barrier in tls_{getsockopt,setsockopt} as there is an
826+
* address dependency between sk->sk_proto->{getsockopt,setsockopt}
827+
* and ctx->sk_proto.
828+
*/
829+
rcu_assign_pointer(icsk->icsk_ulp_data, ctx);
822830
return ctx;
823831
}
824832

0 commit comments

Comments
 (0)