Commit 55597bed authored by Dae R. Jeong's avatar Dae R. Jeong Committed by Liu Jian
Browse files

tls: fix missing memory barrier in tls_init

stable inclusion
from stable-v5.10.219
commit d72e126e9a36d3d33889829df8fc90100bb0e071
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IA7D88
CVE: CVE-2024-36489

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=d72e126e9a36d3d33889829df8fc90100bb0e071



---------------------------

[ 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: d5bee737 ("net/tls: Annotate access to sk_prot with READ_ONCE/WRITE_ONCE")
Signed-off-by: default avatarYewon Choi <woni9911@gmail.com>
Signed-off-by: default avatarDae 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: default avatarPaolo Abeni <pabeni@redhat.com>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
Signed-off-by: default avatarLiu Jian <liujian56@huawei.com>
parent 27858387
Loading
Loading
Loading
Loading
+9 −1
Original line number Diff line number Diff line
@@ -708,9 +708,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;
}