linux-kernel - Re: [PATCH v4 08/19] ublk: move offset check out of __ublk_check_and_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f0c205d9-609f-4b08-af67-3d8730eb6fce@zazolabs.com>
Date: Mon, 12 Jan 2026 20:17:44 +0200
From: Alexander Atanasov <alex@...olabs.com>
To: Caleb Sander Mateos <csander@...estorage.com>,
 Ming Lei <ming.lei@...hat.com>, Jens Axboe <axboe@...nel.dk>,
 Shuah Khan <shuah@...nel.org>
Cc: linux-block@...r.kernel.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org, Stanley Zhang <stazhang@...estorage.com>,
 Uday Shankar <ushankar@...estorage.com>,
 "Martin K . Petersen" <martin.petersen@...cle.com>
Subject: Re: [PATCH v4 08/19] ublk: move offset check out of
 __ublk_check_and_get_req()

On 8.01.26 11:19, Caleb Sander Mateos wrote:
> __ublk_check_and_get_req() checks that the passed in offset is within
> the data length of the specified ublk request. However, only user copy
> (ublk_check_and_get_req()) supports accessing ublk request data at a
> nonzero offset. Zero-copy buffer registration (ublk_register_io_buf())
> always passes 0 for the offset, so the check is unnecessary. Move the
> check from __ublk_check_and_get_req() to ublk_check_and_get_req().
> 
> Signed-off-by: Caleb Sander Mateos <csander@...estorage.com>
> Reviewed-by: Ming Lei <ming.lei@...hat.com>
> ---
>   drivers/block/ublk_drv.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index e7697dc4a812..8eefb838b563 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -253,11 +253,11 @@ struct ublk_params_header {
>   

[snip]

> @@ -2603,13 +2603,10 @@ static inline struct request *__ublk_check_and_get_req(struct ublk_device *ub,
>   		goto fail_put;
>   
>   	if (!ublk_rq_has_data(req))
>   		goto fail_put;
>   
> -	if (offset > blk_rq_bytes(req))
> -		goto fail_put;
> -
>   	return req;
>   fail_put:
>   	ublk_put_req_ref(io, req);
>   	return NULL;
>   }
> @@ -2687,14 +2684,19 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
>   
>   	if (tag >= ub->dev_info.queue_depth)
>   		return -EINVAL;
>   
>   	io = &ubq->ios[tag];
> -	req = __ublk_check_and_get_req(ub, q_id, tag, io, buf_off);
> +	req = __ublk_check_and_get_req(ub, q_id, tag, io);
>   	if (!req)
>   		return -EINVAL;
>   
> +	if (buf_off > blk_rq_bytes(req)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +

Offset is zero based, bytes are count so it should be >= here.

It will work this way but for buf_off == blk_rq_bytes(req) user will get 
0 instead of EINVAL.

static size_t ublk_copy_user_pages(const struct request *req,
                 unsigned offset, struct iov_iter *uiter, int dir)
{
	size_t done = 0;
...
         rq_for_each_segment(bv, req, iter) {
...
                 if (offset >= bv.bv_len) {
                         offset -= bv.bv_len; // bv_len is same as 
blk_rq_bytes(req)
                         continue; // this breaks the loop when ==
                 }
...
	}
	return done; // done is never incremented
}

>   	if (!ublk_check_ubuf_dir(req, dir)) {
>   		ret = -EACCES;
>   		goto out;
>   	}


-- 
have fun,
alex