Re: [PATCH V3 05/12] io_uring: bpf: extend io_uring with bpf struct_ops

[Ming Lei <ming.lei@redhat.com>]

On Fri, Apr 03, 2026 at 09:46:29AM -0600, Jens Axboe wrote:
> On 4/2/26 10:17 PM, Ming Lei wrote:
> > On Wed, Mar 25, 2026 at 08:09:03PM -0600, Jens Axboe wrote:
> >> On 3/24/26 10:37 AM, Ming Lei wrote:
> >>>  int io_uring_bpf_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> >>>  {
> >>> +	struct uring_bpf_data *data = io_kiocb_to_cmd(req, struct uring_bpf_data);
> >>> +	u32 opf = READ_ONCE(sqe->bpf_op_flags);
> >>> +	unsigned char bpf_op = uring_bpf_get_op(opf);
> >>> +	const struct uring_bpf_ops *ops;
> >>> +
> >>> +	if (unlikely(!(req->ctx->flags & IORING_SETUP_BPF_EXT)))
> >>> +		goto fail;
> >>> +
> >>> +	if (bpf_op >= IO_RING_MAX_BPF_OPS)
> >>> +		return -EINVAL;
> >>> +
> >>> +	ops = req->ctx->bpf_ext_ops[bpf_op].ops;
> >>> +	data->opf = opf;
> >>> +	data->ops = ops;
> >>> +	if (ops && ops->prep_fn)
> >>> +		return ops->prep_fn(data, sqe);
> >>> +fail:
> >>>  	return -EOPNOTSUPP;
> >>>  }
> >>
> >> Any early exit should ensure 'data' is sane, so that the cleanup doesn't
> >> potentially touch uninitialized crap. This is something that has bit us
> >> in the past. Not an issue for this patch that adds the code, but it will
> >> be once the next patch is applied. Better to clear ->opf/ops here
> >> upfront, so that we never leave this function without 'data' being fully
> >> initialized.
> > 
> > But ->cleanup() is only called in case of REQ_F_NEED_CLEANUP.
> > 
> > Or maybe you mean other cleanup instead of ->cleanup()?
> 
> I do mean ->cleanup() - what I'm trying to say here is that we've had
> cases of REQ_F_NEED_CLEANUP being set late, and hence missing cleanup
> for easily hit error conditions, and non-initialized data being exposed
> in cleanup. It's very easy to miss for later patches that adds another
> error condition. My recommendation is to fully initialize 'data' and set
> REQ_F_NEED_CLEANUP early, which handily avoids that for future changes
> too.

OK, your recommendation is definitely clean & safe, will take it.


Thanks,
Ming