* [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for
@ 2025-12-18 22:21 Jens Axboe
2025-12-19 19:02 ` Willem de Bruijn
0 siblings, 1 reply; 7+ messages in thread
From: Jens Axboe @ 2025-12-18 22:21 UTC (permalink / raw)
To: netdev
Cc: io-uring, Jakub Kicinski, Willem de Bruijn, Kuniyuki Iwashima,
Julian Orth
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;
redo:
/* Lock the socket to prevent queue disordering
* while sleeps in memcpy_tomsg
@@ -3088,10 +3092,11 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
if (msg) {
scm_recv_unix(sock, msg, &scm, flags);
- if (READ_ONCE(u->recvmsg_inq) || msg->msg_get_inq) {
+ if (msg->msg_get_inq && (copied ?: err) >= 0) {
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);
--
Jens Axboe
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for
2025-12-18 22:21 [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for Jens Axboe
@ 2025-12-19 19:02 ` Willem de Bruijn
2025-12-19 19:55 ` Jens Axboe
0 siblings, 1 reply; 7+ messages in thread
From: Willem de Bruijn @ 2025-12-19 19:02 UTC (permalink / raw)
To: Jens Axboe, netdev
Cc: io-uring, Jakub Kicinski, Willem de Bruijn, Kuniyuki Iwashima,
Julian Orth
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) {
> redo:
> /* Lock the socket to prevent queue disordering
> * while sleeps in memcpy_tomsg
> @@ -3088,10 +3092,11 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
> if (msg) {
> scm_recv_unix(sock, msg, &scm, flags);
>
> - if (READ_ONCE(u->recvmsg_inq) || msg->msg_get_inq) {
> + if (msg->msg_get_inq && (copied ?: err) >= 0) {
> 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);
> --
> Jens Axboe
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for
2025-12-19 19:02 ` Willem de Bruijn
@ 2025-12-19 19:55 ` Jens Axboe
2025-12-19 20:08 ` Willem de Bruijn
0 siblings, 1 reply; 7+ messages in thread
From: Jens Axboe @ 2025-12-19 19:55 UTC (permalink / raw)
To: Willem de Bruijn, netdev
Cc: io-uring, Jakub Kicinski, Willem de Bruijn, Kuniyuki Iwashima,
Julian Orth
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;
}
to avoid needing to check two things at the bottom. Which is actually
why I did this for streams too, as the whole point was to unify the two
and make them look the same.
Like I said, I'm happy to give you guys what you want, but you can't
both ask for consistency and then want it different too. I just want the
bug fixed and out of my hair and into a stable release, as it's causing
regressions.
Let me know, and I'll send out a v3 if needed. But then let's please
have that be it and move on.
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for
2025-12-19 19:55 ` Jens Axboe
@ 2025-12-19 20:08 ` Willem de Bruijn
2025-12-19 20:24 ` Jens Axboe
2025-12-23 17:27 ` Jens Axboe
0 siblings, 2 replies; 7+ messages in thread
From: Willem de Bruijn @ 2025-12-19 20:08 UTC (permalink / raw)
To: Jens Axboe, Willem de Bruijn, netdev
Cc: io-uring, Jakub Kicinski, Willem de Bruijn, Kuniyuki Iwashima,
Julian Orth
[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>
>
> to avoid needing to check two things at the bottom. Which is actually
> why I did this for streams too, as the whole point was to unify the two
> and make them look the same.
>
> Like I said, I'm happy to give you guys what you want, but you can't
> both ask for consistency and then want it different too. I just want the
> bug fixed and out of my hair and into a stable release, as it's causing
> regressions.
>
> Let me know, and I'll send out a v3 if needed. But then let's please
> have that be it and move on.
>
> --
> Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for
2025-12-19 20:08 ` Willem de Bruijn
@ 2025-12-19 20:24 ` Jens Axboe
2025-12-19 22:03 ` Willem de Bruijn
2025-12-23 17:27 ` Jens Axboe
1 sibling, 1 reply; 7+ messages in thread
From: Jens Axboe @ 2025-12-19 20:24 UTC (permalink / raw)
To: Willem de Bruijn, netdev
Cc: io-uring, Jakub Kicinski, Willem de Bruijn, Kuniyuki Iwashima,
Julian Orth
On 12/19/25 1:08 PM, Willem de Bruijn wrote:
> [PATCH net v2] assuming this is intended to go through the net tree.
Assuming this will hit -rc3 then, as netdev PRs usually go out on
thursdays?
> 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.
FWIW, I don't disagree with you, but sorting that out should then be a
followup patch that would then touch both tcp and streams.
> Reviewed-by: Willem de Bruijn <willemb@google.com>
Thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for
2025-12-19 20:24 ` Jens Axboe
@ 2025-12-19 22:03 ` Willem de Bruijn
0 siblings, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2025-12-19 22:03 UTC (permalink / raw)
To: Jens Axboe, Willem de Bruijn, netdev
Cc: io-uring, Jakub Kicinski, Willem de Bruijn, Kuniyuki Iwashima,
Julian Orth
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.
>
> Assuming this will hit -rc3 then, as netdev PRs usually go out on
> thursdays?
>
> > 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.
>
> FWIW, I don't disagree with you, but sorting that out should then be a
> followup patch that would then touch both tcp and streams.
Agreed. That's more for net-next. I'll take a look.
> > Reviewed-by: Willem de Bruijn <willemb@google.com>
>
> Thanks!
>
> --
> Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for
2025-12-19 20:08 ` Willem de Bruijn
2025-12-19 20:24 ` Jens Axboe
@ 2025-12-23 17:27 ` Jens Axboe
1 sibling, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2025-12-23 17:27 UTC (permalink / raw)
To: Willem de Bruijn, netdev
Cc: io-uring, Jakub Kicinski, Willem de Bruijn, Kuniyuki Iwashima,
Julian Orth, Paolo Abeni
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?
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-12-23 17:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-18 22:21 [PATCH v2] af_unix: don't post cmsg for SO_INQ unless explicitly asked for Jens Axboe
2025-12-19 19:02 ` Willem de Bruijn
2025-12-19 19:55 ` Jens Axboe
2025-12-19 20:08 ` Willem de Bruijn
2025-12-19 20:24 ` Jens Axboe
2025-12-19 22:03 ` Willem de Bruijn
2025-12-23 17:27 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox