linux-kernel - Re: [PATCH v2 04/19] ublk: set request integrity params in ublksrv_io

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: <aVkkSxNNUBMz9E61@fedora>
Date: Sat, 3 Jan 2026 22:14:35 +0800
From: Ming Lei <ming.lei@...hat.com>
To: Caleb Sander Mateos <csander@...estorage.com>
Cc: Jens Axboe <axboe@...nel.dk>, Shuah Khan <shuah@...nel.org>,
	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>
Subject: Re: [PATCH v2 04/19] ublk: set request integrity params in
 ublksrv_io_desc

On Fri, Jan 02, 2026 at 05:45:14PM -0700, Caleb Sander Mateos wrote:
> Indicate to the ublk server when an incoming request has integrity data
> by setting UBLK_IO_F_INTEGRITY in the ublksrv_io_desc's op_flags field.
> If the ublk device doesn't support integrity, the request will never
> provide integrity data. If the ublk device supports integrity, the
> request may omit the integrity buffer only if metadata_size matches the
> PI tuple size determined by csum_type. In this case, the ublk server
> should internally generate/verify the protection information from the
> data and sector offset.
> Set the UBLK_IO_F_CHECK_{GUARD,REFTAG,APPTAG} flags based on the
> request's BIP_CHECK_{GUARD,REFTAG,APPTAG} flags, indicating whether to
> verify the guard, reference, and app tags in the protection information.
> The expected reference tag (32 or 48 bits) and app tag (16 bits) are
> indicated in ublksrv_io_desc's new struct ublksrv_io_integrity integrity
> field. This field is unioned with the addr field to avoid changing the

It might be fine to set per-rq app_tag, but bios in one request might have
different app_tag in case of io merge actually.

Also block layer builds ref_tag for each internal, please see
t10_pi_generate() and ext_pi_crc64_generate().

So looks this way is wrong.

More importantly reusing iod->addr for other purpose not related with IO
buffer is very unfriendly for adding new features, and one lesson is for ZONED support
by reusing ublksrv_io_cmd->addr for zoned's append lba.

For example, there is chance to support dma-buf based zero copy for ublk, and
please see the io-uring dma-buf support[1], and iod->addr might carry IO buffer info
in dma-buf format in future.

[1] https://lore.kernel.org/io-uring/cover.1763725387.git.asml.silence@gmail.com/#t


> size of struct ublksrv_io_desc. UBLK_F_INTEGRITY requires
> UBLK_F_USER_COPY and the addr field isn't used for UBLK_F_USER_COPY, so
> the two fields aren't needed simultaneously.
> 
> Signed-off-by: Caleb Sander Mateos <csander@...estorage.com>
> ---
>  drivers/block/ublk_drv.c      | 43 +++++++++++++++++++++++++++++++----
>  include/uapi/linux/ublk_cmd.h | 27 ++++++++++++++++++++--
>  2 files changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 2f9316febf83..51469e0627ff 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -316,10 +316,36 @@ static inline bool ublk_dev_is_zoned(const struct ublk_device *ub)
>  static inline bool ublk_queue_is_zoned(const struct ublk_queue *ubq)
>  {
>  	return ubq->flags & UBLK_F_ZONED;
>  }
>  
> +static void ublk_setup_iod_buf(const struct ublk_queue *ubq,
> +			       const struct request *req,
> +			       struct ublksrv_io_desc *iod)
> +{
> +#ifdef CONFIG_BLK_DEV_INTEGRITY
> +	if (ubq->flags & UBLK_F_INTEGRITY) {
> +		struct bio_integrity_payload *bip;
> +		sector_t ref_tag_seed;
> +
> +		if (!blk_integrity_rq(req))
> +			return;
> +
> +		bip = bio_integrity(req->bio);
> +		ref_tag_seed = bip_get_seed(bip);

As mentioned, t10_pi_generate() and ext_pi_crc64_generate() builds
per-internal ref tag.


> +		iod->integrity.ref_tag_lo = ref_tag_seed;
> +		iod->integrity.ref_tag_hi = ref_tag_seed >> 32;
> +		iod->integrity.app_tag = bip->app_tag;

In case of io merge, each bio may have different ->app_tag.

Given you have to copy meta data via user copy, I suggest to follow the PI
standard and make it per-internal.

Thanks, 
Ming