[PATCH] io_uring/net: clear stale vec on buffer peek error after expansion

[PATCH] io_uring/net: clear stale vec on buffer peek error after expansion

[Feng Xue]

Subject: [PATCH] io_uring/net: clear stale vec on buffer peek error after expansion

When io_ring_buffers_peek() expands the iovec array during a bundle
recv retry, it frees the old array (A) and allocates a new one (B).
If access_ok() then fails, B is also freed and -EFAULT is returned.

The callers io_recv_buf_select() and io_send_select_buffer() only
update kmsg->vec.iovec on success, so on this error path vec.iovec
still points to freed A. The stale pointer survives into the netmsg
alloc cache via io_netmsg_recycle() (vec.nr < IO_VEC_CACHE_SOFT_CAP
so io_vec_free is not called). A subsequent bundle operation reuses
the cached hdr, sees vec.iovec non-NULL, sets REQ_F_NEED_CLEANUP,
and passes the dangling pointer back to io_ring_buffers_peek() ―
which writes iovec entries to freed memory (use-after-free).

If the alloc cache is full, the alternative cleanup path through
io_clean_op() → io_vec_free() kfree()s the already-freed A
(double-free).

Fix this by NULLing vec.iovec and zeroing vec.nr on the error path
when expansion occurred (detected by arg.iovs != kmsg->vec.iovec).
Do not call io_vec_free() here ― A is already freed by the expansion
block, so kfree()ing it again would itself be a double-free.

Apply the same fix to io_send_select_buffer() which has the identical
update-after-success pattern.

