Sorry for not mentioning this before, but could you replace "net" with "bpf" in Subject and rebase the patch on bpf.git so that we can trigger the patchwork's CI ? https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git From: Michal Luczaj <mhal@xxxxxxx> Date: Thu, 20 Jun 2024 22:20:05 +0200 > AF_UNIX socket tracks the most recent OOB packet (in its receive queue) > with an `oob_skb` pointer. BPF redirecting does not account for that: when > an OOB packet is moved between sockets, `oob_skb` is left outdated. This > results in a single skb that may be accessed from two different sockets. > > Take the easy way out: silently drop MSG_OOB data targeting any socket that > is in a sockmap or a sockhash. Note that such silent drop is akin to the > fate of redirected skb's scm_fp_list (SCM_RIGHTS, SCM_CREDENTIALS). > > For symmetry, forbid MSG_OOB in unix_bpf_recvmsg(). > > Suggested-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> > Fixes: 314001f0bf92 ("af_unix: Add OOB support") > Signed-off-by: Michal Luczaj <mhal@xxxxxxx> > --- > net/unix/af_unix.c | 30 +++++++++++++++++++++++++++++- > net/unix/unix_bpf.c | 3 +++ > 2 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c > index 5e695a9a609c..3a55d075f199 100644 > --- a/net/unix/af_unix.c > +++ b/net/unix/af_unix.c > @@ -2653,10 +2653,38 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk, > > static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor) > { > + struct unix_sock *u = unix_sk(sk); > + struct sk_buff *skb; > + int err; > + > if (unlikely(READ_ONCE(sk->sk_state) != TCP_ESTABLISHED)) > return -ENOTCONN; > > - return unix_read_skb(sk, recv_actor); > + mutex_lock(&u->iolock); > + skb = skb_recv_datagram(sk, MSG_DONTWAIT, &err); mutex_unlock(&u->iolock); I think we can drop mutex here as the skb is already unlinked and no receiver can touch it. and the below part can be like the following not to slow down the common case: if (!skb) return err; > + > +#if IS_ENABLED(CONFIG_AF_UNIX_OOB) > + if (skb) { if (unlikely(skb == READ_ONCE(u->oob_skb))) { > + bool drop = false; > + > + spin_lock(&sk->sk_receive_queue.lock); > + if (skb == u->oob_skb) { if (likely(skb == u->oob_skb)) { > + WRITE_ONCE(u->oob_skb, NULL); > + drop = true; > + } > + spin_unlock(&sk->sk_receive_queue.lock); > + > + if (drop) { > + WARN_ON_ONCE(skb_unref(skb)); > + kfree_skb(skb); > + skb = NULL; > + err = -EAGAIN; return -EAGAIN; > + } > + } > +#endif return recv_actor(sk, skb); Thanks! > + > + mutex_unlock(&u->iolock); > + return skb ? recv_actor(sk, skb) : err; > } > > static int unix_stream_read_generic(struct unix_stream_read_state *state, > diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c > index bd84785bf8d6..bca2d86ba97d 100644 > --- a/net/unix/unix_bpf.c > +++ b/net/unix/unix_bpf.c > @@ -54,6 +54,9 @@ static int unix_bpf_recvmsg(struct sock *sk, struct msghdr *msg, > struct sk_psock *psock; > int copied; > > + if (flags & MSG_OOB) > + return -EOPNOTSUPP; > + > if (!len) > return 0; > > -- > 2.45.1
