[PATCH liburing 0/2] Fix op_recv stack corruption

[PATCH liburing 0/2] Fix op_recv stack corruption

[Gabriel Krisman Bertazi]

Jens,

These two tests were failing on my debug infrastructure. It seems gcc
hides the issue with a higher level of optimization, most likely by
smarting out the local variable allocation straight to registers, which
is why we don't observe it more often.  When building with '-O0 -g3', I
could trigger it 100% of time.  Tested with kernel 7.2-rc4.

Gabriel Krisman Bertazi (2):
  test/send_recvmsg: Preserve msghdr until op_recvmsg completes
  test/recv-msgall-stream: Preserve msghdr until op_recvmsg completes

 test/recv-msgall-stream.c | 14 +++++++-------
 test/send_recvmsg.c       | 16 ++++++++--------
 2 files changed, 15 insertions(+), 15 deletions(-)

-- 
2.54.0

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

[Gabriel Krisman Bertazi]

msghdr is allocated on the stack at recv_prep, which means it may go out
of scope before the kernel has a chance to complete the operation.  This
results in spurious test failures when we reach far enough into recv_fn
to reuse the stack space before op_recvmsg executes.  I found it easily
reproducible when compiling with '-O0 -g3' to avoid gcc from optimizing
further local variables out of the stack.

Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>
---
 test/send_recvmsg.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/test/send_recvmsg.c b/test/send_recvmsg.c
index f2e1efc0..9154b6c0 100644
--- a/test/send_recvmsg.c
+++ b/test/send_recvmsg.c
@@ -32,10 +32,9 @@ static int ud;
 static int no_pbuf_ring;
 
 static int recv_prep(struct io_uring *ring, int *sockfd, struct iovec iov[],
-		     int iov_count, int bgid, int async)
+		     int iov_count, int bgid, int async, struct msghdr *msg)
 {
 	struct sockaddr_in saddr;
-	struct msghdr msg;
 	struct io_uring_sqe *sqe;
 	int ret, val = 1;
 
@@ -66,7 +65,7 @@ static int recv_prep(struct io_uring *ring, int *sockfd, struct iovec iov[],
 		return 1;
 	}
 
-	io_uring_prep_recvmsg(sqe, *sockfd, &msg, 0);
+	io_uring_prep_recvmsg(sqe, *sockfd, msg, 0);
 	if (bgid) {
 		iov->iov_base = NULL;
 		sqe->flags |= IOSQE_BUFFER_SELECT;
@@ -76,10 +75,10 @@ static int recv_prep(struct io_uring *ring, int *sockfd, struct iovec iov[],
 	sqe->user_data = ++ud;
 	if (async)
 		sqe->flags |= IOSQE_ASYNC;
-	memset(&msg, 0, sizeof(msg));
-	msg.msg_namelen = sizeof(struct sockaddr_in);
-	msg.msg_iov = iov;
-	msg.msg_iovlen = iov_count;
+	memset(msg, 0, sizeof(*msg));
+	msg->msg_namelen = sizeof(struct sockaddr_in);
+	msg->msg_iov = iov;
+	msg->msg_iovlen = iov_count;
 
 	ret = io_uring_submit(ring);
 	if (ret <= 0) {
@@ -168,6 +167,7 @@ static void *recv_fn(void *data)
 	struct io_uring_buf_ring *br = NULL;
 	char buf[MAX_MSG + 1];
 	struct iovec iov[MAX_IOV_COUNT];
+	struct msghdr msg;
 	struct io_uring ring;
 	int ret, sockfd;
 
@@ -225,7 +225,7 @@ static void *recv_fn(void *data)
 
 	ret = recv_prep(&ring, &sockfd, iov, rd->iov_count,
 			(rd->buf_ring || rd->buf_select) ? BUF_BGID : 0,
-			rd->async);
+			rd->async, &msg);
 	if (ret) {
 		fprintf(stderr, "recv_prep failed: %d\n", ret);
 		goto err;
-- 
2.54.0

[PATCH liburing 2/2] test/recv-msgall-stream: Preserve msghdr until op_recvmsg completes

[Gabriel Krisman Bertazi]

msghdr is allocated on the stack at recv_prep, which means it may go out
of scope before the kernel has a chance to complete the operation.  This
results in spurious test failures when we reach far enough into recv_fn
to reuse the stack space before op_recvmsg executes.  I found it easily
reproducible when compiling with '-O0 -g3' to avoid gcc from optimizing
further local variables out of the stack.

Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>
---
 test/recv-msgall-stream.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/recv-msgall-stream.c b/test/recv-msgall-stream.c
index ff9fd2a2..a056a6e8 100644
--- a/test/recv-msgall-stream.c
+++ b/test/recv-msgall-stream.c
@@ -74,10 +74,9 @@ err:
 }
 
 static int recv_prep(struct io_uring *ring, struct iovec *iov, int *sock,
-		     struct recv_data *rd)
+		     struct recv_data *rd, struct msghdr *msg)
 {
 	struct io_uring_sqe *sqe;
-	struct msghdr msg = { };
 	int sockfd, sockout = -1, ret;
 
 	sockfd = get_conn_sock(rd, &sockout);
@@ -89,10 +88,10 @@ static int recv_prep(struct io_uring *ring, struct iovec *iov, int *sock,
 		io_uring_prep_recv(sqe, sockfd, iov->iov_base, iov->iov_len,
 					MSG_WAITALL);
 	} else {
-		msg.msg_namelen = sizeof(struct sockaddr_in);
-		msg.msg_iov = iov;
-		msg.msg_iovlen = 1;
-		io_uring_prep_recvmsg(sqe, sockfd, &msg, MSG_WAITALL);
+		msg->msg_namelen = sizeof(struct sockaddr_in);
+		msg->msg_iov = iov;
+		msg->msg_iovlen = 1;
+		io_uring_prep_recvmsg(sqe, sockfd, msg, MSG_WAITALL);
 	}
 
 	sqe->user_data = 2;
@@ -198,6 +197,7 @@ static int recv_uring(struct recv_data *rd)
 		.iov_base = buf,
 		.iov_len = sizeof(buf),
 	};
+	struct msghdr msg;
 	struct io_uring_params p = { };
 	struct io_uring ring;
 	int ret, sock = -1, sockout = -1;
@@ -212,7 +212,7 @@ static int recv_uring(struct recv_data *rd)
 		goto err;
 	}
 
-	sock = recv_prep(&ring, &iov, &sockout, rd);
+	sock = recv_prep(&ring, &iov, &sockout, rd, &msg);
 	if (ret) {
 		fprintf(stderr, "recv_prep failed: %d\n", ret);
 		goto err;
-- 
2.54.0

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

[Gabriel Krisman Bertazi]

Gabriel Krisman Bertazi <krisman@suse.de> writes:

> msghdr is allocated on the stack at recv_prep, which means it may go out
> of scope before the kernel has a chance to complete the operation.  This
> results in spurious test failures when we reach far enough into recv_fn
> to reuse the stack space before op_recvmsg executes.  I found it easily
> reproducible when compiling with '-O0 -g3' to avoid gcc from optimizing
> further local variables out of the stack.

In hindsight, I should clarify that by 'test failure' I mean a
segmentation fault in the testcase, not a T_EXIT_FAIL. Should have put
that in the commit message.

-- 
Gabriel Krisman Bertazi

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

[Jens Axboe]

On 7/22/26 12:17 PM, Gabriel Krisman Bertazi wrote:
> msghdr is allocated on the stack at recv_prep, which means it may go out
> of scope before the kernel has a chance to complete the operation.  This
> results in spurious test failures when we reach far enough into recv_fn
> to reuse the stack space before op_recvmsg executes.  I found it easily
> reproducible when compiling with '-O0 -g3' to avoid gcc from optimizing
> further local variables out of the stack.

Hmm, but that should be fine as long as a) we submit in scope, and b)
we're not using SQPOLL, where it does need to remain consistent until
completion.

