xfrm: espintcp: reorder operations within espintcp_init_sk

When enabled on a socket, espintcp sets the socket callbacks to its
own before it has finished setting up its context and published it as
icsk_ulp_data. Any one of those callbacks that gets called before will
dereference a NULL icsk_ulp_data.

Fix this by reording the operations, and add memory barriers to ensure
callers will have a struct espintcp_ctx available. Add a few
WRITE_ONCE/READ_ONCE too while we're there.

Fixes: e27cca96cd ("xfrm: add espintcp (RFC 8229)")
Cc: stable@vger.kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Reported-by: Peihan Liu <ronbogo@outlook.com>
Reported-by: Yilin Zhu <zylzyl2333@gmail.com>
Reported-by: Ren Wei <n05ec@lzu.edu.cn>
Reported-by: Eulgyu Kim <eulgyukim@snu.ac.kr>
Reported-by: Jaeyoung Chung <jjy600901@snu.ac.kr>
Link: https://lore.kernel.org/all/c30b645074a1b379e0f7fe297f917c66137d9964.1778464688.git.zylzyl2333@gmail.com/
Link: https://lore.kernel.org/all/20260819155349.3555804-1-jjy600901@snu.ac.kr
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
This commit is contained in:
Sabrina Dubroca 2026-10-08 14:38:32 +02:00 committed by Steffen Klassert
parent e35a12dffd
commit f9a11716c8
2 changed files with 29 additions and 19 deletions

View File

@ -34,7 +34,8 @@ static inline struct espintcp_ctx *espintcp_getctx(const struct sock *sk)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
/* RCU is only needed for diag */
return (__force void *)icsk->icsk_ulp_data;
/* pairs with smp_wmb() in espintcp_init_sk() */
smp_rmb();
return (__force void *)READ_ONCE(icsk->icsk_ulp_data);
}
#endif

View File

@ -434,7 +434,9 @@ static void espintcp_destruct(struct sock *sk)
bool tcp_is_ulp_esp(struct sock *sk)
{
return sk->sk_prot == &espintcp_prot || sk->sk_prot == &espintcp6_prot;
const struct proto *prot = READ_ONCE(sk->sk_prot);
return prot == &espintcp_prot || prot == &espintcp6_prot;
}
EXPORT_SYMBOL_GPL(tcp_is_ulp_esp);
@ -466,34 +468,41 @@ static int espintcp_init_sk(struct sock *sk)
__sk_dst_reset(sk);
strp_check_rcv(&ctx->strp);
skb_queue_head_init(&ctx->ike_queue);
skb_queue_head_init(&ctx->out_queue);
INIT_WORK(&ctx->work, espintcp_tx_work);
ctx->saved_data_ready = sk->sk_data_ready;
ctx->saved_write_space = sk->sk_write_space;
ctx->saved_destruct = sk->sk_destruct;
/* avoid using task_frag */
sk->sk_allocation = GFP_ATOMIC;
sk->sk_use_task_frag = false;
rcu_assign_pointer(icsk->icsk_ulp_data, ctx);
/* pairs with smp_rmb() in espintcp_getctx() */
smp_wmb();
WRITE_ONCE(sk->sk_data_ready, espintcp_data_ready);
WRITE_ONCE(sk->sk_write_space, espintcp_write_space);
sk->sk_destruct = espintcp_destruct;
if (sk->sk_family == AF_INET) {
sk->sk_prot = &espintcp_prot;
sk->sk_socket->ops = &espintcp_ops;
WRITE_ONCE(sk->sk_prot, &espintcp_prot);
WRITE_ONCE(sk->sk_socket->ops, &espintcp_ops);
} else {
mutex_lock(&tcpv6_prot_mutex);
if (!espintcp6_prot.recvmsg)
build_protos(&espintcp6_prot, &espintcp6_ops, sk->sk_prot, sk->sk_socket->ops);
mutex_unlock(&tcpv6_prot_mutex);
sk->sk_prot = &espintcp6_prot;
sk->sk_socket->ops = &espintcp6_ops;
WRITE_ONCE(sk->sk_prot, &espintcp6_prot);
WRITE_ONCE(sk->sk_socket->ops, &espintcp6_ops);
}
ctx->saved_data_ready = sk->sk_data_ready;
ctx->saved_write_space = sk->sk_write_space;
ctx->saved_destruct = sk->sk_destruct;
sk->sk_data_ready = espintcp_data_ready;
sk->sk_write_space = espintcp_write_space;
sk->sk_destruct = espintcp_destruct;
rcu_assign_pointer(icsk->icsk_ulp_data, ctx);
INIT_WORK(&ctx->work, espintcp_tx_work);
/* avoid using task_frag */
sk->sk_allocation = GFP_ATOMIC;
sk->sk_use_task_frag = false;
strp_check_rcv(&ctx->strp);
return 0;