Re: [PATCHv2 3/6] io_uring: add support for kernel registered bvecs

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

On Fri, Feb 14, 2025 at 08:26:17AM -0700, Keith Busch wrote:
> On Fri, Feb 14, 2025 at 11:30:11AM +0800, Ming Lei wrote:
> > On Mon, Feb 10, 2025 at 04:56:43PM -0800, Keith Busch wrote:
> > > +
> > > +	node->release = release;
> > > +	node->priv = rq;
> > > +
> > > +	nr_bvecs = blk_rq_nr_phys_segments(rq);
> > > +	imu = kvmalloc(struct_size(imu, bvec, nr_bvecs), GFP_KERNEL);
> > > +	if (!imu) {
> > > +		kfree(node);
> > > +		return -ENOMEM;
> > > +	}
> > > +
> > > +	imu->ubuf = 0;
> > > +	imu->len = blk_rq_bytes(rq);
> > > +	imu->acct_pages = 0;
> > > +	imu->nr_bvecs = nr_bvecs;
> > > +	refcount_set(&imu->refs, 1);
> > > +	node->buf = imu;
> > 
> > request buffer direction needs to be stored in `imu`, for READ,
> > the buffer is write-only, and for WRITE, the buffer is read-only,
> > which isn't different with user mapped buffer.
> > 
> > Meantime in read_fixed/write_fixed side or buffer lookup abstraction
> > helper, the buffer direction needs to be validated.
> 
> I suppose we could add that check, but the primary use case doesn't even
> use those operations. They're using uring_cmd with the FIXED flag, and
> io_uring can't readily validate the data direction from that interface.

The check can be added to io_import_fixed().

It is a security trouble. Without the validation:

- kernel data can be redirected to user file via write_fixed,

- kernel page data is over-written unexpectedly via read_fixed, cause fs corruption or
even kernel panic.


Thanks,
Ming