And recv_prep() certainly submits before it returns, and we're not using
SQPOLL. So I'm curious what issue this is?? Same questions on patch 2.

-- 
Jens Axboe

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

[Gabriel Krisman Bertazi]

Jens Axboe <axboe@kernel.dk> writes:

> On 7/22/26 12:17 PM, Gabriel Krisman Bertazi wrote:
>> msghdr is allocated on the stack at recv_prep, which means it may go out
>> of scope before the kernel has a chance to complete the operation.  This
>> results in spurious test failures when we reach far enough into recv_fn
>> to reuse the stack space before op_recvmsg executes.  I found it easily
>> reproducible when compiling with '-O0 -g3' to avoid gcc from optimizing
>> further local variables out of the stack.
>
> Hmm, but that should be fine as long as a) we submit in scope, and b)
> we're not using SQPOLL, where it does need to remain consistent until
> completion.
>
> And recv_prep() certainly submits before it returns, and we're not using
> SQPOLL. So I'm curious what issue this is?? Same questions on patch 2.

Hm, I assumed it was submitted via iowq, which would explain this,
because the execution in io_recvmsg() passes a pointer to the original
memory:

        ret = __sys_recvmsg_sock(sock, &kmsg->msg, sr->umsg,
	    			 kmsg->uaddr, flags);

and __sys_recvmsg_sock does write to it. which makes it clear the msghdr
needs to live until completion.

But honestly, whenever I try to probe to confirm the execution went
through iowq, the timing gets off and I can't reproduce the corruption.

