Re: [RFC PATCH v3 2/5] io_uring/cmd: zero-init pdu in io_uring_cmd_prep() to avoid UB

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

On Tue, Sep 2, 2025 at 3:23 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>
> On Mon, Sep 01, 2025 at 05:34:28PM -0700, Caleb Sander Mateos wrote:
> > On Fri, Aug 22, 2025 at 5:56 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> > >
> > > The pdu field in io_uring_cmd may contain stale data when a request
> > > object is recycled from the slab cache. Accessing uninitialized or
> > > garbage memory can lead to undefined behavior in users of the pdu.
> > >
> > > Ensure the pdu buffer is cleared during io_uring_cmd_prep() so that
> > > each command starts from a well-defined state. This avoids exposing
> > > uninitialized memory and prevents potential misinterpretation of data
> > > from previous requests.
> > >
> > > No functional change is intended other than guaranteeing that pdu is
> > > always zero-initialized before use.
> > >
> > > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > > ---
> > >  io_uring/uring_cmd.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> > > index 053bac89b6c0..2492525d4e43 100644
> > > --- a/io_uring/uring_cmd.c
> > > +++ b/io_uring/uring_cmd.c
> > > @@ -203,6 +203,7 @@ int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> > >         if (!ac)
> > >                 return -ENOMEM;
> > >         ioucmd->sqe = sqe;
> > > +       memset(&ioucmd->pdu, 0, sizeof(ioucmd->pdu));
> >
> > Adding this overhead to every existing uring_cmd() implementation is
> > unfortunate. Could we instead track the initialized/uninitialized
> > state by using different types on the Rust side? The io_uring_cmd
> > could start as an IoUringCmd, where the PDU field is MaybeUninit,
> > write_pdu<T>() could return a new IoUringCmdPdu<T> that guarantees the
> > PDU has been initialized.
>
> I've found a flag IORING_URING_CMD_REISSUE that we could initialize
> the pdu. In uring_cmd callback, we can fill zero when it's not reissued.
> But I don't know that we could call T::default() in miscdevice. If we
> make IoUringCmdPdu<T>, MiscDevice also should be MiscDevice<T>.
>
> How about assign a byte in pdu for checking initialized? In uring_cmd(),
> We could set a byte flag that it's not initialized. And we could return
> error that it's not initialized in read_pdu().

Could we do the zero-initialization (or T::default()) in
MiscdeviceVTable::uring_cmd() if the IORING_URING_CMD_REISSUE flag
isn't set (i.e. on the initial issue)? That way, we avoid any
performance penalty for the existing C uring_cmd() implementations.
I'm not quite sure what you mean by "assign a byte in pdu for checking
initialized".

Best,
Caleb