Add dmabuf read/write via io_uring
Subsystems (9)
v7 cover Add dmabuf read/write via io_uring
The patch set allows to register a dmabuf to an io_uring instance for a specified file and use it with io_uring read / write requests. The infrastructure is not tied to io_uring and there could be more users in the future. A similar idea was attempted some years ago by Keith [1], from where I borrowed a number of changes. Later it was brough back up to life by Tushar and Vishal.
It's an opt-in feature for files, and they need to implement a new file operation to use it. Only NVMe block devices are supported in this series. The user API is built on top of io_uring's "registered buffers", where a dmabuf is registered in a special way, but after it can be used as any other "registered buffer" with IORING_OP_{READ,WRITE}_FIXED requests. It's created via a new file operation and the resulted map is then passed through the I/O stack in a new iterator type. There is some additional infrastructure to glue it together, count requests, manage lifetime and implement invalidation.
Tushar, William, Phil did a lot of testing and experimentation on various devices with previous versions of the patch set, and I've received lots of help from Anuj, Kanchan and Nitesh with investigations, testing, and patching. Earlier benchmarks by Anuj for IOMMU optimisations with udmabuf showed:
STRICT: before = 570 KIOPS, after = 5.01 MIOPS
LAZY: before = 1.93 MIOPS, after = 5.01 MIOPS
PASSTHROUGH: before = 5.01 MIOPS, after = 5.01 MIOPS
# Patch set structure:
- Patches 1-2 introduce internal API and infrastructure mediating io_uring and target subsystem / devices
- Patches 3-5: block layer support + prep patches
- Patches 6-8 implement NVMe support
- Patches 9-13 add io_uring support and uapi.
There are some liburing tests that can serve as an example:
git: https://github.com/isilence/liburing.git rw-dmabuf-tests-v5
url: https://github.com/isilence/liburing/tree/rw-dmabuf-tests-v5
The patches are based on Jens' for-next. Also available as a branch:
git: https://github.com/isilence/linux.git rw-dmabuf-v7
url: https://github.com/isilence/linux/tree/rw-dmabuf-v7
[1] https://lore.kernel.org/io-uring/[email protected]/
v7: - refcount *_io_ctx
- wait rcu grace period for ctx->map before freeing maps
- increment map counters under dma resv lock
- more comments about bio splitting
- make ->unmap responsible for freeing maps to match ->map
- replace dma_sync_single* with full table sync
- detach dma-buf on nvme reset after cancel nvme_cancel_tagset
v6: - remove fences and wait on invalidation synchronously
- report all map requests are detached earlier, outside of wq
- handle nvme removal
- gate nvme_free_descriptors() on iod->nr_descriptors
- fix error handling in nvme_rq_setup_dmabuf_map()
- fix iov_iter_alignment()
- relax the 1G io_uring registered dma-buf limit
- fix type overflow issue in bio split
- allocate struct dma_buf_io_ctx inside the dma-buf-io code
- harden block checks against dma-buf + buffered io
v5: - Reject dma-buf with buffered IO for raw bdev
- Add lim->max_segments bio splitting
- Add bio_iov_iter_set() helper
- Fix io_uring uapi validation
- Other minor changes
NVMe:
- Rename nvme_pci_sgl_set_data() to nvme_pci_dma_iter_set_sgl(), split into its own prep patch.
- Convert segment walk loops to do-while.
- Drop adjacent segment coalescing logic.
- Drop first_dma/first_len from nvme_pci_dmabuf_sgl_nents()
- Remove entries > NVME_MAX_SEGS bailout; moving to the block layer.
- Factor SGL vs PRP decision making into a helper.
v4: - https://lore.kernel.org/all/[email protected]/
- Add sgl support from Anuj
- Move it under drivers/dma-buf/ and rename
- Fix mis-sized allocations
- Fix io_uring re-import mishandling
- Drop map before io_uring "task work"
- Move blk-mq callback to block_device_operations
- Convert bio flag to REQ_OP*
- Other small changes
v3: https://lore.kernel.org/io-uring/[email protected]/
- Rework io_uring registration
- Move token/map infrastructure code out of blk-mq
- Simplify callbacks: remove a separate blk-mq table, which was mostly just forwarding calls (to nvme).
- Don't skip dma sync depending on request direction
- Fix a couple of hangs
- Rename s/dma/dmabuf/
- Other small changes
v2: - Don't pass raw dma addresses, wrap it into a driver specific object
- Split into two objects: token and map
- Implement move_notify
Anuj Gupta (2):
nvme-pci: rename nvme_pci_sgl_set_data to nvme_pci_dma_iter_set_sgl
nvme-pci: add SGL support for the dmabuf path
Pavel Begunkov (11):
dma-buf: introduce initial file I/O infrastructure
iov_iter: add iterator type for dmabuf maps
block: always adjust bi_offset on bio_advance_iter
block: introduce dma map backed bio type
block: add dma-buf support for raw bdev
nvme-pci: implement dma-buf backed requests io_uring/rsrc: introduce buf registration structure io_uring/rsrc: extend buffer update io_uring/rsrc: add uncloneable regbuf flag io_uring/rsrc: add regbuf import flags io_uring/rsrc: add dmabuf backed registered buffers
block/bio.c | 15 +- block/blk-merge.c | 50 ++++ block/fops.c | 27 +- drivers/dma-buf/Makefile | 2 +- drivers/dma-buf/dma-buf-io.c | 230 ++++++++++++++++ drivers/md/dm-io-rewind.c | 6 +- drivers/nvme/host/core.c | 12 + drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/pci.c | 467 ++++++++++++++++++++++++++++++++- include/linux/bio.h | 21 +- include/linux/blk-mq.h | 7 + include/linux/blk_types.h | 14 +- include/linux/blkdev.h | 2 + include/linux/bvec.h | 3 +- include/linux/dma-buf-io.h | 117 +++++++++ include/linux/fs.h | 2 + include/linux/io_uring_types.h | 5 + include/linux/iov_iter.h | 2 + include/linux/uio.h | 11 + include/uapi/linux/io_uring.h | 31 ++- io_uring/io_uring.c | 3 +- io_uring/rsrc.c | 275 ++++++++++++++++--- io_uring/rsrc.h | 45 +++- io_uring/rw.c | 6 +- lib/iov_iter.c | 29 +- 25 files changed, 1304 insertions(+), 80 deletions(-) create mode 100644 drivers/dma-buf/dma-buf-io.c create mode 100644 include/linux/dma-buf-io.h
Signature · 1 line
2.54.0
PATCH v7 1/13 dma-buf: introduce initial file I/O infrastructure
The goal is to be able to natively use dma-buf in the read-write / IO path. This patch adds basic building blocks serving as a glue and API between drivers and upper layer subsystems providing the uAPI. Later patches implement it for NVMe raw block devices and expose it to the user space via io_uring.
There are two main objects. struct dma_buf_io_ctx and struct dma_buf_io_map. The ctx is used during initial registration and serves as an interface between the upper layer user like io_uring and to the importer subsystem / driver. The map represents the actual dma map established for the target device[s] with dma_buf_map_attachment() and stored in a device specific format. The context is created via a new file operation ->init_dma_buf_io_ctx.
The ctx-map separation exists to support map invalidation (see dma_buf_io_invalidate_mappings()). A ctx can create multiple maps during its lifetime, but there can only be no more than one (active) map attached to it. Invalidation drops the active map if present, and the next map will only be attempted to be created once there is a new request that wants to use the dma-buf IO ctx.
The primary task of the dma_buf_io_map object is to count requests using it and to wait for their completion when we want to destroy the DMA map.
[un]mapping and any work with dma addresses is delegated to the importer driver via an ops table stored in the ctx, see struct dma_buf_io_ops. Only the target driver / subsystem knows about devices it wants to use the dma-buf with, especially in case of multi-device filesystems or stacking in the future.
--- drivers/dma-buf/Makefile | 2 +- drivers/dma-buf/dma-buf-io.c | 230 +++++++++++++++++++++++++++++++++++ include/linux/dma-buf-io.h | 117 ++++++++++++++++++ include/linux/fs.h | 2 + 4 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/dma-buf-io.c create mode 100644 include/linux/dma-buf-io.h
drivers/dma-buf/Makefile +1 -1
@@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ - dma-fence-unwrap.o dma-resv.o dma-buf-mapping.o + dma-fence-unwrap.o dma-resv.o dma-buf-mapping.o dma-buf-io.o obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o obj-$(CONFIG_DMABUF_HEAPS) += heaps/ obj-$(CONFIG_SYNC_FILE) += sync_file.o
drivers/dma-buf/dma-buf-io.c new file +230 -0
230 lines not on this page · raw message
include/linux/dma-buf-io.h new file +117 -0
117 lines not on this page · raw message
include/linux/fs.h +2 -0
@@ -1914,6 +1914,7 @@ struct dir_context {#define COPY_FILE_SPLICE (1 << 0)struct io_uring_cmd ; +struct dma_buf_io_ctx ;struct offset_ctx ;struct file_operations {
@@ -1960,6 +1961,7 @@ struct file_operations {int (*uring_cmd_iopoll)(struct io_uring_cmd *,struct io_comp_batch *,unsigned int poll_flags);int (*mmap_prepare)(struct vm_area_desc *); +int (*init_dma_buf_io_ctx)(struct file *,struct dma_buf_io_ctx *); } __randomize_layout;/* Supports async buffered reads */
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v7 01/13] dma-buf: introduce initial file I/O infrastructure
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] dma-buf: map leak and deadlock on invalid min_seg_shift - [Medium] dma-buf: memory leak in…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dma-buf: map leak and deadlock on invalid min_seg_shift
- [Medium] dma-buf: memory leak in dma_buf_io_ctx_create() dev_ops validation --
commit cebbd64805f3eac146806ef3dc1344fdf0a76c98
Author: Pavel Begunkov <[email protected]>
dma-buf: introduce initial file I/O infrastructure
This patch adds basic building blocks serving as an API between drivers and upper layer subsystems to natively use dma-buf in the IO paths. It introduces the dma_buf_io_ctx and dma_buf_io_map structures to handle mapping, unmapping, and invalidation operations.
Quoted 4 lines
drivers/dma-buf/dma-buf-io.c new file +0
-0
[ ... ]
Quoted 2 lines
drivers/dma-buf/dma-buf-io.c new file +2
-0+struct dma_buf_io_map *dma_buf_io_create_map (struct dma_buf_io_ctx *ctx) +{
[ ... ]
drivers/dma-buf/dma-buf-io.c line 138
Quoted 11 lines
drivers/dma-buf/dma-buf-io.c new file +11
-0+ map = ctx->dev_ops->map (ctx); +if (!IS_ERR (map)) { +atomic_inc (&ctx->active_maps); +atomic_inc (&ctx->all_maps); + } +dma_resv_unlock (dmabuf->resv); + +if (IS_ERR (map)) +return map; +if (WARN_ON_ONCE (!map->min_seg_shift)) +return ERR_PTR (-EFAULT );
[Severity: High]
Does this error path leak the map and deadlock the system?
If map->min_seg_shift evaluates to 0 (for example, if an odd-length DMA segment causes __ffs() to return 0), the WARN_ON_ONCE check will trigger.
Since atomic_inc(&ctx->active_maps) and atomic_inc(&ctx->all_maps) have already executed for a valid map, returning ERR_PTR(-EFAULT) here skips dropping the map's percpu_ref count and leaves the atomic counters permanently elevated.
Could subsequent cleanup calls to dma_buf_io_wait_active_maps() or dma_buf_io_wait_maps() then block forever waiting for the counters to reach zero?
[ ... ]
Quoted 5 lines
drivers/dma-buf/dma-buf-io.c new file +5
-0+int dma_buf_io_ctx_create (struct file *file, +struct dma_buf *dmabuf, +enum dma_data_direction dir, +struct dma_buf_io_ctx **out_ctx) +{
[ ... ]
drivers/dma-buf/dma-buf-io.c line 217
Quoted 12 lines
drivers/dma-buf/dma-buf-io.c new file +12
-0+ ret = file->f_op->init_dma_buf_io_ctx (file, ctx); +if (ret) { +kfree (ctx); +dma_buf_put (dmabuf); +return ret; + } + +if (WARN_ON_ONCE (!ctx->dev_ops || + !ctx->dev_ops->map || + !ctx->dev_ops->unmap || + !ctx->dev_ops->release)) +return -EINVAL ;
[Severity: Medium]
Can this early return leak the context and the dma-buf reference?
If the init_dma_buf_io_ctx callback succeeds but fails to populate the required dev_ops, this path returns -EINVAL immediately.
It appears to skip the kfree(ctx) and dma_buf_put(dmabuf) teardown operations that are present in the preceding error handling block.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH v7 01/13] dma-buf: introduce initial file I/O infrastructure
In reply to Pavel BegunkovLe 28/09/2026 à 15:32, Pavel Begunkov a écrit :
Quoted 25 lines
The goal is to be able to natively use dma-buf in the read-write / IO path. This patch adds basic building blocks serving as a glue and API between drivers and upper layer subsystems providing the uAPI. Later patches implement it for NVMe raw block devices and expose it to the user space via
io_uring.There are two main objects. struct
dma_buf_io_ctxand structdma_buf_io_map. The ctx is used during initial registration and serves as an interface between the upper layer user likeio_uringand to the importer subsystem / driver. The map represents the actual dma map established for the target device[s] withdma_buf_map_attachment()and stored in a device specific format. The context is created via a new file operation ->init_dma_buf_io_ctx.The ctx-map separation exists to support map invalidation (see
dma_buf_io_invalidate_mappings()). A ctx can create multiple maps during its lifetime, but there can only be no more than one (active) map attached to it. Invalidation drops the active map if present, and the next map will only be attempted to be created once there is a new request that wants to use the dma-buf IO ctx.The primary task of the
dma_buf_io_mapobject is to count requests using it and to wait for their completion when we want to destroy the DMA map.
Quoted 8 lines
[un]mapping and any work with dma addresses is delegated to the importer driver via an ops table stored in the ctx, see struct
dma_buf_io_ops. Only the target driver / subsystem knows about devices it wants to use the dma-buf with, especially in case of multi-device filesystems or stacking in the future.Signed-off-by Pavel Begunkov <[email protected]>---
Hi,
a few nitpicks below, should it help
drivers/dma-buf/dma-buf-io.c line 153
Quoted 5 lines
drivers/dma-buf/dma-buf-io.c new file +5
-0+static void dma_buf_io_kill_maps (struct dma_buf_io_ctx *ctx,bool final) +{ +struct dma_buf_io_map *map; + + scoped_guard(mutex, &ctx->map_mutex) {
guard() is enough. This saves indentation.
Quoted 11 lines
drivers/dma-buf/dma-buf-io.c new file +11
-0+if (final) + ctx->maps_killed = true; + + map =rcu_dereference_protected (ctx->map, +lockdep_is_held (&ctx->map_mutex)); +if (!map) +return ; +rcu_assign_pointer (ctx->map,NULL ); +percpu_ref_kill (&map->refs); + } +}
...
drivers/dma-buf/dma-buf-io.c line 191
Quoted 12 lines
drivers/dma-buf/dma-buf-io.c new file +12
-0+int dma_buf_io_ctx_create (struct file *file, +struct dma_buf *dmabuf, +enum dma_data_direction dir, +struct dma_buf_io_ctx **out_ctx) +{ +struct dma_buf_io_ctx *ctx; +int ret; + +if (!file->f_op->init_dma_buf_io_ctx) +return -EOPNOTSUPP ; + + ctx =kmalloc_obj (*ctx);
kzalloc_obj() and save the memset() below
Quoted 20 lines
drivers/dma-buf/dma-buf-io.c new file +20
-0+if (!ctx) +return -ENOMEM ; + +memset (ctx,0 ,sizeof (*ctx)); + ctx->dir = dir; + ctx->dmabuf = dmabuf; +get_dma_buf (dmabuf); +mutex_init (&ctx->map_mutex); +mutex_init (&ctx->map_create_mutex); +atomic_set (&ctx->active_maps,0 ); +atomic_set (&ctx->all_maps,0 ); +refcount_set (&ctx->refs,1 ); +init_waitqueue_head (&ctx->maps_wq); + + ret = file->f_op->init_dma_buf_io_ctx (file, ctx); +if (ret) { +kfree (ctx); +dma_buf_put (dmabuf); +return ret; + }
drivers/dma-buf/dma-buf-io.c line 221
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+ +if (WARN_ON_ONCE (!ctx->dev_ops || + !ctx->dev_ops->map || + !ctx->dev_ops->unmap || + !ctx->dev_ops->release)) +return -EINVAL ; + + *out_ctx = ctx; +return 0 ; +}
...
CJ
PATCH v7 2/13 iov_iter: add iterator type for dmabuf maps
In reply to Pavel BegunkovIntroduce a new iterator type for dmabuf maps. The map in an opaque object with internals and format specific to the subsystem / driver, and only it can use that subsystem / driver for issuing IO. The task of the middle layers is to pass the map / iterator further down, maybe doing basic splitting and length checking. The iterator can only be used by operations of the file the associated map was created for.
--- include/linux/iov_iter.h | 2 ++ include/linux/uio.h | 11 +++++++++++ lib/iov_iter.c | 29 +++++++++++++++++++++++------ 3 files changed, 36 insertions(+), 6 deletions(-)
include/linux/iov_iter.h +2 -0
@@ -310,6 +310,8 @@ size_t iterate_and_advance2(struct iov_iter *iter, size_t len, void *priv,return iterate_folioq (iter, len, priv, priv2, step);if (iov_iter_is_xarray (iter))return iterate_xarray (iter, len, priv, priv2, step); +if (WARN_ON_ONCE (iov_iter_is_dmabuf_map (iter))) +return 0 ;return iterate_discard (iter, len, priv, priv2, step); }
include/linux/uio.h +11 -0
@@ -12,6 +12,7 @@struct page ;struct folio_queue ; +struct dma_buf_io_map ;typedef unsigned int __bitwise iov_iter_extraction_t ;
@@ -29,6 +30,7 @@ enum iter_type {ITER_FOLIOQ ,ITER_XARRAY ,ITER_DISCARD , + ITER_DMABUF_MAP, };#define ITER_SOURCE 1 // == WRITE
@@ -71,6 +73,7 @@ struct iov_iter {const struct folio_queue *folioq;struct xarray *xarray;void __user *ubuf; +struct dma_buf_io_map *dmabuf_map; };size_t count; };
@@ -155,6 +158,11 @@ static inline bool iov_iter_is_xarray(const struct iov_iter *i)return iov_iter_type (i) ==ITER_XARRAY ; } +static inline bool iov_iter_is_dmabuf_map (const struct iov_iter *i) +{ +return iov_iter_type (i) ==ITER_DMABUF_MAP ; +} +static inline unsigned char iov_iter_rw (const struct iov_iter *i) {return i->data_source ?WRITE :READ ;
@@ -300,6 +308,9 @@ void iov_iter_folio_queue(struct iov_iter *i, unsigned int direction, unsigned int first_slot, unsigned int offset, size_t count);void iov_iter_xarray (struct iov_iter *i,unsigned int direction,struct xarray *xarray,loff_t start,size_t count); +void iov_iter_dmabuf_map (struct iov_iter *i,unsigned int direction, +struct dma_buf_io_map *map, +loff_t off,size_t count);ssize_t iov_iter_get_pages2 (struct iov_iter *i,struct page **pages,size_t maxsize,unsigned maxpages,size_t *start); ssize_t iov_iter_get_pages_alloc2(struct iov_iter *i, struct page ***pages,
lib/iov_iter.c +23 -6
@@ -575,7 +575,8 @@ void iov_iter_advance(struct iov_iter *i, size_t size) {if (unlikely (i->count < size)) size = i->count; -if (likely (iter_is_ubuf (i)) ||unlikely (iov_iter_is_xarray (i))) { +if (likely (iter_is_ubuf (i)) ||unlikely (iov_iter_is_xarray (i)) || +unlikely (iov_iter_is_dmabuf_map (i))) { i->iov_offset += size; i->count -= size; }else if (likely (iter_is_iovec (i) ||iov_iter_is_kvec (i))) {
@@ -631,7 +632,8 @@ void iov_iter_revert(struct iov_iter *i, size_t unroll)return ; } unroll -= i->iov_offset; -if (iov_iter_is_xarray (i) ||iter_is_ubuf (i)) { +if (iov_iter_is_xarray (i) ||iter_is_ubuf (i) || +iov_iter_is_dmabuf_map (i)) {BUG ();/* We should never go beyond the start of the specified * range since we might then be straying into pages that * aren't pinned.
@@ -775,6 +777,20 @@ void iov_iter_xarray(struct iov_iter *i, unsigned int direction, }EXPORT_SYMBOL (iov_iter_xarray); +void iov_iter_dmabuf_map (struct iov_iter *i,unsigned int direction, +struct dma_buf_io_map *map, +loff_t off,size_t count) +{ +WARN_ON (direction & ~(READ |WRITE )); + *i = (struct iov_iter ){ + .iter_type =ITER_DMABUF_MAP , + .data_source = direction, + .dmabuf_map = map, + .count = count, + .iov_offset = off, + }; +} +/** * iov_iter_discard - Initialise an I/O iterator that discards data * @i: The iterator to initialise.
@@ -856,7 +872,7 @@ unsigned long iov_iter_alignment(const struct iov_iter *i)return iov_iter_alignment_bvec (i);/* With both xarray and folioq types, we're dealing with whole folios. */ -if (iov_iter_is_folioq (i)) +if (iov_iter_is_folioq (i) ||iov_iter_is_dmabuf_map (i))return i->iov_offset | i->count;if (iov_iter_is_xarray (i))return (i->xarray_start + i->iov_offset) | i->count;
@@ -872,7 +888,7 @@ unsigned long iov_iter_gap_alignment(const struct iov_iter *i)size_t size = i->count;unsigned k; -if (iter_is_ubuf (i)) +if (iter_is_ubuf (i) ||iov_iter_is_dmabuf_map (i))return 0 ;if (WARN_ON (!iter_is_iovec (i)))
@@ -1469,11 +1485,12 @@ EXPORT_SYMBOL_GPL(import_ubuf);void iov_iter_restore (struct iov_iter *i,struct iov_iter_state *state) {if (WARN_ON_ONCE (!iov_iter_is_bvec (i) && !iter_is_iovec (i) && - !iter_is_ubuf (i)) && !iov_iter_is_kvec (i)) + !iter_is_ubuf (i) && !iov_iter_is_kvec (i) && + !iov_iter_is_dmabuf_map (i)))return ; i->iov_offset = state->iov_offset; i->count = state->count; -if (iter_is_ubuf (i)) +if (iter_is_ubuf (i) ||iov_iter_is_dmabuf_map (i))return ;/* * For the *vec iters, nr_segs + iov is constant - if we increment
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v7 02/13] iov_iter: add iterator type for dmabuf maps
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] iov_iter: silent data discard for ITER_DMABUF_MAP in iterate_and_advance_kernel - [Medium]…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] iov_iter: silent data discard for ITER_DMABUF_MAP in iterate_and_advance_kernel
- [Medium] iov_iter: iov_iter_revert() fails to prevent ITER_DMABUF_MAP from reverting past start --
commit 592c6072da9af2f29c973e9a7808406da6345041
Author: Pavel Begunkov <[email protected]>iov_iter: add iterator type for dmabuf maps
Introduce a new iterator type for dmabuf maps. The map in an opaque object with internals and format specific to the subsystem / driver, and only it can use that subsystem / driver for issuing IO.
include/linux/iov_iter.h size_t iterate_and_advance2(struct iov_iter *iter, size_t len, void *priv, · line 316
Quoted 12 lines
include/linux/iov_iter.h +2
-0@@ -310,6 +310,8 @@ size_t iterate_and_advance2(struct iov_iter *iter, size_t len, void *priv,return iterate_folioq (iter, len, priv, priv2, step);if (iov_iter_is_xarray (iter))return iterate_xarray (iter, len, priv, priv2, step); +if (WARN_ON_ONCE (iov_iter_is_dmabuf_map (iter))) +return 0 ;return iterate_discard (iter, len, priv, priv2, step); }
[Severity: High]
This safely rejects ITER_DMABUF_MAP by returning 0, but is there a missing guard in iterate_and_advance_kernel() which is defined later in this file?
If a kernel subsystem uses iterate_and_advance_kernel() on an ITER_DMABUF_MAP, it will unconditionally fall through to iterate_discard():
if (iov_iter_is_xarray(iter)) return iterate_xarray(iter, len, priv, priv2, step); return iterate_discard(iter, len, priv, priv2, step);
iterate_discard() decrements count and returns the requested len, falsely indicating that the data was successfully processed without actually calling the step function.
Can this lead to silent data discard or corruption (such as hashing missing data) without any warning or error propagation?
Quoted 4 lines
lib/iov_iter.c +0
-0
[ ... ]
lib/iov_iter.c void iov_iter_revert(struct iov_iter *i, size_t unroll) · line 639
Quoted 10 lines
lib/iov_iter.c +2
-1@@ -631,7 +632,8 @@ void iov_iter_revert(struct iov_iter *i, size_t unroll)return ; } unroll -= i->iov_offset; -if (iov_iter_is_xarray (i) ||iter_is_ubuf (i)) { +if (iov_iter_is_xarray (i) ||iter_is_ubuf (i) || +iov_iter_is_dmabuf_map (i)) {BUG ();/* We should never go beyond the start of the specified * range since we might then be straying into pages that * aren't pinned.
[Severity: Medium]
Does this safely prevent ITER_DMABUF_MAP from reverting past its start?
iov_iter_dmabuf_map() initializes i->iov_offset to the absolute offset (off) rather than 0.
Earlier in iov_iter_revert(), the boundary check compares the revert amount against this absolute offset rather than the amount advanced:
if (unroll <= i->iov_offset) {
i->iov_offset -= unroll;
return;
}Because of this early return, the revert can rewind the iterator by up to off + advanced bytes without triggering the BUG() safeguard shown in this hunk.
Can this allow a caller to silently rewind the iterator to an out-of-bounds offset before the mapped range?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Christoph Hellwig
maintainerRe: [PATCH v7 02/13] iov_iter: add iterator type for dmabuf maps
In reply to Pavel BegunkovQuoted 10 lines · Pavel Begunkov
Introduce a new iterator type for dmabuf maps. The map in an opaque object with internals and format specific to the subsystem / driver, and only it can use that subsystem / driver for issuing IO. The task of the middle layers is to pass the map / iterator further down, maybe doing basic splitting and length checking. The iterator can only be used by operations of the file the associated map was created for.
Suggested-by Keith Busch <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>Signed-off-by Pavel Begunkov <[email protected]>
FYI, I think the Sashiko complaint about iterate_and_advance2 is correct here. We don't really hit that in the series, but leaving iov_iter methods partially implemented and fall through to others seems dangerous. At minimum that needs a WARN_ON to catch that case.
Re: [PATCH v7 02/13] iov_iter: add iterator type for dmabuf maps
Quoted 8 lines · Christoph Hellwig
Quoted 7 lines · Pavel Begunkov
Introduce a new iterator type for dmabuf maps. The map in an opaque object with internals and format specific to the subsystem / driver, and only it can use that subsystem / driver for issuing IO. The task of the middle layers is to pass the map / iterator further down, maybe doing basic splitting and length checking. The iterator can only be used by operations of the file the associated map was created for.
Quoted 8 lines
Quoted 3 lines
Suggested-by Keith Busch <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>Signed-off-by Pavel Begunkov <[email protected]>FYI, I think the Sashiko complaint about
iterate_and_advance2is correct here. We don't really hit that in the series, but leavingiov_itermethods partially implemented and fall through to others seems dangerous. At minimum that needs aWARN_ONto catch that case.
v7 already covers iterate_and_advance2(). It complains about iterate_and_advance_kernel() but there are many types this one doesn't support.
Signature · 1 line
Pavel Begunkov
PATCH v7 3/13 block: always adjust bi_offset on bio_advance_iter
In reply to Pavel BegunkovExtend bio_no_advance_iter() iteration to also increment the offset. It's not currently needed because the currently listed request types don't have a buffer, but will be useful once we add bios backed by dma-buf, which don't have a bvec but work with a single range.
--- drivers/md/dm-io-rewind.c | 6 ++++-- include/linux/bio.h | 12 ++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-)
drivers/md/dm-io-rewind.c +4 -2
@@ -113,10 +113,12 @@ static inline void dm_bio_rewind_iter(const struct bio *bio, iter->bi_sector -= bytes >>9 ;/* No advance means no rewind */ -if (bio_no_advance_iter (bio)) +if (bio_no_advance_iter (bio)) { iter->bi_size += bytes; -else + iter->bi_offset -= bytes; + }else {dm_bvec_iter_rewind (bio->bi_io_vec, iter, bytes); + } }/**
include/linux/bio.h +8 -4
@@ -113,11 +113,13 @@ static inline void bio_advance_iter(const struct bio *bio, { iter->bi_sector += bytes >>9 ; -if (bio_no_advance_iter (bio)) +if (bio_no_advance_iter (bio)) { iter->bi_size -= bytes; -else + iter->bi_offset += bytes; + }else {bvec_iter_advance (bio->bi_io_vec, iter, bytes);/* TODO: It is reasonable to complete bio with error here. */ + } }/* @bytes should be less or equal to bvec[i->bi_idx].bv_len */
@@ -127,10 +129,12 @@ static inline void bio_advance_iter_single(const struct bio *bio, { iter->bi_sector += bytes >>9 ; -if (bio_no_advance_iter (bio)) +if (bio_no_advance_iter (bio)) { iter->bi_size -= bytes; -else + iter->bi_offset += bytes; + }else {bvec_iter_advance_single (bio->bi_io_vec, iter, bytes); + } }void __bio_advance (struct bio *,unsigned bytes);
Signature · 1 line
2.54.0
PATCH v7 4/13 block: introduce dma map backed bio type
In reply to Pavel BegunkovPremapped buffers don't require a generic bio_vec since these have already been dma mapped. Repurpose the bi_io_vec space to store dmabuf maps as they are mutually exclusive.
The bio splitting differs from the normal path because it's already pre-mapped and for the block layer it's just an offset into the dma-buf. The actual segmentation is only available to the importer driver and not the block layer, however, it doesn't contain alignment gaps and we don't have to check it. For the same reason we can't precisely split by the number of segments, but we use ->min_seg_shift stored in the map to calculate the minimum number of bytes a request consisting of lim->max_segments full segments can cover and split by that. It's stricter and can add extra splitting. E.g. for a {4K, 4G} segmentation, the min segment size is 4K, and we'll split it into bios of (4K * lim->max_segments) bytes each, but it should be good enough for now to cover the most popular use cases.
--- block/bio.c | 15 ++++++++++-- block/blk-merge.c | 50 +++++++++++++++++++++++++++++++++++++++ block/fops.c | 2 +- include/linux/bio.h | 9 +++---- include/linux/blk-mq.h | 7 ++++++ include/linux/blk_types.h | 14 ++++++++++- include/linux/bvec.h | 3 ++- 7 files changed, 91 insertions(+), 9 deletions(-)
block/bio.c +13 -2
@@ -881,7 +881,11 @@ static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp) bio->bi_write_stream = bio_src->bi_write_stream; bio->bi_bvec_gap_bit = bio_src->bi_bvec_gap_bit; bio->bi_iter = bio_src->bi_iter; - bio->bi_io_vec = bio_src->bi_io_vec; + +if (op_is_dmabuf (bio->bi_opf)) + bio->bi_dmabuf_map = bio_src->bi_dmabuf_map; +else + bio->bi_io_vec = bio_src->bi_io_vec;if (bio->bi_bdev) { if (bio->bi_bdev == bio_src->bi_bdev &&
@@ -1204,16 +1208,23 @@ EXPORT_SYMBOL_GPL(__bio_release_pages);bool bio_iov_iter_set (struct bio *bio,const struct iov_iter *iter) { -if (!iov_iter_is_bvec (iter)) +if (!iov_iter_is_bvec (iter) && !iov_iter_is_dmabuf_map (iter))return false;WARN_ON_ONCE (bio->bi_max_vecs); +static_assert (offsetof(struct bio , bi_io_vec) == + offsetof(struct bio , bi_dmabuf_map)); +static_assert (offsetof(struct iov_iter , bvec) == + offsetof(struct iov_iter , dmabuf_map)); + bio->bi_io_vec = (struct bio_vec *)iter->bvec; bio->bi_iter.bi_idx =0 ; bio->bi_iter.bi_offset = iter->iov_offset; bio->bi_iter.bi_size =iov_iter_count (iter);bio_set_flag (bio,BIO_CLONED ); +if (iov_iter_is_dmabuf_map (iter)) + bio->bi_opf |=REQ_NOMERGE |REQ_DMABUF ;return true; }
block/blk-merge.c +50 -0
74 lines not on this page · raw message
block/fops.c +1 -1
@@ -363,7 +363,7 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,* Users don't rely on the iterator being in any particular * state for async I/O returning -EIOCBQUEUED, hence we can * avoid expensive iov_iter_advance(). Bypass -* bio_iov_iter_get_pages() and set the bvec directly. +* bio_iov_iter_get_pages() and set the bvec/dmabuf directly. */ if (!bio_iov_iter_set (bio, iter)) { ret =blkdev_iov_iter_get_pages (bio, iter, bdev);
include/linux/bio.h +5 -4
@@ -80,7 +80,8 @@ static inline bool bio_no_advance_iter(const struct bio *bio) {return bio_op (bio) ==REQ_OP_DISCARD ||bio_op (bio) ==REQ_OP_SECURE_ERASE || -bio_op (bio) ==REQ_OP_WRITE_ZEROES ; +bio_op (bio) ==REQ_OP_WRITE_ZEROES || +op_is_dmabuf (bio->bi_opf); }static inline void *bio_data (struct bio *bio)
@@ -444,12 +445,12 @@ static inline void bio_wouldblock_error(struct bio *bio)/* * Calculate number of bvec segments that should be allocated to fit data -* pointed by @iter. If @iter is backed by bvec it's going to be reused -* instead of allocating a new one. +* pointed by @iter. If @iter is backed by a bvec or a dmabuf, the bvec array / +* the dma map are going to be reused, and so no extra allocation is required. */ static inline int bio_iov_vecs_to_alloc (struct iov_iter *iter,int max_segs) { -if (iov_iter_is_bvec (iter)) +if (iov_iter_is_bvec (iter) ||iov_iter_is_dmabuf_map (iter))return 0 ;return iov_iter_npages (iter, max_segs); }
include/linux/blk-mq.h +7 -0
@@ -1017,6 +1017,13 @@ static inline void *blk_mq_rq_to_pdu(struct request *rq)return rq +1 ; } +static inline bool blk_mq_rq_is_dmabuf (struct request *rq) +{ +if (!IS_ENABLED (CONFIG_DMA_SHARED_BUFFER )) +return false; +return rq->bio &&op_is_dmabuf (rq->bio->bi_opf); +} +static inline struct blk_mq_hw_ctx *queue_hctx (struct request_queue *q,int id) {struct blk_mq_hw_ctx *hctx;
include/linux/blk_types.h +13 -1
@@ -233,7 +233,12 @@ struct bio {atomic_t __bi_remaining;/* The actual vec list, preserved by bio_reset() */ -struct bio_vec *bi_io_vec; +union { +struct bio_vec *bi_io_vec; +/* Driver specific dma map, valid IFF REQ_DMABUF is set */ +struct dma_buf_io_map *bi_dmabuf_map; + }; +struct bvec_iter bi_iter;union {
@@ -402,6 +407,7 @@ enum req_flag_bits { __REQ_DRV,/* for driver use */ __REQ_FS_PRIVATE,/* for file system (submitter) use */ __REQ_ATOMIC,/* for atomic write operations */ + __REQ_DMABUF, /* Using premmaped dma buffers *//* * Command specific flags, keep last: */
@@ -434,6 +440,7 @@ enum req_flag_bits {#define REQ_DRV (__force blk_opf_t)(1ULL << __REQ_DRV)#define REQ_FS_PRIVATE (__force blk_opf_t)(1ULL << __REQ_FS_PRIVATE)#define REQ_ATOMIC (__force blk_opf_t)(1ULL << __REQ_ATOMIC) +#define REQ_DMABUF (__force blk_opf_t)(1ULL << __REQ_DMABUF)#define REQ_NOUNMAP (__force blk_opf_t)(1ULL << __REQ_NOUNMAP)
@@ -487,6 +494,11 @@ static inline bool op_is_discard(blk_opf_t op)return (op &REQ_OP_MASK ) ==REQ_OP_DISCARD ; } +static inline bool op_is_dmabuf (blk_opf_t op) +{ +return op &REQ_DMABUF ; +} +/* * Check if a bio or request operation is a zone management operation. */
include/linux/bvec.h +2 -1
@@ -108,7 +108,8 @@ struct bvec_iter {unsigned int bi_idx;/* -* Current offset in the bvec entry pointed to by `bi_idx`. +* Current offset in the bvec entry pointed to by `bi_idx` or into +* a dma-buf map. */ unsigned int bi_offset; } __packed__aligned(4) ;
Signature · 1 line
2.54.0
PATCH v7 5/13 block: add dma-buf support for raw bdev
In reply to Pavel BegunkovAdd a simple proxy implementation of init_dma_buf_io_ctx() forwarding the call to a new struct block_device_operations operation. Also reject dma-buf backed iterators for buffered IO.
--- block/fops.c | 25 ++++++++++++++++++++++++- include/linux/blkdev.h | 2 ++ 2 files changed, 26 insertions(+), 1 deletion(-)
block/fops.c +24 -1
@@ -783,7 +783,8 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)if (iocb->ki_flags &IOCB_DIRECT ) { ret =blkdev_direct_write (iocb, from); -if (ret >=0 &&iov_iter_count (from)) { +if (ret >=0 &&iov_iter_count (from) && + !iov_iter_is_dmabuf_map (from)) { ret =direct_write_fallback (iocb, from, ret,blkdev_buffered_write (iocb, from)); need_sync = true;
@@ -796,6 +797,9 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) need_sync = true; } }else { +if (unlikely (iov_iter_is_dmabuf_map (from))) +return -EOPNOTSUPP ; +/* * Take i_rwsem and invalidate_lock to avoid racing with * set_blocksize changing i_blkbits/folio order and punching
@@ -851,6 +855,11 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)if (ret <0 || !count) goto reexpand; } +if (unlikely (iov_iter_is_dmabuf_map (to))) { +if (!ret) + ret = -EOPNOTSUPP ; + goto reexpand; + }/* * Take i_rwsem and invalidate_lock to avoid racing with set_blocksize
@@ -954,6 +963,19 @@ static int blkdev_mmap_prepare(struct vm_area_desc *desc)return generic_file_mmap_prepare (desc); } +static int blkdev_init_dma_buf_io_ctx (struct file *file, +struct dma_buf_io_ctx *ctx) +{ +struct block_device *bdev =file_bdev (file); +struct gendisk *disk = bdev->bd_disk; + +if (!(file->f_flags &O_DIRECT )) +return -EINVAL ; +if (!disk->fops->init_dma_buf_io_ctx) +return -EOPNOTSUPP ; +return disk->fops->init_dma_buf_io_ctx (bdev, ctx); +} +const struct file_operations def_blk_fops = { .open = blkdev_open, .release = blkdev_release,
@@ -972,6 +994,7 @@ const struct file_operations def_blk_fops = { .fallocate = blkdev_fallocate, .uring_cmd = blkdev_uring_cmd, .fop_flags = FOP_BUFFER_RASYNC | FOP_DONTCACHE, + .init_dma_buf_io_ctx = blkdev_init_dma_buf_io_ctx, };static __init int blkdev_init (void )
include/linux/blkdev.h +2 -0
@@ -1599,6 +1599,8 @@ struct block_device_operations {/* returns the length of the identifier or a negative errno: */ int (*get_unique_id)(struct gendisk *disk,u8 id[16 ],enum blk_unique_id id_type); +int (*init_dma_buf_io_ctx)(struct block_device *, +struct dma_buf_io_ctx *);struct module *owner;const struct pr_ops *pr_ops;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v7 05/13] block: add dma-buf support for raw bdev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] block: blkdev_write_iter swallows -EBUSY and returns 0 for dmabuf direct writes --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] block: blkdev_write_iter swallows -EBUSY and returns 0 for dmabuf direct writes --
commit afbe81886595b7f9b2e6456a34c387448997c06f
Author: Pavel Begunkov <[email protected]>
block: add dma-buf support for raw bdev
Add a simple proxy implementation of init_dma_buf_io_ctx() forwarding the call to a new struct block_device_operations operation. Also reject dma-buf backed iterators for buffered IO.
block/fops.c static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) · line 792
Quoted 11 lines
block/fops.c +2
-1@@ -788,7 +788,8 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)if (iocb->ki_flags &IOCB_DIRECT ) { ret =blkdev_direct_write (iocb, from); -if (ret >=0 &&iov_iter_count (from)) { +if (ret >=0 &&iov_iter_count (from) && + !iov_iter_is_dmabuf_map (from)) {
[Severity: Medium]
Could skipping this fallback result in returning a false 0 to userspace instead of an error code?
If a dmabuf direct write is submitted via io_uring to a block device region where page cache invalidation fails (e.g., due to an elevated refcount from mmap), blkdev_direct_write() intentionally translates -EBUSY to a 0 return value to trigger a fallback to buffered I/O:
block/fops.c:blkdev_direct_write() {
...
written = kiocb_invalidate_pages(iocb, count);
if (written) {
if (written == -EBUSY)
return 0;
return written;
}
...
}Since the !iov_iter_is_dmabuf_map(from) check skips the direct_write_fallback() path, ret remains 0. Would this cause userspace to incorrectly receive 0 (indicating EOF or no bytes written) instead of an error code like -EOPNOTSUPP or -EBUSY? Applications that retry on 0-byte writes might enter an infinite retry loop.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
Christoph Hellwig
maintainerRe: [PATCH v7 05/13] block: add dma-buf support for raw bdev
In reply to Pavel BegunkovQuoted 7 lines · Pavel Begunkov
Add a simple proxy implementation of
init_dma_buf_io_ctx()forwarding the call to a new structblock_device_operationsoperation. Also reject dma-buf backed iterators for buffered IO.Reviewed-by Christoph Hellwig <[email protected]>[pavel: reject dma-buf without O_DIRECT]Signed-off-by Pavel Begunkov <[email protected]>
The Sashiko report about the errno on fallback here looks real, and is probably easily fixed by setting a proper errno.
Re: [PATCH v7 05/13] block: add dma-buf support for raw bdev
Quoted 11 lines · Christoph Hellwig
Quoted 7 lines · Pavel Begunkov
Add a simple proxy implementation of
init_dma_buf_io_ctx()forwarding the call to a new structblock_device_operationsoperation. Also reject dma-buf backed iterators for buffered IO.Reviewed-by Christoph Hellwig <[email protected]>[pavel: reject dma-buf without O_DIRECT]Signed-off-by Pavel Begunkov <[email protected]>The Sashiko report about the errno on fallback here looks real, and is probably easily fixed by setting a proper errno.
I'm going to move the check into blkdev_buffered_write(), this should cover both paths. direct_write_fallback() will see -EOPNOTSUPP and fail appropriately.
Signature · 1 line
Pavel Begunkov
PATCH v7 6/13 nvme-pci: implement dma-buf backed requests
In reply to Pavel BegunkovEnable BIO_DMABUF_MAP backed requests. On registration we map the dma-buf and store it as a prp list, which is then used to initialise requests. All attached contexts are stored in a new list dmabuf_ctxs, and additions/removals are synchronised with dmabuf_lock.
--- drivers/nvme/host/core.c | 12 ++ drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/pci.c | 308 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 322 insertions(+)
drivers/nvme/host/core.c +12 -0
@@ -2699,6 +2699,17 @@ static int nvme_report_zones(struct gendisk *disk, sector_t sector,#define nvme_report_zones NULL#endif /* CONFIG_BLK_DEV_ZONED */ +static int nvme_init_dma_buf_io_ctx (struct block_device *bdev, +struct dma_buf_io_ctx *ctx) +{ +struct nvme_ns *ns = bdev->bd_disk->private_data; +struct nvme_ctrl *ctrl = ns->ctrl; + +if (!ctrl->ops->init_dma_buf_io_ctx) +return -EINVAL ; +return ctrl->ops->init_dma_buf_io_ctx (ctrl, ctx); +} +const struct block_device_operations nvme_bdev_ops = { .owner =THIS_MODULE , .ioctl = nvme_ioctl,
@@ -2709,6 +2720,7 @@ const struct block_device_operations nvme_bdev_ops = { .get_unique_id = nvme_get_unique_id, .report_zones = nvme_report_zones, .pr_ops = &nvme_pr_ops, + .init_dma_buf_io_ctx = nvme_init_dma_buf_io_ctx, }; static int nvme_wait_ready(struct nvme_ctrl *ctrl, u32 mask, u32 val,
drivers/nvme/host/nvme.h +2 -0
@@ -667,6 +667,8 @@ struct nvme_ctrl_ops {int (*get_address)(struct nvme_ctrl *ctrl,char *buf,int size);void (*print_device_info)(struct nvme_ctrl *ctrl);bool (*supports_pci_p2pdma)(struct nvme_ctrl *ctrl); +int (*init_dma_buf_io_ctx)(struct nvme_ctrl *ctrl, +struct dma_buf_io_ctx *ctx);unsigned long (*get_virt_boundary)(struct nvme_ctrl *ctrl,bool is_admin); };
drivers/nvme/host/pci.c +308 -0
373 lines not on this page · raw message
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Critical] block: out-of-bounds heap read in nvme_rq_setup_dmabuf_map allows arbitrary DMA - [High] mm…
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] block: out-of-bounds heap read in nvme_rq_setup_dmabuf_map allows arbitrary DMA
- [High] mm: invalid kfree() on kvmalloc_flex() allocation in nvme_dma_buf_io_map
- [High] mm: heap buffer overflow in nvme_dma_buf_io_map due to truncated nr_entries
- [High] mm: double free of PRP lists on nvme_rq_setup_dmabuf_map error path --
commit 0b541f35b5346961c2c0d5b3cd4aa8c755c332a2
Author: Pavel Begunkov <[email protected]>
nvme-pci: implement dma-buf backed requests
Enable BIO_DMABUF_MAP backed requests. On registration we map the dma-buf and store it as a prp list, which is then used to initialise requests. All attached contexts are stored in a new list dmabuf_ctxs, and additions/removals are synchronised with dmabuf_lock.
Quoted 3 lines
drivers/nvme/host/pci.c +0
-0
[ ... ]
Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+static void nvme_rq_clean_dmabuf_map (struct nvme_dev *dev, +struct request *req) +{ +struct nvme_iod *iod =blk_mq_rq_to_pdu (req); + +nvme_dmabuf_map_sync_for_cpu (dev, req); + +if (iod->nr_descriptors) +nvme_free_descriptors (req); +}
[ ... ]
Quoted 16 lines
drivers/nvme/host/pci.c +16
-0+static blk_status_t nvme_rq_setup_dmabuf_map (struct request *req, +struct nvme_queue *nvmeq) +{ +struct nvme_iod *iod =blk_mq_rq_to_pdu (req); +struct bio *bio = req->bio; +struct nvme_dmabuf_map *map =to_nvme_dmabuf_map (bio->bi_dmabuf_map); +unsigned bvec_done = bio->bi_iter.bi_offset; +unsigned map_idx = bvec_done /NVME_CTRL_PAGE_SIZE ; +unsigned offset = bvec_done & (NVME_CTRL_PAGE_SIZE -1 ); +int length =blk_rq_payload_bytes (req) - (NVME_CTRL_PAGE_SIZE - offset); +dma_addr_t *dma_list = map->dma_list; +u64 prp1_dma = dma_list[map_idx++] + offset; +u64 dma_addr, prp2_dma; +dma_addr_t prp_dma; +__le64 *prp_list; +unsigned i;
drivers/nvme/host/pci.c static void nvme_free_descriptors(struct request *req) · line 945
Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+ +nvme_dmabuf_map_sync_for_device (nvmeq->dev, req); + +if (length <=0 ) { + prp2_dma =0 ; + goto done; + } + +if (length <=NVME_CTRL_PAGE_SIZE ) { + prp2_dma = dma_list[map_idx];
[Severity: Critical]
Is map_idx checked against map->nr_entries here?
If dmabuf->size is 4608 bytes, nr_entries is truncated to 1, allocating 1 array element. Userspace could submit a 4608-byte IO. The driver calculates 'length' up to 4608 and advances 'map_idx' to 1 for the remaining 512 bytes. It reads 'dma_list[1]' (which is out-of-bounds) and uses it as 'prp2_dma', potentially resulting in arbitrary physical DMA (read or write).
Quoted 2 lines
drivers/nvme/host/pci.c +2
-0+ goto done; + }
[ ... ]
drivers/nvme/host/pci.c static void nvme_free_descriptors(struct request *req) · line 990
Quoted 5 lines
drivers/nvme/host/pci.c +5
-0+free_prps: + iod->cmd.common.dptr.prp1 =cpu_to_le64 (prp1_dma); + iod->cmd.common.dptr.prp2 =cpu_to_le64 (prp2_dma); +nvme_free_descriptors (req); +return BLK_STS_RESOURCE ;
[Severity: High]
Does this code leave iod->nr_descriptors greater than 0 on the error path?
If dma_pool_alloc() fails in nvme_rq_setup_dmabuf_map(), it jumps to 'free_prps:', which calls nvme_free_descriptors() but leaves 'iod->nr_descriptors' unchanged. The block layer may requeue the failed request.
If the queue is subsequently torn down, the cancellation path invokes nvme_unmap_data() -> nvme_rq_clean_dmabuf_map(). Seeing the non-zero 'nr_descriptors', it calls nvme_free_descriptors() again, resulting in a double free.
Quoted 1 line
+}
[ ... ]
Quoted 25 lines
drivers/nvme/host/pci.c +25
-0+static struct dma_buf_io_map *nvme_dma_buf_io_map (struct dma_buf_io_ctx *ctx) +{ +unsigned nr_entries = ctx->dmabuf->size /NVME_CTRL_PAGE_SIZE ; +struct nvme_dma_buf_io_ctx *nvme_ctx = ctx->dev_priv; +struct dma_buf_attachment *attach = nvme_ctx->attach; +unsigned long tmp, i =0 ; +struct nvme_dmabuf_map *map; +struct scatterlist *sg; +struct sg_table *sgt; +int ret; + +dma_resv_assert_held (ctx->dmabuf->resv); + + map =kvmalloc_flex (*map, dma_list, nr_entries); +if (!map) +return ERR_PTR (-ENOMEM ); + + sgt =dma_buf_map_attachment (attach, ctx->dir); +if (IS_ERR (sgt)) { + ret =PTR_ERR (sgt); + sgt =NULL ; + goto err; + } + +for_each_sgtable_dma_sg (sgt, sg, tmp) {
drivers/nvme/host/pci.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) · line 2475
Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+dma_addr_t dma_addr =sg_dma_address (sg); +unsigned long sg_len =sg_dma_len (sg); + +if ((sg_len %NVME_CTRL_PAGE_SIZE ) || + (dma_addr %NVME_CTRL_PAGE_SIZE )) { + ret = -EINVAL ; + goto err; + } +while (sg_len) { + map->dma_list[i++] = dma_addr;
[Severity: High]
Does this loop overflow dma_list[]?
'nr_entries' is computed via integer division of dmabuf->size. If the dmabuf size is not page-aligned (e.g., 4097 bytes), nr_entries truncates down (to 1). However, exporters often pad sg_table allocations to full pages (e.g., 8192 bytes).
The loop unconditionally writes all sg_len chunks to 'dma_list', exceeding the allocated 'nr_entries' capacity.
Quoted 5 lines
drivers/nvme/host/pci.c +5
-0+ dma_addr +=NVME_CTRL_PAGE_SIZE ; + sg_len -=NVME_CTRL_PAGE_SIZE ; + } + } +
drivers/nvme/host/pci.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) · line 2490
Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+ ret =dma_buf_io_init_map (ctx, &map->base, sgt); +if (ret) + goto err; + map->nr_entries = nr_entries; + map->sgt = sgt; +return &map->base; +err: +if (sgt) +dma_buf_unmap_attachment (attach, sgt, ctx->dir); +kfree (map);
[Severity: High]
Is it safe to free a kvmalloc_flex() allocation with kfree()?
If the dmabuf setup fails, the code jumps to the 'err' label and executes kfree(map). Calling kfree() on memory allocated via vmalloc() causes undefined behavior, typically a kernel panic.
Quoted 2 lines
drivers/nvme/host/pci.c +2
-0+return ERR_PTR (ret); +}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
Re: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests
On 9/28/26 15:02, [email protected] wrote:
...>> + nvme_dmabuf_map_sync_for_device(nvmeq->dev, req);
Quoted 12 lines
drivers/nvme/host/pci.c static void nvme_free_descriptors(struct request *req) · line 945
Quoted 8 lines
drivers/nvme/host/pci.c +8
-0+ +if (length <=0 ) { + prp2_dma =0 ; + goto done; + } + +if (length <=NVME_CTRL_PAGE_SIZE ) { + prp2_dma = dma_list[map_idx];[Severity: Critical]
Ismap_idxchecked againstmap->nr_entrieshere?
Quoted 1 line
If
dmabuf->sizeis 4608 bytes,nr_entriesis truncated to 1, allocating 1
It'd be rejected, not truncated, see below.
Quoted 10 lines
array element. Userspace could submit a 4608-byte IO. The driver calculates 'length' up to 4608 and advances '
map_idx' to 1 for the remaining 512 bytes. It reads 'dma_list[1]' (which is out-of-bounds) and uses it as 'prp2_dma', potentially resulting in arbitrary physical DMA (read or write).Quoted 2 lines
(quoted diff) +2
-0+ goto done; + }[ ... ]
Quoted 8 lines
drivers/nvme/host/pci.c static void nvme_free_descriptors(struct request *req) · line 990
Quoted 5 lines
drivers/nvme/host/pci.c +5
-0+free_prps: + iod->cmd.common.dptr.prp1 =cpu_to_le64 (prp1_dma); + iod->cmd.common.dptr.prp2 =cpu_to_le64 (prp2_dma); +nvme_free_descriptors (req); +return BLK_STS_RESOURCE ;[Severity: High]
Does this code leaveiod->nr_descriptorsgreater than 0 on the error path?
I can clear it, but unless I missed sth that's what e.g. nvme_pci_setup_data_prp() already does.
nvme_pci_setup_data_prp() // goto bad_sgl:
nvme_unmap_data() nvme_free_descriptors();
Quoted 53 lines
If
dma_pool_alloc()fails innvme_rq_setup_dmabuf_map(), it jumps to 'free_prps:', which callsnvme_free_descriptors()but leaves 'iod->nr_descriptors' unchanged. The block layer may requeue the failed request.If the queue is subsequently torn down, the cancellation path invokes
nvme_unmap_data()->nvme_rq_clean_dmabuf_map(). Seeing the non-zero 'nr_descriptors', it callsnvme_free_descriptors()again, resulting in a double free.Quoted 1 line
+}
[ ... ]
Quoted 25 lines
drivers/nvme/host/pci.c +25
-0+static struct dma_buf_io_map *nvme_dma_buf_io_map (struct dma_buf_io_ctx *ctx) +{ +unsigned nr_entries = ctx->dmabuf->size /NVME_CTRL_PAGE_SIZE ; +struct nvme_dma_buf_io_ctx *nvme_ctx = ctx->dev_priv; +struct dma_buf_attachment *attach = nvme_ctx->attach; +unsigned long tmp, i =0 ; +struct nvme_dmabuf_map *map; +struct scatterlist *sg; +struct sg_table *sgt; +int ret; + +dma_resv_assert_held (ctx->dmabuf->resv); + + map =kvmalloc_flex (*map, dma_list, nr_entries); +if (!map) +return ERR_PTR (-ENOMEM ); + + sgt =dma_buf_map_attachment (attach, ctx->dir); +if (IS_ERR (sgt)) { + ret =PTR_ERR (sgt); + sgt =NULL ; + goto err; + } + +for_each_sgtable_dma_sg (sgt, sg, tmp) {drivers/nvme/host/pci.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) · line 2475
Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+dma_addr_t dma_addr =sg_dma_address (sg); +unsigned long sg_len =sg_dma_len (sg); + +if ((sg_len %NVME_CTRL_PAGE_SIZE ) || + (dma_addr %NVME_CTRL_PAGE_SIZE )) { + ret = -EINVAL ; + goto err; + } +while (sg_len) { + map->dma_list[i++] = dma_addr;[Severity: High]
Does this loop overflow dma_list[]?
Quoted 2 lines
'
nr_entries' is computed via integer division ofdmabuf->size. If the dmabuf size is not page-aligned (e.g., 4097 bytes),nr_entriestruncates down (to 1).
It'll fail on the alignment check in this loop for [4096, 4097)
Quoted 16 lines
However, exporters often pad
sg_tableallocations to full pages (e.g., 8192 bytes).The loop unconditionally writes all
sg_lenchunks to 'dma_list', exceeding the allocated 'nr_entries' capacity.Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+ dma_addr +=NVME_CTRL_PAGE_SIZE ; + sg_len -=NVME_CTRL_PAGE_SIZE ; + } + } + + ret =dma_buf_io_init_map (ctx, &map->base, sgt); +if (ret) + goto err; + map->nr_entries = nr_entries; + map->sgt = sgt;
Quoted 8 lines
drivers/nvme/host/pci.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) · line 2490
Quoted 5 lines
drivers/nvme/host/pci.c +5
-0+return &map->base; +err: +if (sgt) +dma_buf_unmap_attachment (attach, sgt, ctx->dir); +kfree (map);[Severity: High]
Is it safe to free akvmalloc_flex()allocation withkfree()?
Going to fix.
Signature · 1 line
Pavel Begunkov
Re: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests
In reply to Pavel BegunkovLe 28/09/2026 à 15:32, Pavel Begunkov a écrit :
Quoted 7 lines
Enable
BIO_DMABUF_MAPbacked requests. On registration we map the dma-buf and store it as a prp list, which is then used to initialise requests. All attached contexts are stored in a new listdmabuf_ctxs, and additions/removals are synchronised withdmabuf_lock.Suggested-by Keith Busch <[email protected]>Signed-off-by Pavel Begunkov <[email protected]>
Hi,
a few nitpick below, should it help.
Quoted 6 lines
--- drivers/nvme/host/core.c | 12 ++ drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/pci.c | 308 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 322 insertions(+)
[...]
drivers/nvme/host/pci.c line 31
Quoted 10 lines
drivers/nvme/host/pci.c +2
-0@@ -27,6 +27,8 @@#include <linux/io-64-nonatomic-lo-hi.h> #include <linux/io-64-nonatomic-hi-lo.h> #include <linux/sed-opal.h> +#include <linux/dma-buf-io.h> +#include <linux/dma-resv.h>
Move up, to keep better alphabetical order ?
Quoted 13 lines
drivers/nvme/host/pci.c +2
-0#include "trace.h" #include "nvme.h" @@ -318,6 +320,8 @@ struct nvme_dev {bool hmb;struct sg_table *hmb_sgt;mempool_t *dmavec_mempool; +struct list_head dmabuf_ctxs; +struct mutex dmabuf_lock;/* shadow doorbell buffer support: */ __le32 *dbbuf_dbs;
drivers/nvme/host/pci.c struct nvme_queue { · line 408
Quoted 8 lines
drivers/nvme/host/pci.c +5
-0@@ -397,6 +401,13 @@ struct nvme_queue {struct completion delete_done; }; +struct nvme_dmabuf_map { +struct dma_buf_io_map base; +struct sg_table *sgt; +unsigned nr_entries; +dma_addr_t dma_list[];
Add __counted_by(nr_entries) and update nvme_dma_buf_io_map() so that nr_entries is set at the right time ?
Quoted 5 lines
drivers/nvme/host/pci.c +2
-0+}; +/* bits for iod->flags */ enum nvme_iod_flags {/* this command has been aborted by the timeout handler */
...
Quoted 40 lines
drivers/nvme/host/pci.c +40
-0+static struct dma_buf_io_map *nvme_dma_buf_io_map (struct dma_buf_io_ctx *ctx) +{ +unsigned nr_entries = ctx->dmabuf->size /NVME_CTRL_PAGE_SIZE ; +struct nvme_dma_buf_io_ctx *nvme_ctx = ctx->dev_priv; +struct dma_buf_attachment *attach = nvme_ctx->attach; +unsigned long tmp, i =0 ; +struct nvme_dmabuf_map *map; +struct scatterlist *sg; +struct sg_table *sgt; +int ret; + +dma_resv_assert_held (ctx->dmabuf->resv); + + map =kvmalloc_flex (*map, dma_list, nr_entries); +if (!map) +return ERR_PTR (-ENOMEM ); + + sgt =dma_buf_map_attachment (attach, ctx->dir); +if (IS_ERR (sgt)) { + ret =PTR_ERR (sgt); + sgt =NULL ; + goto err; + } + +for_each_sgtable_dma_sg (sgt, sg, tmp) { +dma_addr_t dma_addr =sg_dma_address (sg); +unsigned long sg_len =sg_dma_len (sg); + +if ((sg_len %NVME_CTRL_PAGE_SIZE ) || + (dma_addr %NVME_CTRL_PAGE_SIZE )) { + ret = -EINVAL ; + goto err; + } +while (sg_len) { + map->dma_list[i++] = dma_addr; + dma_addr +=NVME_CTRL_PAGE_SIZE ; + sg_len -=NVME_CTRL_PAGE_SIZE ; + } + } +
drivers/nvme/host/pci.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) · line 2490
Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+ ret =dma_buf_io_init_map (ctx, &map->base, sgt); +if (ret) + goto err; + map->nr_entries = nr_entries; + map->sgt = sgt; +return &map->base; +err: +if (sgt) +dma_buf_unmap_attachment (attach, sgt, ctx->dir); +kfree (map);
kvfree()?
drivers/nvme/host/pci.c
Quoted 2 lines
drivers/nvme/host/pci.c +2
-0+return ERR_PTR (ret); +}
...
CJ
Re: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests
On 9/29/26 16:45, Christophe JAILLET wrote:
...>> + for_each_sgtable_dma_sg(sgt, sg, tmp) {
Quoted 19 lines
Quoted 19 lines
+
dma_addr_tdma_addr=sg_dma_address(sg); + unsigned longsg_len=sg_dma_len(sg);drivers/nvme/host/pci.c +17
-0+ + if ((sg_len % NVME_CTRL_PAGE_SIZE) || + (dma_addr % NVME_CTRL_PAGE_SIZE)) { + ret = -EINVAL; + goto err; + } + while (sg_len) { + map->dma_list[i++] = dma_addr; + dma_addr += NVME_CTRL_PAGE_SIZE; + sg_len -= NVME_CTRL_PAGE_SIZE; + } + } + + ret = dma_buf_io_init_map(ctx, &map->base, sgt); + if (ret) + goto err; + map->nr_entries = nr_entries;
Quoted 8 lines
Quoted 6 lines
+
map->sgt= sgt;
+ return&map->base;
+err:
+ if (sgt)
+dma_buf_unmap_attachment(attach, sgt, ctx->dir); + kfree(map);
kvfree()?
Sashiko also flagged this one. I'm going to apply the nits from both emails, thanks.
Signature · 1 line
Pavel Begunkov
Christoph Hellwig
maintainerRe: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests
In reply to Christophe JAILLETQuoted 8 lines · Christophe JAILLET
drivers/nvme/host/pci.c line 31
Quoted 6 lines
drivers/nvme/host/pci.c +2
-0@@ -27,6 +27,8 @@#include <linux/io-64-nonatomic-lo-hi.h> #include <linux/io-64-nonatomic-hi-lo.h> #include <linux/sed-opal.h> +#include <linux/dma-buf-io.h> +#include <linux/dma-resv.h> Move up, to keep better alphabetical order ?
Doesn't really matter.
Quoted 8 lines
drivers/nvme/host/pci.c struct nvme_queue { · line 408
Quoted 5 lines
drivers/nvme/host/pci.c +4
-0+struct nvme_dmabuf_map { +struct dma_buf_io_map base; +struct sg_table *sgt; +unsigned nr_entries; +dma_addr_t dma_list[];Add
__counted_by(nr_entries)and updatenvme_dma_buf_io_map()so thatnr_entriesis set at the right time ?
Yeah, that would be useful.
Christoph Hellwig
maintainerRe: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests
In reply to Pavel Begunkovdrivers/nvme/host/pci.c static void nvme_free_descriptors(struct request *req) · line 894
Quoted 10 lines · Pavel Begunkov
drivers/nvme/host/pci.c +10
-0+static void nvme_dmabuf_map_sync_for_cpu (struct nvme_dev *nvme_dev, +struct request *req) +{ +struct device *dev = nvme_dev->dev; +enum dma_data_direction dma_dir; +struct bio *bio = req->bio; +struct nvme_dmabuf_map *map =to_nvme_dmabuf_map (bio->bi_dmabuf_map); + + dma_dir =rq_data_dir (req) ==READ ?DMA_FROM_DEVICE :DMA_TO_DEVICE ; +dma_sync_sgtable_for_cpu (dev, map->sgt, dma_dir);
This can use rq_dma_dir in the argument. Same for the other DMA API calls.
Btw, given how larger the dmabuf code is now, have you considered splitting it into a new nvme-pci-dmabuf.c file?
Re: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests
Quoted 14 lines · Christoph Hellwig
drivers/nvme/host/pci.c static void nvme_free_descriptors(struct request *req) · line 894
Quoted 10 lines · Pavel Begunkov
drivers/nvme/host/pci.c +10
-0+static void nvme_dmabuf_map_sync_for_cpu (struct nvme_dev *nvme_dev, +struct request *req) +{ +struct device *dev = nvme_dev->dev; +enum dma_data_direction dma_dir; +struct bio *bio = req->bio; +struct nvme_dmabuf_map *map =to_nvme_dmabuf_map (bio->bi_dmabuf_map); + + dma_dir =rq_data_dir (req) ==READ ?DMA_FROM_DEVICE :DMA_TO_DEVICE ; +dma_sync_sgtable_for_cpu (dev, map->sgt, dma_dir);This can use
rq_dma_dirin the argument. Same for the other DMA API calls.
Will change
Quoted 2 lines
Btw, given how larger the dmabuf code is now, have you considered splitting it into a new
nvme-pci-dmabuf.cfile?
Let me see
Signature · 1 line
Pavel Begunkov
PATCH v7 7/13 nvme-pci: rename nvme_pci_sgl_set_data to nvme_pci_dma_iter_set_sgl
In reply to Pavel BegunkovFrom: Anuj Gupta <[email protected]>
Rename the blk_dma_iter based nvme_pci_sgl_set_data() to nvme_pci_dma_iter_set_sgl().
--- drivers/nvme/host/pci.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/nvme/host/pci.c +5 -5
@@ -1277,7 +1277,7 @@ static blk_status_t nvme_pci_setup_data_prp(struct request *req,return BLK_STS_IOERR ; } -static void nvme_pci_sgl_set_data (struct nvme_sgl_desc *sge, +static void nvme_pci_dma_iter_set_sgl (struct nvme_sgl_desc *sge,struct blk_dma_iter *iter) { sge->addr =cpu_to_le64 (iter->addr);
@@ -1307,7 +1307,7 @@ static blk_status_t nvme_pci_setup_data_sgl(struct request *req, iod->cmd.common.flags =NVME_CMD_SGL_METABUF ;if (entries ==1 ||blk_rq_dma_map_coalesce (&iod->dma_state)) { -nvme_pci_sgl_set_data (&iod->cmd.common.dptr.sgl, iter); +nvme_pci_dma_iter_set_sgl (&iod->cmd.common.dptr.sgl, iter); iod->total_len += iter->len;return BLK_STS_OK ; }
@@ -1329,7 +1329,7 @@ static blk_status_t nvme_pci_setup_data_sgl(struct request *req, iter->status =BLK_STS_IOERR ;break ; } -nvme_pci_sgl_set_data (&sg_list[mapped++], iter); +nvme_pci_dma_iter_set_sgl (&sg_list[mapped++], iter); iod->total_len += iter->len; }while (blk_rq_dma_map_iter_next (req, nvmeq->dev->dev, iter));
@@ -1492,13 +1492,13 @@ static blk_status_t nvme_pci_setup_meta_iter(struct request *req) iod->cmd.common.metadata =cpu_to_le64 (sgl_dma);if (entries ==1 ) { iod->meta_total_len = iter.len; -nvme_pci_sgl_set_data (sg_list, &iter); +nvme_pci_dma_iter_set_sgl (sg_list, &iter);return BLK_STS_OK ; } sgl_dma +=sizeof (*sg_list);do { -nvme_pci_sgl_set_data (&sg_list[++i], &iter); +nvme_pci_dma_iter_set_sgl (&sg_list[++i], &iter); iod->meta_total_len += iter.len; }while (blk_rq_integrity_dma_map_iter_next (req, dev->dev, &iter));
Signature · 1 line
2.54.0
PATCH v7 8/13 nvme-pci: add SGL support for the dmabuf path
In reply to Pavel BegunkovFrom: Anuj Gupta <[email protected]>
Add SGL support in addition to PRP for dmabuf-backed requests, building the descriptor list from the mapping's sg_table.
--- drivers/nvme/host/pci.c | 153 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 149 insertions(+), 4 deletions(-)
drivers/nvme/host/pci.c +149 -4
@@ -1277,12 +1277,18 @@ static blk_status_t nvme_pci_setup_data_prp(struct request *req,return BLK_STS_IOERR ; } +static void nvme_pci_sgl_set_data (struct nvme_sgl_desc *sge, +dma_addr_t addr,u32 len) +{ + sge->addr =cpu_to_le64 (addr); + sge->length =cpu_to_le32 (len); + sge->type =NVME_SGL_FMT_DATA_DESC <<4 ; +} +static void nvme_pci_dma_iter_set_sgl (struct nvme_sgl_desc *sge,struct blk_dma_iter *iter) { - sge->addr =cpu_to_le64 (iter->addr); - sge->length =cpu_to_le32 (iter->len); - sge->type =NVME_SGL_FMT_DATA_DESC <<4 ; +nvme_pci_sgl_set_data (sge, iter->addr, iter->len); } static void nvme_pci_sgl_set_seg(struct nvme_sgl_desc *sge,
@@ -1293,6 +1299,145 @@ static void nvme_pci_sgl_set_seg(struct nvme_sgl_desc *sge, sge->type =NVME_SGL_FMT_LAST_SEG_DESC <<4 ; } +static unsigned int nvme_pci_dmabuf_sgl_nents (struct request *req) +{ +struct bio *bio = req->bio; +struct nvme_dmabuf_map *map =to_nvme_dmabuf_map (bio->bi_dmabuf_map); +struct scatterlist *sg; +unsigned long tmp; +size_t offset = bio->bi_iter.bi_offset; +size_t remaining =blk_rq_payload_bytes (req); +unsigned int nents =0 ; + +for_each_sgtable_dma_sg (map->sgt, sg, tmp) { +size_t sg_len =sg_dma_len (sg); + +if (!remaining) +break ; +if (offset >= sg_len) { + offset -= sg_len; +continue ; + } + + sg_len -= offset; + offset =0 ; + +do { +size_t chunk =min (remaining, sg_len); + + nents++; + sg_len -= chunk; + remaining -= chunk; + }while (sg_len && remaining); + } + +if (unlikely (remaining)) +return 0 ; + +return nents; +} + +static blk_status_t nvme_rq_setup_dmabuf_sgl (struct request *req, +struct nvme_queue *nvmeq,unsigned int entries) +{ +struct nvme_iod *iod =blk_mq_rq_to_pdu (req); +struct bio *bio = req->bio; +struct nvme_dmabuf_map *map =to_nvme_dmabuf_map (bio->bi_dmabuf_map); +size_t length =blk_rq_payload_bytes (req); +struct nvme_sgl_desc *sg_list = &iod->cmd.common.dptr.sgl; +bool pooled = entries >1 ; +dma_addr_t sgl_dma =0 ; +unsigned int mapped =0 ; +unsigned long tmp; +struct scatterlist *sg; +size_t offset = bio->bi_iter.bi_offset; +size_t remaining = length; + +if (!entries) +return BLK_STS_IOERR ; + + iod->cmd.common.flags =NVME_CMD_SGL_METABUF ; + iod->total_len = length; + +nvme_dmabuf_map_sync_for_device (nvmeq->dev, req); + +/* entries == 1 fits in the inline descriptor; more needs a pool. */ +if (pooled) { +if (entries <=NVME_SMALL_POOL_SIZE /sizeof (*sg_list)) + iod->flags |=IOD_SMALL_DESCRIPTOR ; + + sg_list =dma_pool_alloc (nvme_dma_pool (nvmeq, iod),GFP_ATOMIC , + &sgl_dma); +if (!sg_list) +return BLK_STS_RESOURCE ; + iod->descriptors[iod->nr_descriptors++] = sg_list; + } + +for_each_sgtable_dma_sg (map->sgt, sg, tmp) { +size_t sg_len =sg_dma_len (sg); +dma_addr_t addr =sg_dma_address (sg); + +if (!remaining) +break ; +if (offset >= sg_len) { + offset -= sg_len; +continue ; + } + + addr += offset; + sg_len -= offset; + offset =0 ; + +do { +u32 chunk =min_t (size_t, remaining, sg_len); + +if (WARN_ON_ONCE (mapped == entries)) + goto err_free; +nvme_pci_sgl_set_data (&sg_list[mapped++], addr, chunk); + + addr += chunk; + sg_len -= chunk; + remaining -= chunk; + }while (sg_len && remaining); + } + +if (unlikely (remaining)) + goto err_free; + +if (pooled) +nvme_pci_sgl_set_seg (&iod->cmd.common.dptr.sgl, sgl_dma, + mapped); +return BLK_STS_OK ; +err_free: +if (pooled) { + iod->nr_descriptors--; +dma_pool_free (nvme_dma_pool (nvmeq, iod), sg_list, sgl_dma); + } +return BLK_STS_IOERR ; +} + +static blk_status_t nvme_rq_setup_dmabuf (struct request *req, +struct nvme_queue *nvmeq,enum nvme_use_sgl use_sgl) +{ +unsigned int entries; +size_t avg_seg; + +if (use_sgl ==SGL_UNSUPPORTED ) +return nvme_rq_setup_dmabuf_map (req, nvmeq); + + entries =nvme_pci_dmabuf_sgl_nents (req); + +if (use_sgl ==SGL_FORCED ) +return nvme_rq_setup_dmabuf_sgl (req, nvmeq, entries); + + avg_seg = entries ? +DIV_ROUND_UP (blk_rq_payload_bytes (req), entries) :0 ; +if (sgl_threshold && avg_seg >= sgl_threshold) +return nvme_rq_setup_dmabuf_sgl (req, nvmeq, entries); + +return nvme_rq_setup_dmabuf_map (req, nvmeq); +} +static blk_status_t nvme_pci_setup_data_sgl (struct request *req,struct blk_dma_iter *iter) {
@@ -1388,7 +1533,7 @@ static blk_status_t nvme_map_data(struct request *req)blk_status_t ret;if (blk_mq_rq_is_dmabuf (req)) -return nvme_rq_setup_dmabuf_map (req, nvmeq); +return nvme_rq_setup_dmabuf (req, nvmeq, use_sgl);/* * Try to skip the DMA iterator for single segment requests, as that
Signature · 1 line
2.54.0
PATCH v7 9/13 io_uring/rsrc: introduce buf registration structure
In reply to Pavel BegunkovIn preparation to following changes, instead of passing an iovec for buffer registration introduce a new structure. It'll be moved to uapi later, but for now it's initialised early from a user provided iovec.
--- io_uring/rsrc.c | 47 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-)
io_uring/rsrc.c +32 -15
@@ -27,8 +27,13 @@ struct io_rsrc_update {u32 offset; }; +struct io_uring_regbuf_desc { +__u64 uaddr; +__u64 size; +}; +static struct io_rsrc_node *io_sqe_buffer_register (struct io_ring_ctx *ctx, -struct iovec *iov); +struct io_uring_regbuf_desc *desc);static int hpage_acct_ref (struct io_ring_ctx *ctx,struct page *hpage,bool *acct_new)
@@ -81,6 +86,15 @@ static bool hpage_acct_unref(struct io_ring_ctx *ctx, struct page *hpage)#define IO_CACHED_BVECS_SEGS 32 +static void io_iov_to_regbuf_desc (const struct iovec *iov, +struct io_uring_regbuf_desc *desc) +{ + *desc = (struct io_uring_regbuf_desc ) { + .uaddr = (u64 )(uintptr_t )iov->iov_base, + .size = iov->iov_len, + }; +} +int __io_account_mem (struct user_struct *user,unsigned long nr_pages) {unsigned long page_limit, cur_pages, new_pages;
@@ -381,6 +395,7 @@ static int __io_sqe_buffers_update(struct io_ring_ctx *ctx,return -EINVAL ;for (done =0 ; done < nr_args; done++) { +struct io_uring_regbuf_desc desc;struct io_rsrc_node *node;u64 tag =0 ;
@@ -394,7 +409,9 @@ static int __io_sqe_buffers_update(struct io_ring_ctx *ctx, err = -EFAULT ;break ; } - node =io_sqe_buffer_register (ctx, iov); + +io_iov_to_regbuf_desc (iov, &desc); + node =io_sqe_buffer_register (ctx, &desc);if (IS_ERR (node)) { err =PTR_ERR (node);break ;
@@ -853,26 +870,26 @@ bool io_check_coalesce_buffer(struct page **page_array, int nr_pages, }static struct io_rsrc_node *io_sqe_buffer_register (struct io_ring_ctx *ctx, -struct iovec *iov) +struct io_uring_regbuf_desc *desc) { +unsigned long uaddr = (unsigned long )desc->uaddr; +size_t size = desc->size;struct io_mapped_ubuf *imu =NULL ;struct page **pages =NULL ;struct io_rsrc_node *node;unsigned long off; -size_t size;int ret, nr_pages, i;struct io_imu_folio_data data;bool coalesced = false; -if (!iov->iov_base) { -if (iov->iov_len) +if (!uaddr) { +if (size)return ERR_PTR (-EFAULT );/* remove the buffer without installing a new one */ return NULL ; } - ret =io_validate_user_buf_range ((unsigned long )iov->iov_base, - iov->iov_len); + ret =io_validate_user_buf_range (uaddr, size);if (ret)return ERR_PTR (ret);
@@ -881,8 +898,7 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx,return ERR_PTR (-ENOMEM ); ret = -ENOMEM ; - pages =io_pin_pages ((unsigned long ) iov->iov_base, iov->iov_len, - &nr_pages); + pages =io_pin_pages (uaddr, size, &nr_pages);if (IS_ERR (pages)) { ret =PTR_ERR (pages); pages =NULL ;
@@ -904,10 +920,9 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx,if (ret) goto done; - size = iov->iov_len;/* store original address for later verification */ - imu->ubuf = (unsigned long ) iov->iov_base; - imu->len = iov->iov_len; + imu->ubuf = uaddr; + imu->len = size; imu->folio_shift =PAGE_SHIFT ; imu->release = io_release_ubuf; imu->priv = imu;
@@ -917,7 +932,7 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx, imu->folio_shift = data.folio_shift;refcount_set (&imu->refs,1 ); - off = (unsigned long )iov->iov_base & ~PAGE_MASK ; + off = uaddr & ~PAGE_MASK ;if (coalesced) off += data.first_folio_page_idx <<PAGE_SHIFT ;
@@ -969,6 +984,7 @@ int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,memset (iov,0 ,sizeof (*iov));for (i =0 ; i < nr_args; i++) { +struct io_uring_regbuf_desc desc;struct io_rsrc_node *node;u64 tag =0 ;
@@ -992,7 +1008,8 @@ int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg, } } - node =io_sqe_buffer_register (ctx, iov); +io_iov_to_regbuf_desc (iov, &desc); + node =io_sqe_buffer_register (ctx, &desc);if (IS_ERR (node)) { ret =PTR_ERR (node);break ;
Signature · 1 line
2.54.0
PATCH v7 10/13 io_uring/rsrc: extend buffer update
In reply to Pavel BegunkovWe need to pass more information to buffer registration than we can fit into a single struct iovec. This patch allows users to optionally pass struct io_uring_regbuf_desc. Apart from having more space for future use cases, it also introduces registration types.
Currently, the type can be either of IO_REGBUF_TYPE_UADDR, which mirrors the iovec path, or IO_REGBUF_TYPE_EMPTY for leaving a buffer table slot empty. The next patch introduces a dmabuf backed type, and can be useful for other extensions like splicing a list of user addresses (i.e. iovec[]), interoperability with zcrx, kernel allocated memory like was brough up by Cristoph. Note, the type only represents a registration option, which is distinct from how io_uring internally stores it.
The flags field is not used yet but always useful to have, e.g. we can encode read-only / write-only restrictions using it.
--- include/uapi/linux/io_uring.h | 27 +++++++++++++- io_uring/rsrc.c | 69 ++++++++++++++++++++++------------- 2 files changed, 69 insertions(+), 27 deletions(-)
include/uapi/linux/io_uring.h +26 -1
@@ -790,13 +790,38 @@ struct io_uring_rsrc_update {struct io_uring_rsrc_update2 {__u32 offset; -__u32 resv; +__u32 flags;__aligned_u64 data;__aligned_u64 tags;__u32 nr;__u32 resv2; }; +/* struct io_uring_rsrc_update2::flags */ +enum io_uring_rsrc_reg_flags { +/* +* Use the extended descriptor format for buffer updates, +* see struct io_uring_regbuf_desc +*/ +IORING_RSRC_UPDATE_EXTENDED =1U <<1 , +}; + +/* Buffer registration type, passed in struct io_uring_regbuf_desc::type */ +enum io_uring_regbuf_type { +IO_REGBUF_TYPE_EMPTY , +IO_REGBUF_TYPE_UADDR , + + __IO_REGBUF_TYPE_MAX, +}; + +struct io_uring_regbuf_desc { +__u32 type;/* enum io_uring_regbuf_type */ +__u32 flags; +__u64 size; +__u64 uaddr; +__u64 __resv[7 ]; +}; +/* Skip updating fd indexes set to this value in the fd table */ #define IORING_REGISTER_FILES_SKIP (-2)
io_uring/rsrc.c +43 -26
151 lines not on this page · raw message
Signature · 1 line
2.54.0
PATCH v7 11/13 io_uring/rsrc: add uncloneable regbuf flag
In reply to Pavel BegunkovIt's hard to implement cloning if the internal structure needs to be mutable and/or relies on other ring resources. In preparation to such buffer types, add a flag indicating that the buffer can't be cloned. It might be possible to add cloning in the future for them, but that would likely need reallocating the structure and reacquiring resources in case by case manner.
--- io_uring/rsrc.c | 5 +++++ io_uring/rsrc.h | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-)
io_uring/rsrc.c +5 -0
@@ -1434,6 +1434,11 @@ static int io_clone_buffers(struct io_ring_ctx *ctx, struct io_ring_ctx *src_ctxif (!src_node) { dst_node =NULL ; }else { +if (src_node->buf->flags &IO_REGBUF_F_UNCLONEABLE ) { +io_rsrc_data_free (ctx, &data); +return -EOPNOTSUPP ; + } + dst_node =io_rsrc_node_alloc (ctx,IORING_RSRC_BUFFER );if (!dst_node) {io_rsrc_data_free (ctx, &data);
io_uring/rsrc.h +2 -1
@@ -26,7 +26,8 @@ struct io_rsrc_node { };enum { -IO_REGBUF_F_KBUF =1 , +IO_REGBUF_F_KBUF =1 <<0 , +IO_REGBUF_F_UNCLONEABLE =1 <<1 , };struct io_mapped_ubuf {
Signature · 1 line
2.54.0
PATCH v7 12/13 io_uring/rsrc: add regbuf import flags
In reply to Pavel BegunkovWe'll have special registered buffer types that can't be used with all opcodes and need special handling. Add separate flags to control registered buffer import, which will be used to specify what kind of buffers the caller can handle.
--- io_uring/rsrc.c | 8 ++++---- io_uring/rsrc.h | 24 ++++++++++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-)
io_uring/rsrc.c +4 -4
@@ -1298,9 +1298,9 @@ inline struct io_rsrc_node *io_find_buf_node(struct io_kiocb *req,return NULL ; } -int io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter, +int __io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter,u64 buf_addr,size_t len,int ddir, -unsigned issue_flags) +unsigned issue_flags,unsigned import_flags) {struct io_rsrc_node *node;
@@ -1709,9 +1709,9 @@ static int io_kern_bvec_size(struct iovec *iov, unsigned nr_iovs,return 0 ; } -int io_import_reg_vec (int ddir,struct iov_iter *iter, +int __io_import_reg_vec (int ddir,struct iov_iter *iter,struct io_kiocb *req,struct iou_vec *vec, -unsigned nr_iovs,unsigned issue_flags) +unsigned nr_iovs,unsigned issue_flags,unsigned import_flags) {struct io_rsrc_node *node;struct io_mapped_ubuf *imu;
io_uring/rsrc.h +20 -4
@@ -62,12 +62,28 @@ int io_rsrc_data_alloc(struct io_rsrc_data *data, unsigned nr);struct io_rsrc_node *io_find_buf_node (struct io_kiocb *req,unsigned issue_flags); -int io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter, +int __io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter,u64 buf_addr,size_t len,int ddir, -unsigned issue_flags); -int io_import_reg_vec (int ddir,struct iov_iter *iter, +unsigned issue_flags,unsigned import_flags); +int __io_import_reg_vec (int ddir,struct iov_iter *iter,struct io_kiocb *req,struct iou_vec *vec, -unsigned nr_iovs,unsigned issue_flags); +unsigned nr_iovs,unsigned issue_flags, +unsigned import_flags); + +static inline int io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter, +u64 buf_addr,size_t len,int ddir, +unsigned issue_flags) +{ +return __io_import_reg_buf (req, iter, buf_addr, len, ddir, issue_flags,0 ); +} + +static inline int io_import_reg_vec (int ddir,struct iov_iter *iter, +struct io_kiocb *req,struct iou_vec *vec, +unsigned nr_iovs,unsigned issue_flags) +{ +return __io_import_reg_vec (ddir, iter, req, vec, nr_iovs, issue_flags,0 ); +} +int io_prep_reg_iovec (struct io_kiocb *req,struct iou_vec *iv,const struct iovec __user *uvec,size_t uvec_segs);
Signature · 1 line
2.54.0
PATCH v7 13/13 io_uring/rsrc: add dmabuf backed registered buffers
In reply to Pavel BegunkovImplement dmabuf backed registered buffers. To register them, the user should specify IO_REGBUF_TYPE_DMABUF for the regitration and pass the desired dmabuf fd and a file for which it should be registered.
From there, it can be used with io_uring read/write requests IORING_OP_{READ,WRITE}_FIXED) as normal. The requests should be issued against the file specified during registration, and otherwise they'll be failed. The user should also be prepared to handle spurious -EAGAIN by reissuing the request.
Internally, dmabuf registered buffers is an optin feature for io_uring request opcodes and they should pass a special flag on import to use it.
--- include/linux/io_uring_types.h | 5 + include/uapi/linux/io_uring.h | 6 +- io_uring/io_uring.c | 3 +- io_uring/rsrc.c | 162 ++++++++++++++++++++++++++++++++- io_uring/rsrc.h | 18 ++++ io_uring/rw.c | 6 +- 6 files changed, 192 insertions(+), 8 deletions(-)
include/linux/io_uring_types.h +5 -0
@@ -11,6 +11,7 @@struct iou_loop_params ;struct io_uring_bpf_ops ; +struct dma_buf_io_map ;enum {/*
@@ -610,6 +611,7 @@ enum {REQ_F_IMPORT_BUFFER_BIT ,REQ_F_SQE_COPIED_BIT ,REQ_F_IOPOLL_BIT , +REQ_F_DROP_DMABUF_BIT ,/* not a real bit, just to check we're not overflowing the space */ __REQ_F_LAST_BIT,
@@ -705,6 +707,8 @@ enum {REQ_F_SQE_COPIED =IO_REQ_FLAG (REQ_F_SQE_COPIED_BIT ),/* request must be iopolled to completion (set in ->issue()) */ REQ_F_IOPOLL =IO_REQ_FLAG (REQ_F_IOPOLL_BIT ), +/* there is a dma map attached to request that needs to be dropped */ + REQ_F_DROP_DMABUF = IO_REQ_FLAG(REQ_F_DROP_DMABUF_BIT), }; struct io_tw_req {
@@ -827,6 +831,7 @@ struct io_kiocb {/* custom credentials, valid IFF REQ_F_CREDS is set */ const struct cred *creds;struct io_wq_work work; +struct dma_buf_io_map *dmabuf_map;struct io_big_cqe {u64 extra1;
include/uapi/linux/io_uring.h +5 -1
@@ -810,6 +810,7 @@ enum io_uring_rsrc_reg_flags {enum io_uring_regbuf_type {IO_REGBUF_TYPE_EMPTY ,IO_REGBUF_TYPE_UADDR , +IO_REGBUF_TYPE_DMABUF , __IO_REGBUF_TYPE_MAX, };
@@ -819,7 +820,10 @@ struct io_uring_regbuf_desc {__u32 flags;__u64 size;__u64 uaddr; -__u64 __resv[7 ]; + +__s32 dmabuf_fd; +__s32 target_fd; +__u64 __resv[6 ]; };/* Skip updating fd indexes set to this value in the fd table */
io_uring/io_uring.c +2 -1
@@ -109,7 +109,7 @@#define IO_REQ_CLEAN_SLOW_FLAGS (REQ_F_REFCOUNT | IO_REQ_LINK_FLAGS | \ REQ_F_REISSUE | REQ_F_POLLED | \ - IO_REQ_CLEAN_FLAGS) + IO_REQ_CLEAN_FLAGS | REQ_F_DROP_DMABUF)#define IO_TCTX_REFS_CACHE_NR (1U << 10)
@@ -1134,6 +1134,7 @@ static void io_free_batch_list(struct io_ring_ctx *ctx,io_queue_next (req);if (unlikely (req->flags &IO_REQ_CLEAN_FLAGS ))io_clean_op (req); +io_req_drop_dmabuf (req); }io_put_file (req);io_req_put_rsrc_nodes (req);
io_uring/rsrc.c +158 -4
205 lines not on this page · raw message
io_uring/rsrc.h +18 -0
@@ -28,6 +28,11 @@ struct io_rsrc_node {enum {IO_REGBUF_F_KBUF =1 <<0 ,IO_REGBUF_F_UNCLONEABLE =1 <<1 , +IO_REGBUF_F_DMABUF =1 <<3 , +}; + +enum { +IO_REGBUF_IMPORT_ALLOW_DMABUF =1 <<1 , };struct io_mapped_ubuf {
@@ -170,4 +175,17 @@ static inline void io_alloc_cache_vec_kasan(struct iou_vec *iv) io_vec_free(iv); } +void io_drop_dmabuf_node (struct io_kiocb *req); + +static inline void io_req_drop_dmabuf (struct io_kiocb *req) +{ +if (!IS_ENABLED (CONFIG_DMA_SHARED_BUFFER )) +return ; +if (!(req->flags &REQ_F_DROP_DMABUF )) +return ; +if (WARN_ON_ONCE (!(req->flags &REQ_F_BUF_NODE ))) +return ; +io_drop_dmabuf_node (req); +} +#endif
io_uring/rw.c +4 -2
@@ -367,8 +367,8 @@ static int io_init_rw_fixed(struct io_kiocb *req, unsigned int issue_flags,if (io->bytes_done)return 0 ; - ret =io_import_reg_buf (req, &io->iter, rw->addr, rw->len, ddir, - issue_flags); + ret =__io_import_reg_buf (req, &io->iter, rw->addr, rw->len, ddir, + issue_flags,IO_REGBUF_IMPORT_ALLOW_DMABUF );iov_iter_save_state (&io->iter, &io->iter_state);return ret; }
@@ -583,6 +583,8 @@ static void io_complete_rw(struct kiocb *kiocb, long res) struct io_rw *rw = container_of(kiocb, struct io_rw, kiocb);struct io_kiocb *req =cmd_to_io_kiocb (rw); +io_req_drop_dmabuf (req); +/* ring owner may block in freeze_super() before task_work runs */ if (kiocb->ki_flags &IOCB_WRITE )io_req_end_write (req);
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v7 13/13] io_uring/rsrc: add dmabuf backed registered buffers
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] io_uring: UAF of dmabuf map on async I/O reissue with partial progress - [High] io_uring: UAF of…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] io_uring: UAF of dmabuf map on async I/O reissue with partial progress
- [High] io_uring: UAF of driver module due to fput() race in io_release_reg_dmabuf() --
commit 72563100c98fb5b83c9261811926c28b98fe4b8b
Author: Pavel Begunkov <[email protected]>
io_uring/rsrc: add dmabuf backed registered buffers
This commit introduces support for dmabuf-backed registered buffers, allowing users to register a dmabuf file descriptor and a target file. Direct I/O requests can then be issued against the target file using standard io_uring fixed read/write opcodes.
Quoted 4 lines
io_uring/rsrc.c +0
-0
[ ... ]
io_uring/rsrc.c bool io_check_coalesce_buffer(struct page **page_array, int nr_pages, · line 895
Quoted 6 lines
io_uring/rsrc.c +6
-0+static void io_release_reg_dmabuf (void *priv) +{ +struct io_regbuf_dma *db = priv; + +fput (db->target_file); +dma_buf_io_ctx_release (db->ctx);
[Severity: High]
Could this sequence create a race condition with module unloading?
If fput() is called during process exit (PF_EXITING), it might schedule delayed_fput_work on the system workqueue. A concurrent worker thread could execute __fput(), dropping the final module reference of target_file->f_op->owner.
If that happens, and the original thread then proceeds to call dma_buf_io_ctx_release(db->ctx) which invokes ctx->dev_ops->release(ctx), would this execute unmapped memory if the target module is unloaded in the interim?
Quoted 2 lines
io_uring/rsrc.c +2
-0+kfree (db); +}
[ ... ]
io_uring/rw.c static int io_init_rw_fixed(struct io_kiocb *req, unsigned int issue_flags, · line 368
Quoted 7 lines
io_uring/rw.c +0
-0@@ -367,8 +367,8 @@ static int io_init_rw_fixed(struct io_kiocb *req, unsigned int issue_flags,if (io->bytes_done)return 0 ;
[Severity: High]
Does this early return leave a dangling pointer in the iterator if the request is being reissued after partial progress?
If an async I/O request on a dmabuf-backed fixed buffer makes partial progress (io->bytes_done > 0) but then completes asynchronously with -EAGAIN, it appears io_complete_rw() unconditionally calls io_req_drop_dmabuf(), dropping the map reference.
When the request is caught and requeued to io-wq (because __io_complete_rw_common() marks it with REQ_F_REISSUE), returning early here bypasses io_import_dmabuf(). Would the iterator then retain a dangling pointer to the previously freed map, leading to a use-after-free when the block layer processes the remaining bytes?
Quoted 8 lines
io_uring/rw.c +2
-2- ret =io_import_reg_buf (req, &io->iter, rw->addr, rw->len, ddir, - issue_flags); + ret =__io_import_reg_buf (req, &io->iter, rw->addr, rw->len, ddir, + issue_flags,IO_REGBUF_IMPORT_ALLOW_DMABUF );iov_iter_save_state (&io->iter, &io->iter_state);return ret; }
[ ... ]
io_uring/rw.c static void io_complete_rw(struct kiocb *kiocb, long res) · line 586
Quoted 5 lines
io_uring/rw.c +1
-0@@ -583,6 +583,8 @@ static void io_complete_rw(struct kiocb *kiocb, long res) struct io_rw *rw = container_of(kiocb, struct io_rw, kiocb);struct io_kiocb *req =cmd_to_io_kiocb (rw); +io_req_drop_dmabuf (req);
[Severity: High]
Is it safe to unconditionally drop the dmabuf map here before checking if the request will be reissued?
If this request made partial progress but completes with -EAGAIN, __io_complete_rw_common() will see the result and set REQ_F_REISSUE. Since io_req_drop_dmabuf() has already dropped the map, would the reissued request in io-wq end up with a dangling map reference if io_init_rw_fixed() skips re-importing it?
Quoted 4 lines
io_uring/rw.c +1
-0+/* ring owner may block in freeze_super() before task_work runs */ if (kiocb->ki_flags &IOCB_WRITE )io_req_end_write (req);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13