Re: [PATCH liburing 1/2] test/send_recvmsg: Preserve msghdr until op_recvmsg completes

Jens Axboe <[email protected]> writes:

> On 7/26/26 6:13 PM, Gabriel Krisman Bertazi wrote:
>> Gabriel Krisman Bertazi <[email protected]> writes:

>>>
>>> So, __sys_recvmsg_sock during the inline attempt throws -EAGAIN at
>>> first, which makes io_recv return IOU_RETRY, which punts to tw after
>>> the socket is ready.  By tracing, I can see the tw is executed only
>>> during the io_uring_enter from io_uring_wait_cqe, which is when
>>> __sys_recvmsg_sock touches sr->umsg pointing to an already out-of-scope
>>> stack variable.
>> 
>> Hello, do you disagree or did I miss anything?
>
> Still on vacation and sporadically available, just haven't had time to
> look at it yet. I agree it looks fishy! I would encourage you to dig into
> the kernel side and get to the bottom of it.

AH, I thought you were back already. Sorry, please enjoy your time off,
this is absolutely not time-sensitive.

But there is no mystery here. the explanation I shared in
[email protected] is correct. Here:

>>> So, __sys_recvmsg_sock during the inline attempt throws -EAGAIN at
>>> first, which makes io_recv return IOU_RETRY, which punts to tw after
>>> the socket is ready.  By tracing, I can see the tw is executed only
>>> during the io_uring_enter from io_uring_wait_cqe, which is when
>>> __sys_recvmsg_sock touches sr->umsg pointing to an already out-of-scope
>>> stack variable.

IOW, the problem boils down to the fact recvmsg touches umsg deep inside
the net/ code and io_uring does not submit in scope if the data is not
ready.

In more detail, the initial -EAGAIN happens because the recv_fn thread
raced with the send, there is no data in the socket, and the inline
obviously cannot block in __skb_recv_udp. So we fall back from inline,
arm the poll and sit on our hands waiting.  By the time the data arrives
and the kernel retries, msg is out of scope.

The exact corruption that *I* am observing in this test is that
____sys_recvmsg writes 0 to the userspace-mapped msg->msg_controllen
during ->issue() because, well, this is UDP and doesn't need ancillary
msg_control. This behavior is documented in recvmsg(2).

In my failing builds, for instance, in the send_recvmsg.c test, once msg
goes out of scope, the stack space collides with the pointer of (struct
recv_data *rd) in the call do_recvmsg(). The difference is that with
-O0, gcc sticks it in the stack during the prologue of do_recvmsg(),
instead of preserving the pointer in a register.  Then we have the first
opportunity for the TW to run during the io_uring_wait_cqe. When it
comes back, the stack was touched by the kernel, thinking it is touching
&msg, and boom. With -O3, the stack layout changes and the kernel writes
over other stack data that is no longer touched by the testcase.

msg_p below is a pointer to the  original stack variable during
recv_prep. The type is struct msghdr*):

(gdb) p &(msg_p->msg_controllen)
$13 = (size_t *) 0x7ffff7d90c08

Which collides with

(gdb) p &rd
$14 = (struct recv_data **) 0x7ffff7d90c08

Since msg_controllen is zeroed by the kernel, rd=0x0 and this causes a
bad deref:

if (rd->no_buf_add && (rd->buf_select || rd->buf_ring))

In summary, the issue is that msg must live until completion which
is what these patches fix.

ButI see this contradicts the NOTES section of
io_uring_prep_recvmsg():

> As with any request that passes in data in a struct, that data must
> remain valid until the request has been successfully submitted. It need
> not remain valid until completion. Once a request has been submitted,
> the in- kernel state is stable. Very early kernels (5.4 and earlier)
> required state to be stable until the completion occurred. Applications
> can test for this behavior by inspecting the IORING_FEAT_SUBMIT_STABLE
> flag passed back

Should I change that section of the manpage or do we want to ensure the
stability of msg here (i.e. don't allow recvmsg to touch the msghdr
coming from io_uring?)

Either way, vacation takes priority :) see you soon.

-- 
Gabriel Krisman Bertazi