I will take another look and see if I can explain better.

> -- 
> Jens Axboe

-- 
Gabriel Krisman Bertazi

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

[Gabriel Krisman Bertazi]

Gabriel Krisman Bertazi <krisman@suse.de> writes:

> Jens Axboe <axboe@kernel.dk> writes:
>
>> On 7/22/26 12:17 PM, Gabriel Krisman Bertazi wrote:
>>> msghdr is allocated on the stack at recv_prep, which means it may go out
>>> of scope before the kernel has a chance to complete the operation.  This
>>> results in spurious test failures when we reach far enough into recv_fn
>>> to reuse the stack space before op_recvmsg executes.  I found it easily
>>> reproducible when compiling with '-O0 -g3' to avoid gcc from optimizing
>>> further local variables out of the stack.
>>
>> Hmm, but that should be fine as long as a) we submit in scope, and b)
>> we're not using SQPOLL, where it does need to remain consistent until
>> completion.
>>
>> And recv_prep() certainly submits before it returns, and we're not using
>> SQPOLL. So I'm curious what issue this is?? Same questions on patch 2.
>
> Hm, I assumed it was submitted via iowq, which would explain this,
> because the execution in io_recvmsg() passes a pointer to the original
> memory:

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.

FWIW, I could only trace this with good old printk.  Any
tracepoint/bpftrace made the issue disappear. I think that is because
with these tracing tools, we end up executing the tw inside the previous
io_uring_enter, since it took time to complete and the socket got ready in
the meantime.

>
>> -- 
>> Jens Axboe
>
> -- 
> Gabriel Krisman Bertazi

-- 
Gabriel Krisman Bertazi

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

[Gabriel Krisman Bertazi]

Gabriel Krisman Bertazi <krisman@suse.de> writes:

> Gabriel Krisman Bertazi <krisman@suse.de> writes:
>
>> Jens Axboe <axboe@kernel.dk> writes:
>>
>>> On 7/22/26 12:17 PM, Gabriel Krisman Bertazi wrote:
>>>> msghdr is allocated on the stack at recv_prep, which means it may go out
>>>> of scope before the kernel has a chance to complete the operation.  This
>>>> results in spurious test failures when we reach far enough into recv_fn
>>>> to reuse the stack space before op_recvmsg executes.  I found it easily
>>>> reproducible when compiling with '-O0 -g3' to avoid gcc from optimizing
>>>> further local variables out of the stack.
>>>
>>> Hmm, but that should be fine as long as a) we submit in scope, and b)
>>> we're not using SQPOLL, where it does need to remain consistent until
>>> completion.
>>>
>>> And recv_prep() certainly submits before it returns, and we're not using
>>> SQPOLL. So I'm curious what issue this is?? Same questions on patch 2.
>>
>> Hm, I assumed it was submitted via iowq, which would explain this,
>> because the execution in io_recvmsg() passes a pointer to the original
>> memory:
>
> 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?

-- 
Gabriel Krisman Bertazi

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

[Jens Axboe]

On 7/26/26 6:13 PM, Gabriel Krisman Bertazi wrote:
> Gabriel Krisman Bertazi <krisman@suse.de> writes:
> 
>> Gabriel Krisman Bertazi <krisman@suse.de> writes:
>>
>>> Jens Axboe <axboe@kernel.dk> writes:
>>>
>>>> On 7/22/26 12:17 PM, Gabriel Krisman Bertazi wrote:
>>>>> msghdr is allocated on the stack at recv_prep, which means it may go out
>>>>> of scope before the kernel has a chance to complete the operation.  This
>>>>> results in spurious test failures when we reach far enough into recv_fn
>>>>> to reuse the stack space before op_recvmsg executes.  I found it easily
>>>>> reproducible when compiling with '-O0 -g3' to avoid gcc from optimizing
>>>>> further local variables out of the stack.
>>>>
>>>> Hmm, but that should be fine as long as a) we submit in scope, and b)
>>>> we're not using SQPOLL, where it does need to remain consistent until
>>>> completion.
>>>>
>>>> And recv_prep() certainly submits before it returns, and we're not using
>>>> SQPOLL. So I'm curious what issue this is?? Same questions on patch 2.
>>>
>>> Hm, I assumed it was submitted via iowq, which would explain this,
>>> because the execution in io_recvmsg() passes a pointer to the original
>>> memory:
>>
>> 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.

-- 
Jens Axboe

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

[Gabriel Krisman Bertazi]

Jens Axboe <axboe@kernel.dk> writes:

> On 7/26/26 6:13 PM, Gabriel Krisman Bertazi wrote:
>> Gabriel Krisman Bertazi <krisman@suse.de> 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
87cxweepdj.fsf@mailhost.krisman.be 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