[PATCHSET 0/2] Fix SO_INQ for af_unix

[PATCHSET 0/2] Fix SO_INQ for af_unix

[Jens Axboe]

Hi,

We ran into an issue with the recently added SO_INQ support for
unix/stream sockets. First patch fixes the unconditional posting of
cmsg for io_uring cases, which it should not do, and the second patch
fixes the condition for when to post an SO_INQ cmsg in general (which
is only for the non-error case).

Please review and apply if you're happy with them, these patches fix
a regression introduced in 6.17 and newer kernels and hence are marked
for stable as well.

 net/unix/af_unix.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

-- 
Jens Axboe

[PATCH 1/2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for

[Jens Axboe]

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@vger.kernel.org
Fixes: df30285b3670 ("af_unix: Introduce SO_INQ.")
Reported-by: Julian Orth <ju.orth@gmail.com>
Link: https://github.com/axboe/liburing/issues/1509
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 net/unix/af_unix.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

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);
-- 
2.51.0

[PATCH 2/2] af_unix: only post SO_INQ cmsg for non-error case

[Jens Axboe]

As is done for TCP sockets, don't post an SCM_INQ cmsg for an error
case. Only post them for the non-error case, which is when
unix_stream_read_generic will return >= 0.

Cc: stable@vger.kernel.org
Fixes: df30285b3670 ("af_unix: Introduce SO_INQ.")
Reported-by: Julian Orth <ju.orth@gmail.com>
Link: https://github.com/axboe/liburing/issues/1509
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 net/unix/af_unix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 110d716087b5..72dc5d5bcac8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -3091,7 +3091,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 		scm_recv_unix(sock, msg, &scm, flags);
 
 		do_cmsg = READ_ONCE(u->recvmsg_inq);
