Liu Jian wrote:
> With two Msgs, msgA and msgB and a user doing nonblocking sendmsg calls (or
> multiple cores) on a single socket 'sk' we could get the following flow.
>
> msgA, sk msgB, sk
> ----------- ---------------
> tcp_bpf_sendmsg()
> lock(sk)
> psock = sk->psock
> tcp_bpf_sendmsg()
> lock(sk) ... blocking
> tcp_bpf_send_verdict
> if (psock->eval == NONE)
> psock->eval = sk_psock_msg_verdict
> ..
> < handle SK_REDIRECT case >
> release_sock(sk) < lock dropped so grab here >
> ret = tcp_bpf_sendmsg_redir
> psock = sk->psock
> tcp_bpf_send_verdict
> lock_sock(sk) ... blocking on B
> if (psock->eval == NONE) <- boom.
> psock->eval will have msgA state
>
> The problem here is we dropped the lock on msgA and grabbed it with msgB.
> Now we have old state in psock and importantly psock->eval has not been
> cleared. So msgB will run whatever action was done on A and the verdict
> program may never see it.
>
> Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
> Signed-off-by: Liu Jian <liujian56@xxxxxxxxxx>
Yep thanks for digging into this. Nice catch. And commit looks good now.
Acked-by: John Fastabend <john.fastabend@xxxxxxxxx>