Signed-off-by: Feng Xue <feng.xue@outlook.com>
Assisted by: XGPT
---
 io_uring/net.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index XXXXXXX..XXXXXXX 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -631,8 +631,15 @@ static int io_send_select_buffer(struct io_kiocb *req, unsigned int issue_flags,
 
 	ret = io_buffers_select(req, &arg, sel, issue_flags);
-	if (unlikely(ret < 0))
+	if (unlikely(ret < 0)) {
+		/*
+		 * Buffer selection may have freed the old iovec during
+		 * expansion. Clear vec to prevent stale-pointer reuse.
+		 */
+		if (kmsg->vec.iovec && arg.iovs != kmsg->vec.iovec) {
+			kmsg->vec.iovec = NULL;
+			kmsg->vec.nr = 0;
+		}
 		return ret;
+	}
 
 	if (arg.iovs != &kmsg->fast_iov && arg.iovs != kmsg->vec.iovec) {
@@ -1174,8 +1181,15 @@ static int io_recv_buf_select(struct io_kiocb *req,
 
 		ret = io_buffers_peek(req, &arg, sel);
-		if (unlikely(ret < 0))
+		if (unlikely(ret < 0)) {
+			/*
+			 * Peek may have freed the old iovec during expansion.
+			 * Clear vec to prevent stale-pointer reuse or
+			 * double-free via io_vec_free on the cleanup path.
+			 */
+			if (kmsg->vec.iovec && arg.iovs != kmsg->vec.iovec) {
+				kmsg->vec.iovec = NULL;
+				kmsg->vec.nr = 0;
+			}
 			return ret;
+		}
 
 		if (arg.iovs != &kmsg->fast_iov && arg.iovs != kmsg->vec.iovec) {

Re: [PATCH] io_uring/net: clear stale vec on buffer peek error after expansion

[Gabriel Krisman Bertazi]

Feng Xue <feng.xue@outlook.com> writes:

> Subject: [PATCH] io_uring/net: clear stale vec on buffer peek error after expansion
>
> When io_ring_buffers_peek() expands the iovec array during a bundle
> recv retry, it frees the old array (A) and allocates a new one (B).
> If access_ok() then fails, B is also freed and -EFAULT is returned.
>
> The callers io_recv_buf_select() and io_send_select_buffer() only
> update kmsg->vec.iovec on success, so on this error path vec.iovec
> still points to freed A. The stale pointer survives into the netmsg
> alloc cache via io_netmsg_recycle() (vec.nr < IO_VEC_CACHE_SOFT_CAP
> so io_vec_free is not called). A subsequent bundle operation reuses
> the cached hdr, sees vec.iovec non-NULL, sets REQ_F_NEED_CLEANUP,
> and passes the dangling pointer back to io_ring_buffers_peek() —
> which writes iovec entries to freed memory (use-after-free).
>
> If the alloc cache is full, the alternative cleanup path through
> io_clean_op() → io_vec_free() kfree()s the already-freed A
> (double-free).
>
> Fix this by NULLing vec.iovec and zeroing vec.nr on the error path
> when expansion occurred (detected by arg.iovs != kmsg->vec.iovec).
> Do not call io_vec_free() here — A is already freed by the expansion
> block, so kfree()ing it again would itself be a double-free.
>
> Apply the same fix to io_send_select_buffer() which has the identical
> update-after-success pattern.

cleaning in the caller makes the issue much more likely to happen again
in a future use of this function.  It would be better to fix the bad
semantics of io_ring_buffers_peek instead.

In fact, this is exactly the point of this patch, which I believe
already fixed this issue:

https://lore.kernel.org/io-uring/178338543579.49877.9882374687710864124.b4-ty@b4/T/#t

>
> Signed-off-by: Feng Xue <feng.xue@outlook.com>
> Assisted by: XGPT
> ---
>  io_uring/net.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/io_uring/net.c b/io_uring/net.c
> index XXXXXXX..XXXXXXX 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -631,8 +631,15 @@ static int io_send_select_buffer(struct io_kiocb *req, unsigned int issue_flags,
>  
>  	ret = io_buffers_select(req, &arg, sel, issue_flags);
> -	if (unlikely(ret < 0))
> +	if (unlikely(ret < 0)) {
> +		/*
> +		 * Buffer selection may have freed the old iovec during
> +		 * expansion. Clear vec to prevent stale-pointer reuse.
> +		 */
> +		if (kmsg->vec.iovec && arg.iovs != kmsg->vec.iovec) {
> +			kmsg->vec.iovec = NULL;
> +			kmsg->vec.nr = 0;
> +		}
>  		return ret;
> +	}
>  
>  	if (arg.iovs != &kmsg->fast_iov && arg.iovs != kmsg->vec.iovec) {
> @@ -1174,8 +1181,15 @@ static int io_recv_buf_select(struct io_kiocb *req,
>  
>  		ret = io_buffers_peek(req, &arg, sel);
> -		if (unlikely(ret < 0))
> +		if (unlikely(ret < 0)) {
> +			/*
> +			 * Peek may have freed the old iovec during expansion.
> +			 * Clear vec to prevent stale-pointer reuse or
> +			 * double-free via io_vec_free on the cleanup path.
> +			 */
> +			if (kmsg->vec.iovec && arg.iovs != kmsg->vec.iovec) {
> +				kmsg->vec.iovec = NULL;
> +				kmsg->vec.nr = 0;
> +			}
>  			return ret;
> +		}
>  
>  		if (arg.iovs != &kmsg->fast_iov && arg.iovs != kmsg->vec.iovec) {

-- 
Gabriel Krisman Bertazi

Re: [PATCH] io_uring/net: clear stale vec on buffer peek error after expansion

[Jens Axboe]

On 7/8/26 9:45 AM, Gabriel Krisman Bertazi wrote:
> Feng Xue <feng.xue@outlook.com> writes:
> 
>> Subject: [PATCH] io_uring/net: clear stale vec on buffer peek error after expansion
>>
>> When io_ring_buffers_peek() expands the iovec array during a bundle
>> recv retry, it frees the old array (A) and allocates a new one (B).
>> If access_ok() then fails, B is also freed and -EFAULT is returned.
>>
>> The callers io_recv_buf_select() and io_send_select_buffer() only
>> update kmsg->vec.iovec on success, so on this error path vec.iovec
>> still points to freed A. The stale pointer survives into the netmsg
>> alloc cache via io_netmsg_recycle() (vec.nr < IO_VEC_CACHE_SOFT_CAP
>> so io_vec_free is not called). A subsequent bundle operation reuses
>> the cached hdr, sees vec.iovec non-NULL, sets REQ_F_NEED_CLEANUP,
>> and passes the dangling pointer back to io_ring_buffers_peek() ?
>> which writes iovec entries to freed memory (use-after-free).
>>
>> If the alloc cache is full, the alternative cleanup path through
>> io_clean_op() ? io_vec_free() kfree()s the already-freed A
>> (double-free).
>>
>> Fix this by NULLing vec.iovec and zeroing vec.nr on the error path
>> when expansion occurred (detected by arg.iovs != kmsg->vec.iovec).
>> Do not call io_vec_free() here ? A is already freed by the expansion
>> block, so kfree()ing it again would itself be a double-free.
>>
>> Apply the same fix to io_send_select_buffer() which has the identical
>> update-after-success pattern.
> 
> cleaning in the caller makes the issue much more likely to happen again
> in a future use of this function.  It would be better to fix the bad
> semantics of io_ring_buffers_peek instead.
> 
> In fact, this is exactly the point of this patch, which I believe
> already fixed this issue:
> 
> https://lore.kernel.org/io-uring/178338543579.49877.9882374687710864124.b4-ty@b4/T/#t

Indeed, this version is just a terrible LLM version of trying to fix the
same thing, but not understanding the issue.

-- 
Jens Axboe