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

[Jens Axboe <axboe@kernel.dk>]

On 12/28/25 8:02 AM, Paolo Abeni wrote:
> On 12/23/25 6:27 PM, Jens Axboe wrote:
>> On 12/19/25 1:08 PM, Willem de Bruijn wrote:
>>> [PATCH net v2] assuming this is intended to go through the net tree.
>>>
>>> Jens Axboe wrote:
>>>> On 12/19/25 12:02 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 a cmsg if u->recvmsg_inq is set.
>>>>>>
>>>>>> Additionally, mirror how TCP handles inquiry handling in that it should
>>>>>> only be done for a successful return. This makes the logic for the two
>>>>>> identical.
>>>>>>
>>>>>> 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>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> V2:
>>>>>> - Unify logic with tcp
>>>>>> - Squash the two patches into one
>>>>>>
>>>>>> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
>>>>>> index 55cdebfa0da0..a7ca74653d94 100644
>>>>>> --- a/net/unix/af_unix.c
>>>>>> +++ b/net/unix/af_unix.c
>>>>>> @@ -2904,6 +2904,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
>>>>>>  	unsigned int last_len;
>>>>>>  	struct unix_sock *u;
>>>>>>  	int copied = 0;
>>>>>> +	bool do_cmsg;
>>>>>>  	int err = 0;
>>>>>>  	long timeo;
>>>>>>  	int target;
>>>>>> @@ -2929,6 +2930,9 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
>>>>>>  
>>>>>>  	u = unix_sk(sk);
>>>>>>  
>>>>>> +	do_cmsg = READ_ONCE(u->recvmsg_inq);
>>>>>> +	if (do_cmsg)
>>>>>> +		msg->msg_get_inq = 1;
>>>>>
>>>>> I would avoid overwriting user written fields if it's easy to do so.
>>>>>
>>>>> In this case it probably is harmless. But we've learned the hard way
>>>>> that applications can even get confused by recvmsg setting msg_flags.
>>>>> I've seen multiple reports of applications failing to scrub that field
>>>>> inbetween calls.
>>>>>
>>>>> Also just more similar to tcp:
>>>>>
>>>>>        do_cmsg = READ_ONCE(u->recvmsg_inq);
>>>>>        if ((do_cmsg || msg->msg_get_inq) && (copied ?: err) >= 0) {
>>>>
>>>> I think you need to look closer, because this is actually what the tcp
>>>> path does:
>>>>
>>>> if (tp->recvmsg_inq) {
>>>> 	[...]
>>>> 	msg->msg_get_inq = 1;
>>>> }
>>>
>>> I indeed missed that TCP does the same. Ack. Indeed consistency was what I asked for.
>>>
>>> Reviewed-by: Willem de Bruijn <willemb@google.com>
>>
>> Can someone get this applied, please?
> 
> For a few more days it's just me. That means a significantly longer than
> usual latency, but I'm almost there.

Thanks Paolo!

-- 
Jens Axboe