Re: [PATCH v1] io_uring/kbuf: use WRITE_ONCE() for userspace-shared buffer ring fields

[Caleb Sander Mateos <csander@purestorage.com>]

On Thu, Dec 4, 2025 at 3:55 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> buf->addr and buf->len reside in memory shared with userspace. They
> should be written with WRITE_ONCE() to guarantee atomic stores and
> prevent tearing or other unsafe compiler optimizations.

I considered this too, but I'm not sure it's necessary. A correctly
written userspace program won't access these fields concurrently with
the kernel, right? I'm not too familiar with the buffer ring UAPI, but
I would assume userspace is notified somehow (via a posted CQE?) that
the io_uring_buf slots have been consumed and are available to reuse.
In that case, a torn store here would only be observable to a buggy
userspace program, so I don't think we need to add WRITE_ONCE().

Best,
Caleb

>
> Fixes: ae98dbf43d75 ("io_uring/kbuf: add support for incremental buffer consumption")
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> Cc: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  io_uring/kbuf.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
> index 52b636d00a6b..796d131107dd 100644
> --- a/io_uring/kbuf.c
> +++ b/io_uring/kbuf.c
> @@ -44,11 +44,11 @@ static bool io_kbuf_inc_commit(struct io_buffer_list *bl, int len)
>                 buf_len -= this_len;
>                 /* Stop looping for invalid buffer length of 0 */
>                 if (buf_len || !this_len) {
> -                       buf->addr = READ_ONCE(buf->addr) + this_len;
> -                       buf->len = buf_len;
> +                       WRITE_ONCE(buf->addr, READ_ONCE(buf->addr) + this_len);
> +                       WRITE_ONCE(buf->len, buf_len);
>                         return false;
>                 }
> -               buf->len = 0;
> +               WRITE_ONCE(buf->len, 0);
>                 bl->head++;
>                 len -= this_len;
>         }
> @@ -291,7 +291,7 @@ static int io_ring_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
>                                 arg->partial_map = 1;
>                                 if (iov != arg->iovs)
>                                         break;
> -                               buf->len = len;
> +                               WRITE_ONCE(buf->len, len);
>                         }
>                 }
>
> --
> 2.47.3
>