Re: [PATCH v6] io_uring/register: add IORING_REGISTER_CLONE_FILES opcode

Harshal Chavan <[email protected]>
Newsgroups org.kernel.vger.io-uring,org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel
Message-ID <CADCAkb6wuR7vT3chfFtBL0qma+gx-2mJM7v+JJLS00W3oyb5dg@mail.gmail.com>
>> +     file = io_slot_file(src_node);
>> +     get_file(file);
>> +     io_fixed_file_set(dst_node, file);

> don't you need to copy the src_node->tag here as well?

Yes, sorry for the silly mistakes, thanks for catching this!

> I didn't get a chance to run it yet, sorry.  I'd suggest you wait for
> Jens feedback before pushing the v7 too, so you don't need to keep
> iterating drop by drop :)

Okay, Yes I will wait for feedback from Jens and send the changes together
Thanks for the review.

Regards,
Harshal Chavan

On Fri, Jun 26, 2026 at 2:03 AM Gabriel Krisman Bertazi <[email protected]> wrote:
>
> Harshal Chavan <[email protected]> writes:
>
> > Currently, if an application wants to duplicate registered file
> > descriptors from one io_uring instance to another, it must manually
> > unregister and re-register them, incurring unnecessary overhead.
> >
> > Add IORING_REGISTER_CLONE_FILES to allow direct cloning of the file
> > table from a source ring to a destination ring. This implementation
> > strictly mirrors the io_clone_buffers UAPI, supporting partial offsets
> > and the IORING_REGISTER_DST_REPLACE flag.
> >
> > To ensure lock synchronization safety, destination nodes are strictly
> > allocated as new, private io_rsrc_nodes rather than sharing references
> > across rings.
> >
> > Signed-off-by: Harshal Chavan <[email protected]>
> >
> > ---
> > Sorry for the noise on the previous email! I accidentally sent the patch
> > before running checkpatch and missed a whitespace error. This v6 corrects it.
> >
> > v6:
> >   - Fixed trailing whitespace checkpatch error.
> > v5:
> >   - Added missing spacing in comment (Gabriel).
> >   - Removed ctx->user and mm_account checks (Gabriel).
> >   - Used !! for boolean conversion (Gabriel).
> >   - Moved mutex_unlock unconditionally above the out label (Gabriel).
> >   - liburing implementation and tests: https://github.com/axboe/liburing/pull/1606
> > v4:
> >   - Updated Signed-off-by to use real name and moved above the scissors line (Greg KH).
> > v3:
> >   - Rewrote the cloning loop to allocate private destination nodes via io_rsrc_node_alloc to fix non-atomic ref lock synchronization (Jens).
> >   - Maintained partial offset/copy support to mirror io_clone_buffers UAPI (Jens).
> >   - Gated the replacement free check on ctx->file_table.data.nr (Gabriel).
> >   - Prevented self-cloning by checking ctx == src_ctx (Gabriel).
> >   - Removed submitter_task check to allow cross-thread pooling setups (Gabriel).
> > v2:
> >   - Dropped unrelated whitespace formatting changes from v1
>
> > +static int io_clone_file_node(struct io_ring_ctx *ctx,
> > +                           struct io_rsrc_node *src_node,
> > +                           int dst_index,
> > +                           struct io_file_table *new_table)
> > +{
> > +     struct io_rsrc_node *dst_node;
> > +     struct file *file;
> > +
> > +     dst_node = io_rsrc_node_alloc(ctx, IORING_RSRC_FILE);
> > +     if (!dst_node)
> > +             return -ENOMEM;
> > +
> > +     file = io_slot_file(src_node);
> > +     get_file(file);
> > +     io_fixed_file_set(dst_node, file);
>
> don't you need to copy the src_node->tag here as well?
>
> I didn't get a chance to run it yet, sorry.  I'd suggest you wait for
> Jens feedback before pushing the v7 too, so you don't need to keep
> iterating drop by drop :)
>
> > +
> > +     new_table->data.nodes[dst_index] = dst_node;
> > +     io_file_bitmap_set(new_table, dst_index);
> > +
> > +     return 0;
> > +}
> > +
> > +static int io_clone_files(struct io_ring_ctx *ctx, struct io_ring_ctx *src_ctx,
> > +                       struct io_uring_clone_files *arg)
> > +{
> > +     struct io_file_table new_file_table;
> > +     unsigned int dst_nr = ctx->file_table.data.nr;
> > +     unsigned int src_nr = src_ctx->file_table.data.nr;
> > +     unsigned int new_nr, i;
> > +
> > +     lockdep_assert_held(&ctx->uring_lock);
> > +     lockdep_assert_held(&src_ctx->uring_lock);
> > +
> > +     if (dst_nr && !(arg->flags & IORING_REGISTER_DST_REPLACE))
> > +             return -EBUSY;
> > +
> > +     if (!src_nr)
> > +             return -ENXIO;
> > +
> > +     if (!arg->nr)
> > +             arg->nr = src_nr;
> > +     else if (arg->nr > src_nr)
> > +             return -EINVAL;
> > +
> > +     if (check_add_overflow(arg->src_off, arg->nr, &i) || i > src_nr)
> > +             return -EINVAL;
> > +     if (check_add_overflow(arg->dst_off, arg->nr, &i))
> > +             return -EINVAL;
> > +
> > +     new_nr = max(dst_nr, arg->dst_off + arg->nr);
> > +     if (new_nr > IORING_MAX_FIXED_FILES)
> > +             return -EINVAL;
> > +
> > +     memset(&new_file_table, 0, sizeof(new_file_table));
> > +     if (!io_alloc_file_tables(ctx, &new_file_table, new_nr))
> > +             return -ENOMEM;
> > +
> > +     /* Copy original nodes from before the cloned range */
> > +     for (i = 0; i < min(arg->dst_off, dst_nr); i++) {
> > +             struct io_rsrc_node *src_node = io_rsrc_node_lookup(&ctx->file_table.data, i);
> > +
> > +             if (!src_node)
> > +                     continue;
> > +             if (io_clone_file_node(ctx, src_node, i, &new_file_table))
> > +                     goto out;
> > +     }
> > +
> > +     /* Copy the actual cloned range from the source ring */
> > +     for (i = 0; i < arg->nr; i++) {
> > +             struct io_rsrc_node *src_node = io_rsrc_node_lookup(&src_ctx->file_table.data,
> > +                             arg->src_off + i);
> > +
> > +             if (!src_node)
> > +                     continue;
> > +             if (io_clone_file_node(ctx, src_node, arg->dst_off + i, &new_file_table))
> > +                     goto out;
> > +     }
> > +
> > +     /* Copy original nodes from after the cloned range */
> > +     for (i = arg->dst_off + arg->nr; i < dst_nr; i++) {
> > +             struct io_rsrc_node *src_node = io_rsrc_node_lookup(&ctx->file_table.data, i);
> > +
> > +             if (!src_node)
> > +                     continue;
> > +             if (io_clone_file_node(ctx, src_node, i, &new_file_table))
> > +                     goto out;
> > +     }
> > +
> > +     /* free the old file table if there is any data present */
> > +     if (dst_nr)
> > +             io_free_file_tables(ctx, &ctx->file_table);
> > +
> > +     WARN_ON_ONCE(ctx->file_table.data.nr);
> > +     ctx->file_table = new_file_table;
> > +     io_file_table_set_alloc_range(ctx, 0, ctx->file_table.data.nr);
> > +     return 0;
> > +
> > +out:
> > +     /* Error Path: Safely destroy whatever we partially built */
> > +     io_free_file_tables(ctx, &new_file_table);
> > +     return -ENOMEM;
> > +}
> > +
> > +int io_register_clone_files(struct io_ring_ctx *ctx, void __user *arg)
> > +{
> > +     struct io_uring_clone_files clone_arg;
> > +     struct io_ring_ctx *src_ctx;
> > +     bool registered_src;
> > +     struct file *file;
> > +     int ret;
> > +
> > +     if (copy_from_user(&clone_arg, arg, sizeof(clone_arg)))
> > +             return -EFAULT;
> > +     if (clone_arg.flags &
> > +         ~(IORING_REGISTER_SRC_REGISTERED | IORING_REGISTER_DST_REPLACE))
> > +             return -EINVAL;
> > +
> > +     if (memchr_inv(clone_arg.pad, 0, sizeof(clone_arg.pad)))
> > +             return -EINVAL;
> > +
> > +     registered_src = !!(clone_arg.flags & IORING_REGISTER_SRC_REGISTERED);
> > +     file = io_uring_ctx_get_file(clone_arg.src_fd, registered_src);
> > +     if (IS_ERR(file))
> > +             return PTR_ERR(file);
> > +
> > +     src_ctx = file->private_data;
> > +     /* Same ring clone is not allowed */
> > +     if (src_ctx == ctx) {
> > +             ret = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     mutex_unlock(&ctx->uring_lock);
> > +     lock_two_rings(ctx, src_ctx);
> > +
> > +     ret = io_clone_files(ctx, src_ctx, &clone_arg);
> > +
> > +     mutex_unlock(&src_ctx->uring_lock);
> > +
> > +out:
> > +     if (!registered_src)
> > +             fput(file);
> > +     return ret;
> > +}
> > +
> >  void io_vec_free(struct iou_vec *iv)
> >  {
> >       if (!iv->iovec)
> > diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
> > index 44e3386f7c1c..32f5c47c46af 100644
> > --- a/io_uring/rsrc.h
> > +++ b/io_uring/rsrc.h
> > @@ -75,6 +75,7 @@ int io_prep_reg_iovec(struct io_kiocb *req, struct iou_vec *iv,
> >                       const struct iovec __user *uvec, size_t uvec_segs);
> >
> >  int io_register_clone_buffers(struct io_ring_ctx *ctx, void __user *arg);
> > +int io_register_clone_files(struct io_ring_ctx *ctx, void __user *arg);
> >  int io_sqe_buffers_unregister(struct io_ring_ctx *ctx);
> >  int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,
> >                           unsigned int nr_args, u64 __user *tags);
> > --
> > 2.54.0
> >
>
> --
> Gabriel Krisman Bertazi
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.