Re: [PATCH v6 05/13] block: add dma-buf support for raw bdev

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] block: blkdev_read_iter() discards partial direct read success for dma-bufs
- [Low] block: dmabuf error paths skip iov_iter_reexpand() teardown
--

commit 1ef88ba1be9691ca9af0098196766a3034d434e9
Author: Pavel Begunkov <asml.silence@gmail.com>

    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.

> diff --git a/block/fops.c b/block/fops.c
> index 641621db700f8..15fe5e39c68aa 100644
> --- a/block/fops.c
> +++ b/block/fops.c

[ ... ]

> @@ -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;
                        ^^^^^^^^^^^^^^^^^^

[Severity: Low]
At the start of blkdev_write_iter(), iov_iter_truncate() restricts the I/O
length and tracks the truncated amount in shorted. By returning directly here,
does this bypass the iov_iter_reexpand() call at the end of the function,
leaving the iterator incorrectly truncated?

> +
>  		/*
>  		 * Take i_rwsem and invalidate_lock to avoid racing with
>  		 * set_blocksize changing i_blkbits/folio order and punching

[ ... ]

> @@ -851,6 +855,8 @@ 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)))
> +		return -EOPNOTSUPP;
                ^^^^^^^^^^^^^^^^^^

[Severity: Medium]
If a partial O_DIRECT read using a dma-buf iterator successfully reads data and
advances ki_pos in the preceding direct I/O path, could this check
unconditionally return -EOPNOTSUPP? Does this overwrite the successful byte
count (ret > 0) with an error and hide the read data from userspace?

[Severity: Low]
Similar to blkdev_write_iter(), if this returns directly, does it bypass the
goto reexpand path, leaving the iterator incorrectly truncated if
iov_iter_truncate() was called earlier in blkdev_read_iter()?

>  
>  	/*
>  	 * Take i_rwsem and invalidate_lock to avoid racing with set_blocksize

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789997898.git.asml.silence@gmail.com?part=5