-		if (do_cmsg || msg->msg_get_inq) {
+		if ((do_cmsg || msg->msg_get_inq) && (copied ?: err) >= 0) {
 			msg->msg_inq = READ_ONCE(u->inq_len);
 			if (do_cmsg)
 				put_cmsg(msg, SOL_SOCKET, SCM_INQ,
-- 
2.51.0

Re: [PATCH 1/2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for

[Willem de Bruijn]

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@vger.kernel.org
> Fixes: df30285b3670 ("af_unix: Introduce SO_INQ.")
> Reported-by: Julian Orth <ju.orth@gmail.com>
> Link: https://github.com/axboe/liburing/issues/1509
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  net/unix/af_unix.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> 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);

Is it intentional that msg_inq is set also if msg_get_inq is not set,
but do_cmsg is?

It just seems a bit surprising behavior.

That is an entangling of two separate things.
- msg_get_inq sets msg_inq, and
- cmsg_flags & TCP_CMSG_INQ inserts TCP_CM_INQ cmsg

The original TCP patch also entangles them, but in another way.
The cmsg is written only if msg_get_inq is requested.

	-       if (cmsg_flags && ret >= 0) {
	+       if ((cmsg_flags || msg->msg_get_inq) && ret >= 0) {
			if (cmsg_flags & TCP_CMSG_TS)
				tcp_recv_timestamp(msg, sk, &tss);
	-               if (cmsg_flags & TCP_CMSG_INQ) {
	-                       inq = tcp_inq_hint(sk);
	-                       put_cmsg(msg, SOL_TCP, TCP_CM_INQ, sizeof(inq), &inq);
	+               if (msg->msg_get_inq) {
	+                       msg->msg_inq = tcp_inq_hint(sk);
	+                       if (cmsg_flags & TCP_CMSG_INQ)
	+                               put_cmsg(msg, SOL_TCP, TCP_CM_INQ,
	+                                        sizeof(msg->msg_inq), &msg->msg_inq);

With this patch the two are still not entirely consistent.

>  		}
>  	} else {
>  		scm_destroy(&scm);
> -- 
> 2.51.0
> 

Re: [PATCH 1/2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for

[Jens Axboe]

On 12/18/25 1:35 PM, Willem de Bruijn wrote:
> 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@vger.kernel.org
>> Fixes: df30285b3670 ("af_unix: Introduce SO_INQ.")
>> Reported-by: Julian Orth <ju.orth@gmail.com>
>> Link: https://github.com/axboe/liburing/issues/1509
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  net/unix/af_unix.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> 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);
> 
> Is it intentional that msg_inq is set also if msg_get_inq is not set,
> but do_cmsg is?

It doesn't really matter, what matters is the actual cmsg posting be
guarded. The msg_inq should only be used for a successful return anyway,
I think we're better off reading it unconditionally than having multiple
branches.

Not really important, if you prefer to keep them consistent, that's fine
with me too.

> 
> It just seems a bit surprising behavior.
> 
> That is an entangling of two separate things.
> - msg_get_inq sets msg_inq, and
> - cmsg_flags & TCP_CMSG_INQ inserts TCP_CM_INQ cmsg
> 
> The original TCP patch also entangles them, but in another way.
> The cmsg is written only if msg_get_inq is requested.

The cmsg is written iff TCP_CMSG_INQ is set, not if ->msg_get_inq is the
only thing set. That part is important.

But yes, both need the data left.

-- 
Jens Axboe

Re: [PATCH 1/2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for

[Willem de Bruijn]

Jens Axboe wrote:
> On 12/18/25 1:35 PM, Willem de Bruijn wrote:
> > 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@vger.kernel.org
> >> Fixes: df30285b3670 ("af_unix: Introduce SO_INQ.")
> >> Reported-by: Julian Orth <ju.orth@gmail.com>
> >> Link: https://github.com/axboe/liburing/issues/1509
> >> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >> ---
> >>  net/unix/af_unix.c | 10 +++++++---
> >>  1 file changed, 7 insertions(+), 3 deletions(-)
> >>
> >> 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);
> > 
> > Is it intentional that msg_inq is set also if msg_get_inq is not set,
> > but do_cmsg is?
> 
> It doesn't really matter, what matters is the actual cmsg posting be
> guarded. The msg_inq should only be used for a successful return anyway,
> I think we're better off reading it unconditionally than having multiple
> branches.
> 
> Not really important, if you prefer to keep them consistent, that's fine
> with me too.
> 
> > 
> > It just seems a bit surprising behavior.
> > 
> > That is an entangling of two separate things.
> > - msg_get_inq sets msg_inq, and
> > - cmsg_flags & TCP_CMSG_INQ inserts TCP_CM_INQ cmsg
> > 
> > The original TCP patch also entangles them, but in another way.
> > The cmsg is written only if msg_get_inq is requested.
> 
> The cmsg is written iff TCP_CMSG_INQ is set, not if ->msg_get_inq is the
> only thing set. That part is important.
> 
> But yes, both need the data left.

I see, writing msg_inq if not requested is benign indeed. The inverse
is not true.

Ok. I do think it would be good to have the protocols consistent.
Simpler to reason about the behavior and intent long term.
 
> -- 
> Jens Axboe

Re: [PATCH 1/2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for

[Jens Axboe]

On 12/18/25 2:15 PM, Willem de Bruijn wrote:
> Jens Axboe wrote:
>> On 12/18/25 1:35 PM, Willem de Bruijn wrote:
>>> 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@vger.kernel.org
>>>> Fixes: df30285b3670 ("af_unix: Introduce SO_INQ.")
>>>> Reported-by: Julian Orth <ju.orth@gmail.com>
>>>> Link: https://github.com/axboe/liburing/issues/1509
>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>> ---
>>>>  net/unix/af_unix.c | 10 +++++++---
>>>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> 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);
>>>
>>> Is it intentional that msg_inq is set also if msg_get_inq is not set,
>>> but do_cmsg is?
>>
>> It doesn't really matter, what matters is the actual cmsg posting be
>> guarded. The msg_inq should only be used for a successful return anyway,
>> I think we're better off reading it unconditionally than having multiple
>> branches.
>>
>> Not really important, if you prefer to keep them consistent, that's fine
>> with me too.
>>
>>>
>>> It just seems a bit surprising behavior.
>>>
>>> That is an entangling of two separate things.
>>> - msg_get_inq sets msg_inq, and
>>> - cmsg_flags & TCP_CMSG_INQ inserts TCP_CM_INQ cmsg
>>>
>>> The original TCP patch also entangles them, but in another way.
>>> The cmsg is written only if msg_get_inq is requested.
>>
>> The cmsg is written iff TCP_CMSG_INQ is set, not if ->msg_get_inq is the
>> only thing set. That part is important.
>>
>> But yes, both need the data left.
> 
> I see, writing msg_inq if not requested is benign indeed. The inverse
> is not true.
> 
> Ok. I do think it would be good to have the protocols consistent.
> Simpler to reason about the behavior and intent long term.

Sure, I can do that. Would you prefer patch 1 and 2 folded as well, or
keep them separate? If we're mirroring the logic, seems like 1 patch
would be better.

-- 
Jens Axboe