netdev - Re: [PATCH] af_unix: don't post cmsg for SO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <72a26b79-a469-4e6e-b0f0-92c72014e7fb@kernel.dk>
Date: Wed, 17 Dec 2025 19:24:53 -0700
From: Jens Axboe <axboe@...nel.dk>
To: netdev <netdev@...r.kernel.org>
Cc: Jakub Kicinski <kuba@...nel.org>, Kuniyuki Iwashima <kuniyu@...gle.com>,
 Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH] af_unix: don't post cmsg for SO_INQ unless explicitly
 asked for

On 12/17/25 7:19 PM, Jens Axboe wrote:
> A previous commit added SO_INQ support for AF_UNIX (SOCK_STREAM), but
> it posts a SCM_INQ cmsg even if just msg->msg_get_inq is set. This is
> incorrect, as ->msg_get_inq is just the caller asking for the remainder
> to be passed back in msg->msg_inq, it has nothing to do with cmsg. The
> original commit states that this is done to make sockets
> io_uring-friendly", but it's actually incorrect as io_uring doesn't
> use cmsg headers internally at all, and it's actively wrong as this
> means that cmsg's are always posted if someone does recvmsg via
> io_uring.
> 
> Fix that up by only posting cmsg if u->recvmsg_inq is set.
> 
> Cc: stable@...r.kernel.org
> Fixes: df30285b3670 ("af_unix: Introduce SO_INQ.")
> Signed-off-by: Jens Axboe <axboe@...nel.dk>
> 
> ---
> 
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 55cdebfa0da0..110d716087b5 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -3086,12 +3086,16 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
>  
>  	mutex_unlock(&u->iolock);
>  	if (msg) {
> +		bool do_cmsg;
> +
>  		scm_recv_unix(sock, msg, &scm, flags);
>  
> -		if (READ_ONCE(u->recvmsg_inq) || msg->msg_get_inq) {
> +		do_cmsg = READ_ONCE(u->recvmsg_inq);
> +		if (do_cmsg || msg->msg_get_inq) {
>  			msg->msg_inq = READ_ONCE(u->inq_len);
> -			put_cmsg(msg, SOL_SOCKET, SCM_INQ,
> -				 sizeof(msg->msg_inq), &msg->msg_inq);
> +			if (do_cmsg)
> +				put_cmsg(msg, SOL_SOCKET, SCM_INQ,
> +					 sizeof(msg->msg_inq), &msg->msg_inq);
>  		}
>  	} else {
>  		scm_destroy(&scm);
> 

Note, on top of this bug, I also believe the correct check here should be:

if ((do_cmsg || msg->msg_get_inq) && copied >= 0)

rather than always post a cmsg (or pass back inq data) if the socket
read has failed.

Was going to post that patch separately, but can fold it into this one
as well. Let me know.

Also note that this is commit is actively breaking some io_uring uses on
streamed sockets, as you can now end up with multiple SCM_INQ cmsg
postings per socket with retries. These were not requested. So would
appreciate if we can get this one sorted out soonish and post for stable
too. It affects 6.17 and newer.

-- 
Jens Axboe