Re: [PATCH bpf-next v3 2/3] bpf, sockmap: Fix FIONREAD for sockmap

bot+bpf-ci
2025-11-21T14:49:48Z

[-- Attachment #1: Type: text/plain, Size: 2324 bytes --]

> diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
> index 49847888c..87772846f 100644
> --- a/include/linux/skmsg.h
> +++ b/include/linux/skmsg.h

[ ... ]

> +static inline ssize_t sk_psock_get_msg_size(struct sk_psock *psock)
> +{
> + return psock->ingress_size;
> +}

[ ... ]

> +static inline ssize_t sk_psock_msg_inq(struct sock *sk)
> +{
> + struct sk_psock *psock;
> + ssize_t inq = 0;
> +
> + psock = sk_psock_get(sk);
> + if (likely(psock)) {
> + inq = sk_psock_get_msg_size(psock);
> + sk_psock_put(sk, psock);
> + }
> + return inq;
> +}

Does this create a data race on ingress_size? The field is read here
without holding ingress_lock, but all modifications happen under that
lock in sk_psock_queue_msg(), sk_psock_dequeue_msg(), and
sk_msg_recvmsg().

When ioctl(FIONREAD) executes concurrently with recv(), one thread
reads ingress_size locklessly in sk_psock_msg_inq() while another
thread modifies it under lock in sk_msg_recvmsg(). On 32-bit systems
with 32-bit ssize_t, this could produce torn reads.

Would READ_ONCE/WRITE_ONCE be needed for the lockless access?

> +/* for udp */
> +static inline ssize_t sk_msg_first_length(struct sock *sk)
> +{
> + struct sk_psock *psock;
> + struct sk_msg *msg;
> + ssize_t inq = 0;
> +
> + psock = sk_psock_get(sk);
> + if (likely(psock)) {
> + msg = sk_psock_peek_msg(psock);
> + if (msg)
> + inq = msg->sg.size;
> + sk_psock_put(sk, psock);
> + }
> + return inq;
> +}

Can msg be freed between sk_psock_peek_msg() and reading msg->sg.size?
The peek function releases ingress_lock before returning the pointer.
If another thread in sk_msg_recvmsg() fully consumes the message after
the peek but before the size read, won't sk_psock_dequeue_msg() free
the message, leading to a use-after-free when we read msg->sg.size?

The call chain would be:
Thread A: udp_bpf_ioctl()->sk_msg_first_length()->sk_psock_peek_msg()
returns msg, releases lock, then reads msg->sg.size
Thread B: udp_bpf_recvmsg()->sk_msg_recvmsg() consumes msg fully,
calls sk_psock_dequeue_msg() which frees msg

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19573628679