linux-kernel - Re: [PATCH 06/20] ublk: support UBLK_PARAM_TYPE

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aUlZ7APsr7tIdrWq@fedora>
Date: Mon, 22 Dec 2025 22:47:08 +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 06/20] ublk: support UBLK_PARAM_TYPE_INTEGRITY in device
 creation

On Tue, Dec 16, 2025 at 10:34:40PM -0700, Caleb Sander Mateos wrote:
> From: Stanley Zhang <stazhang@...estorage.com>
> 
> If the UBLK_PARAM_TYPE_INTEGRITY flag is set, validate the integrity
> parameters and apply them to the blk_integrity limits.
> UBLK_PARAM_TYPE_INTEGRITY requires CONFIG_BLK_DEV_INTEGRITY=y,
> UBLK_F_USER_COPY, and metadata_size > 0. Reuse the block metadata ioctl
> LBMD_PI_CAP_* and LBMD_PI_CSUM_* constants from the linux/fs.h UAPI
> header for the flags and csum_type field values.
> The struct ublk_param_integrity validations are based on the checks in
> blk_validate_integrity_limits(). Any invalid parameters should be
> rejected before being applied to struct blk_integrity.
> 
> Signed-off-by: Stanley Zhang <stazhang@...estorage.com>
> [csander: add param validation]
> Signed-off-by: Caleb Sander Mateos <csander@...estorage.com>
> ---
>  drivers/block/ublk_drv.c | 92 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 4da5d8ff1e1d..2893a9172220 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -42,10 +42,12 @@
>  #include <linux/mm.h>
>  #include <asm/page.h>
>  #include <linux/task_work.h>
>  #include <linux/namei.h>
>  #include <linux/kref.h>
> +#include <linux/blk-integrity.h>
> +#include <uapi/linux/fs.h>
>  #include <uapi/linux/ublk_cmd.h>
>  
>  #define UBLK_MINORS		(1U << MINORBITS)
>  
>  #define UBLK_INVALID_BUF_IDX 	((u16)-1)
> @@ -81,11 +83,12 @@
>  
>  /* All UBLK_PARAM_TYPE_* should be included here */
>  #define UBLK_PARAM_TYPE_ALL                                \
>  	(UBLK_PARAM_TYPE_BASIC | UBLK_PARAM_TYPE_DISCARD | \
>  	 UBLK_PARAM_TYPE_DEVT | UBLK_PARAM_TYPE_ZONED |    \
> -	 UBLK_PARAM_TYPE_DMA_ALIGN | UBLK_PARAM_TYPE_SEGMENT)
> +	 UBLK_PARAM_TYPE_DMA_ALIGN | UBLK_PARAM_TYPE_SEGMENT | \
> +	 UBLK_PARAM_TYPE_INTEGRITY)
>  
>  struct ublk_uring_cmd_pdu {
>  	/*
>  	 * Store requests in same batch temporarily for queuing them to
>  	 * daemon context.
> @@ -613,10 +616,57 @@ static void ublk_dev_param_basic_apply(struct ublk_device *ub)
>  		set_disk_ro(ub->ub_disk, true);
>  
>  	set_capacity(ub->ub_disk, p->dev_sectors);
>  }
>  
> +static int ublk_integrity_flags(u32 flags)
> +{
> +	int ret_flags = 0;
> +
> +	if (flags & LBMD_PI_CAP_INTEGRITY) {
> +		flags &= ~LBMD_PI_CAP_INTEGRITY;
> +		ret_flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
> +	}
> +	if (flags & LBMD_PI_CAP_REFTAG) {
> +		flags &= ~LBMD_PI_CAP_REFTAG;
> +		ret_flags |= BLK_INTEGRITY_REF_TAG;
> +	}
> +	return flags ? -EINVAL : ret_flags;
> +}
> +
> +static int ublk_integrity_pi_tuple_size(u8 csum_type)
> +{
> +	switch (csum_type) {
> +	case LBMD_PI_CSUM_NONE:
> +		return 0;
> +	case LBMD_PI_CSUM_IP:
> +	case LBMD_PI_CSUM_CRC16_T10DIF:
> +		return 8;
> +	case LBMD_PI_CSUM_CRC64_NVME:
> +		return 16;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static enum blk_integrity_checksum ublk_integrity_csum_type(u8 csum_type)
> +{
> +	switch (csum_type) {
> +	case LBMD_PI_CSUM_NONE:
> +		return BLK_INTEGRITY_CSUM_NONE;
> +	case LBMD_PI_CSUM_IP:
> +		return BLK_INTEGRITY_CSUM_IP;
> +	case LBMD_PI_CSUM_CRC16_T10DIF:
> +		return BLK_INTEGRITY_CSUM_CRC;
> +	case LBMD_PI_CSUM_CRC64_NVME:
> +		return BLK_INTEGRITY_CSUM_CRC64;
> +	default:
> +		WARN_ON_ONCE(1);
> +		return BLK_INTEGRITY_CSUM_NONE;
> +	}
> +}
> +
>  static int ublk_validate_params(const struct ublk_device *ub)
>  {
>  	/* basic param is the only one which must be set */
>  	if (ub->params.types & UBLK_PARAM_TYPE_BASIC) {
>  		const struct ublk_param_basic *p = &ub->params.basic;
> @@ -675,10 +725,35 @@ static int ublk_validate_params(const struct ublk_device *ub)
>  			return -EINVAL;
>  		if (p->max_segment_size < UBLK_MIN_SEGMENT_SIZE)
>  			return -EINVAL;
>  	}
>  
> +	if (ub->params.types & UBLK_PARAM_TYPE_INTEGRITY) {
> +		const struct ublk_param_integrity *p = &ub->params.integrity;
> +		int pi_tuple_size = ublk_integrity_pi_tuple_size(p->csum_type);
> +		int flags = ublk_integrity_flags(p->flags);
> +
> +		if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
> +			return -EINVAL;
> +		if (!ublk_dev_support_user_copy(ub))
> +			return -EINVAL;

UBLK_IO_F_INTEGRITY should be checked here, and ublk_dev_support_user_copy() can be
validated with UBLK_IO_F_INTEGRITY together in ublk_ctrl_add_dev(), so
mis-matched features can be failed earlier.

Same for IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY).

> +		if (flags < 0)
> +			return flags;
> +		if (pi_tuple_size < 0)
> +			return pi_tuple_size;
> +		if (!p->metadata_size)
> +			return -EINVAL;

blk_validate_integrity_limits() allows zero p->metadata_size with
LBMD_PI_CSUM_NONE, maybe document ublk's support for zero metadata_size & LBMD_PI_CSUM_NONE?



Thanks, 
Ming