[PATCH 1/2] RDMA/srp: don't set a max_segment_size when virt_boundary_mask is set

[PATCH 1/2] RDMA/srp: don't set a max_segment_size when virt_boundary_mask is set

[Christoph Hellwig]

virt_boundary_mask implies an unlimited max_segment_size.  Setting both
can lead to data corruption, and we're going to check for this in the
SCSI midlayer soon.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 1378651735f6..23ed2fc688f0 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3705,9 +3705,10 @@ static ssize_t add_target_store(struct device *dev,
 	target_host->max_id      = 1;
 	target_host->max_lun     = -1LL;
 	target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
-	target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
 
-	if (!(ibdev->attrs.kernel_cap_flags & IBK_SG_GAPS_REG))
+	if (ibdev->attrs.kernel_cap_flags & IBK_SG_GAPS_REG)
+		target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
+	else
 		target_host->virt_boundary_mask = ~srp_dev->mr_page_mask;
 
 	target = host_to_target(target_host);
-- 
2.47.2

Re: [PATCH 1/2] RDMA/srp: don't set a max_segment_size when virt_boundary_mask is set

[Bart Van Assche]

On 6/23/25 1:02 AM, Christoph Hellwig wrote:
> virt_boundary_mask implies an unlimited max_segment_size.  Setting both
> can lead to data corruption, and we're going to check for this in the
> SCSI midlayer soon.

Please make this patch description more detailed and mention that
__blk_rq_map_sg() may split sg-lists such that the virt_boundary_mask is
not respected if max_segment_size != UINT_MAX.

Thanks,

Bart.

Re: [PATCH 1/2] RDMA/srp: don't set a max_segment_size when virt_boundary_mask is set

[Hannes Reinecke]

On 6/23/25 10:02, Christoph Hellwig wrote:
> virt_boundary_mask implies an unlimited max_segment_size.  Setting both
> can lead to data corruption, and we're going to check for this in the
> SCSI midlayer soon.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/infiniband/ulp/srp/ib_srp.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index 1378651735f6..23ed2fc688f0 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -3705,9 +3705,10 @@ static ssize_t add_target_store(struct device *dev,
>   	target_host->max_id      = 1;
>   	target_host->max_lun     = -1LL;
>   	target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
> -	target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
>   
> -	if (!(ibdev->attrs.kernel_cap_flags & IBK_SG_GAPS_REG))
> +	if (ibdev->attrs.kernel_cap_flags & IBK_SG_GAPS_REG)
> +		target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
> +	else
>   		target_host->virt_boundary_mask = ~srp_dev->mr_page_mask;
>   
>   	target = host_to_target(target_host);

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

fix virt_boundary_mask handling in SCSI v2

[Christoph Hellwig]

Hi all,

this series fixes a corruption when drivers using virt_boundary_mask set
a limited max_segment_size by accident, which Red Hat reported as causing
data corruption with storvsc.  I did audit the tree and also found that
this can affect SRP and iSER as well.

Changes since v1:
 - improve the srp commit log
 - slightly simplify the limits assignment in hosts.c

Diffstat:
 infiniband/ulp/srp/ib_srp.c |    5 +++--
 scsi/hosts.c                |   18 +++++++++++-------
 2 files changed, 14 insertions(+), 9 deletions(-)

[PATCH 1/2] RDMA/srp: don't set a max_segment_size when virt_boundary_mask is set

[Christoph Hellwig]

virt_boundary_mask implies an unlimited max_segment_size.  Setting both
can lead to data corruption because __blk_rq_map_sg() can split requests
so that the virt_boundary_mask is not respected if max_segment_size is
not UINT_MAX.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 1378651735f6..23ed2fc688f0 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3705,9 +3705,10 @@ static ssize_t add_target_store(struct device *dev,
 	target_host->max_id      = 1;
 	target_host->max_lun     = -1LL;
 	target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
-	target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
 
-	if (!(ibdev->attrs.kernel_cap_flags & IBK_SG_GAPS_REG))
+	if (ibdev->attrs.kernel_cap_flags & IBK_SG_GAPS_REG)
+		target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
+	else
 		target_host->virt_boundary_mask = ~srp_dev->mr_page_mask;
 
 	target = host_to_target(target_host);
-- 
2.47.2

[PATCH 2/2] scsi: enforce unlimited max_segment_size when virt_boundary_mask is set

[Christoph Hellwig]

The virt_boundary_mask limit requires an unlimited max_segment_size for
bio splitting to not corrupt data.  Historically, the block layer tried
to validate this, although the check was half-hearted until the addition
of the atomic queue limits API.  The full blown check than triggered
issues with stacked devices incorrectly inheriting limits such as the
virt boundary and got disabled in commit b561ea56a264 ("block: allow
device to have both virt_boundary_mask and max segment size") instead of
fixing the issue properly.

Ensure that the SCSI mid layer doesn't set the default low
max_segment_size limit for this case, and check for invalid
max_segment_size values in the host template, similar to the original
block layer check given that SCSI devices can't be stacked.

This fixes reported data corruption on storvsc, although as far as I can
tell storvsc always failed to properly set the max_segment_size limit as
the SCSI APIs historically applied that when setting up the host, while
storvsc only set the virt_boundary_mask when configuring the scsi_device.

Fixes: 81988a0e6b03 ("storvsc: get rid of bounce buffer")
Fixes: b561ea56a264 ("block: allow device to have both virt_boundary_mask and max segment size")
Reported-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/hosts.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index e021f1106bea..cc5d05dc395c 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -473,10 +473,17 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
 	else
 		shost->max_sectors = SCSI_DEFAULT_MAX_SECTORS;
 
-	if (sht->max_segment_size)
-		shost->max_segment_size = sht->max_segment_size;
-	else
-		shost->max_segment_size = BLK_MAX_SEGMENT_SIZE;
+	shost->virt_boundary_mask = sht->virt_boundary_mask;
+	if (shost->virt_boundary_mask) {
+		WARN_ON_ONCE(sht->max_segment_size &&
+			     sht->max_segment_size != UINT_MAX);
+		shost->max_segment_size = UINT_MAX;
+	} else {
+		if (sht->max_segment_size)
+			shost->max_segment_size = sht->max_segment_size;
+		else
+			shost->max_segment_size = BLK_MAX_SEGMENT_SIZE;
+	}
 
 	/* 32-byte (dword) is a common minimum for HBAs. */
 	if (sht->dma_alignment)
@@ -492,9 +499,6 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
 	else
 		shost->dma_boundary = 0xffffffff;
 
-	if (sht->virt_boundary_mask)
-		shost->virt_boundary_mask = sht->virt_boundary_mask;
-
 	device_initialize(&shost->shost_gendev);
 	dev_set_name(&shost->shost_gendev, "host%d", shost->host_no);
 	shost->shost_gendev.bus = &scsi_bus_type;
-- 
2.47.2

Re: [PATCH 1/2] RDMA/srp: don't set a max_segment_size when virt_boundary_mask is set

[John Garry]

On 24/06/2025 13:52, Christoph Hellwig wrote:
> virt_boundary_mask implies an unlimited max_segment_size.  Setting both
> can lead to data corruption because __blk_rq_map_sg() can split requests
> so that the virt_boundary_mask is not respected if max_segment_size is
> not UINT_MAX.
> 
> Signed-off-by: Christoph Hellwig<hch@lst.de>
> Reviewed-by: Hannes Reinecke<hare@suse.de>

Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH 1/2] RDMA/srp: don't set a max_segment_size when virt_boundary_mask is set

[Bart Van Assche]

On 6/24/25 5:52 AM, Christoph Hellwig wrote:
> virt_boundary_mask implies an unlimited max_segment_size.  Setting both
> can lead to data corruption because __blk_rq_map_sg() can split requests
> so that the virt_boundary_mask is not respected if max_segment_size is
> not UINT_MAX.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/infiniband/ulp/srp/ib_srp.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index 1378651735f6..23ed2fc688f0 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -3705,9 +3705,10 @@ static ssize_t add_target_store(struct device *dev,
>   	target_host->max_id      = 1;
>   	target_host->max_lun     = -1LL;
>   	target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
> -	target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
>   
> -	if (!(ibdev->attrs.kernel_cap_flags & IBK_SG_GAPS_REG))
> +	if (ibdev->attrs.kernel_cap_flags & IBK_SG_GAPS_REG)
> +		target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
> +	else
>   		target_host->virt_boundary_mask = ~srp_dev->mr_page_mask;
>   
>   	target = host_to_target(target_host);

Acked-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH 2/2] scsi: enforce unlimited max_segment_size when virt_boundary_mask is set

[Bart Van Assche]

On 6/24/25 5:52 AM, Christoph Hellwig wrote:
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index e021f1106bea..cc5d05dc395c 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -473,10 +473,17 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
>   	else
>   		shost->max_sectors = SCSI_DEFAULT_MAX_SECTORS;
>   
> -	if (sht->max_segment_size)
> -		shost->max_segment_size = sht->max_segment_size;
> -	else
> -		shost->max_segment_size = BLK_MAX_SEGMENT_SIZE;
> +	shost->virt_boundary_mask = sht->virt_boundary_mask;
> +	if (shost->virt_boundary_mask) {
> +		WARN_ON_ONCE(sht->max_segment_size &&
> +			     sht->max_segment_size != UINT_MAX);
> +		shost->max_segment_size = UINT_MAX;
> +	} else {
> +		if (sht->max_segment_size)
> +			shost->max_segment_size = sht->max_segment_size;
> +		else
> +			shost->max_segment_size = BLK_MAX_SEGMENT_SIZE;
> +	}
>   
>   	/* 32-byte (dword) is a common minimum for HBAs. */
>   	if (sht->dma_alignment)
> @@ -492,9 +499,6 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
>   	else
>   		shost->dma_boundary = 0xffffffff;
>   
> -	if (sht->virt_boundary_mask)
> -		shost->virt_boundary_mask = sht->virt_boundary_mask;
> -
>   	device_initialize(&shost->shost_gendev);
>   	dev_set_name(&shost->shost_gendev, "host%d", shost->host_no);
>   	shost->shost_gendev.bus = &scsi_bus_type;

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: fix virt_boundary_mask handling in SCSI v2

[Martin K. Petersen]

On Tue, 24 Jun 2025 14:52:26 +0200, Christoph Hellwig wrote:

> this series fixes a corruption when drivers using virt_boundary_mask set
> a limited max_segment_size by accident, which Red Hat reported as causing
> data corruption with storvsc.  I did audit the tree and also found that
> this can affect SRP and iSER as well.
> 
> Changes since v1:
>  - improve the srp commit log
>  - slightly simplify the limits assignment in hosts.c
> 
> [...]

Applied to 6.16/scsi-fixes, thanks!

[1/2] RDMA/srp: don't set a max_segment_size when virt_boundary_mask is set
      https://git.kernel.org/mkp/scsi/c/844c6a160e69
[2/2] scsi: enforce unlimited max_segment_size when virt_boundary_mask is set
      https://git.kernel.org/mkp/scsi/c/4937e604ca24

-- 
Martin K. Petersen	Oracle Linux Engineering