* Re: [PATCH net 1/1] af_unix: Hold receive queue lock in ioctl(SIOCATMARK)
2026-04-10 6:31 ` [PATCH net 1/1] af_unix: Hold receive queue lock in ioctl(SIOCATMARK) Ren Wei
@ 2026-04-10 8:48 ` Eric Dumazet
2026-04-10 20:06 ` Kuniyuki Iwashima
1 sibling, 0 replies; 3+ messages in thread
From: Eric Dumazet @ 2026-04-10 8:48 UTC (permalink / raw)
To: Ren Wei
Cc: netdev, kuniyu, davem, kuba, pabeni, horms, rao.shoaib, yifanwucs,
tomapufckgml, yuantan098, bird, enjou1224z, wangjiexun2025
On Thu, Apr 9, 2026 at 11:32 PM Ren Wei <n05ec@lzu.edu.cn> wrote:
>
> From: Jiexun Wang <wangjiexun2025@gmail.com>
>
> unix_ioctl() peeks at the receive queue and may check both the head skb
> and its successor while deciding whether SIOCATMARK should report the
> mark. However, u->iolock does not stabilize receive-queue element
> lifetime. Queue teardown paths can purge or splice the queue under
> sk->sk_receive_queue.lock and free the skb while unix_ioctl() still
> uses it.
>
> Take sk->sk_receive_queue.lock while inspecting the queue so the skb
> and next_skb stay alive for the whole decision.
>
> Fixes: 314001f0bf92 ("af_unix: Add OOB support")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Ren Wei <enjou1224z@gmail.com>
> Signed-off-by: Jiexun Wang <wangjiexun2025@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> net/unix/af_unix.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index b23c33df8b46..54f12d5cda37 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -3301,6 +3301,8 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> int answ = 0;
>
> mutex_lock(&u->iolock);
> + /* The receive queue lock keeps skb and next_skb alive. */
> + spin_lock(&sk->sk_receive_queue.lock);
>
> skb = skb_peek(&sk->sk_receive_queue);
> if (skb) {
> @@ -3315,6 +3317,7 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> answ = 1;
> }
>
> + spin_unlock(&sk->sk_receive_queue.lock);
> mutex_unlock(&u->iolock);
>
> err = put_user(answ, (int __user *)arg);
Ok, but I think we also should remove the READ_ONCE() now we have
proper locking.
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 5d4baac499af8d0a9cc36f652acf129a2e3619b2..1c3b80b25ef8b37d8e6553066fb7da20d81294a5
100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -3301,10 +3301,12 @@ static int unix_ioctl(struct socket *sock,
unsigned int cmd, unsigned long arg)
int answ = 0;
mutex_lock(&u->iolock);
+ /* The receive queue lock keeps skb and
next_skb alive. */
+ spin_lock(&sk->sk_receive_queue.lock);
skb = skb_peek(&sk->sk_receive_queue);
if (skb) {
- struct sk_buff *oob_skb = READ_ONCE(u->oob_skb);
+ struct sk_buff *oob_skb = u->oob_skb;
struct sk_buff *next_skb;
next_skb = skb_peek_next(skb,
&sk->sk_receive_queue);
@@ -3314,7 +3316,7 @@ static int unix_ioctl(struct socket *sock,
unsigned int cmd, unsigned long arg)
(!oob_skb || next_skb == oob_skb)))
answ = 1;
}
-
+ spin_unlock(&sk->sk_receive_queue.lock);
mutex_unlock(&u->iolock);
err = put_user(answ, (int __user *)arg);
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net 1/1] af_unix: Hold receive queue lock in ioctl(SIOCATMARK)
2026-04-10 6:31 ` [PATCH net 1/1] af_unix: Hold receive queue lock in ioctl(SIOCATMARK) Ren Wei
2026-04-10 8:48 ` Eric Dumazet
@ 2026-04-10 20:06 ` Kuniyuki Iwashima
1 sibling, 0 replies; 3+ messages in thread
From: Kuniyuki Iwashima @ 2026-04-10 20:06 UTC (permalink / raw)
To: n05ec
Cc: bird, davem, edumazet, enjou1224z, horms, kuba, kuniyu, netdev,
pabeni, rao.shoaib, tomapufckgml, wangjiexun2025, yifanwucs,
yuantan098
From: Ren Wei <n05ec@lzu.edu.cn>
Date: Fri, 10 Apr 2026 14:31:57 +0800
> From: Jiexun Wang <wangjiexun2025@gmail.com>
>
> unix_ioctl() peeks at the receive queue and may check both the head skb
> and its successor while deciding whether SIOCATMARK should report the
> mark. However, u->iolock does not stabilize receive-queue element
> lifetime. Queue teardown paths can purge or splice the queue under
Please be more specific here.
> sk->sk_receive_queue.lock and free the skb while unix_ioctl() still
> uses it.
>
> Take sk->sk_receive_queue.lock while inspecting the queue so the skb
> and next_skb stay alive for the whole decision.
>
> Fixes: 314001f0bf92 ("af_unix: Add OOB support")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Ren Wei <enjou1224z@gmail.com>
> Signed-off-by: Jiexun Wang <wangjiexun2025@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> net/unix/af_unix.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index b23c33df8b46..54f12d5cda37 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -3301,6 +3301,8 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> int answ = 0;
>
> mutex_lock(&u->iolock);
> + /* The receive queue lock keeps skb and next_skb alive. */
> + spin_lock(&sk->sk_receive_queue.lock);
I think this is not the correct fix.
SIOCATMARK is apparently for MSG_OOB,
$ man 3 sockatmark
and non SOCK_STREAM sockets do not support MSG_OOB
and should not abuse SIOCATMARK.
---8<---
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index eebabf0bd850..868b26c963ab 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -3299,6 +3299,9 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
struct sk_buff *skb;
int answ = 0;
+ if (sk->sk_type != SOCK_STREAM)
+ return -EOPNOTSUPP;
+
mutex_lock(&u->iolock);
skb = skb_peek(&sk->sk_receive_queue);
---8<---
^ permalink raw reply related [flat|nested] 3+ messages in thread