[PATCH v2 0/2] Fix integrity sysfs reporting inconsistencies

[PATCH v2 0/2] Fix integrity sysfs reporting inconsistencies

[Anuj Gupta]

Patch 1: Ensures DM devices correctly propagate device_is_integrity_capable
Patch 2: initialize nogenerate and noverify correctly

v1 -> v2
initialize BLK_INTEGRITY_NOGENERATE and BLK_INTEGRITY_NOVERIFY in
blk_validate_integrity_limits rather than doing it in the driver (hch)

Anuj Gupta (2):
  block: ensure correct integrity capability propagation in stacked
    devices
  block: Correctly initialize BLK_INTEGRITY_NOGENERATE and
    BLK_INTEGRITY_NOVERIFY

 block/blk-settings.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v2 1/2] block: ensure correct integrity capability propagation in stacked devices

[Anuj Gupta]

queue_limits_stack_integrity() incorrectly sets
BLK_INTEGRITY_DEVICE_CAPABLE for a DM device even when none of its
underlying devices support integrity. This happens because the flag is
inherited from the first base device, even if it lacks integrity support.
This patch ensures that BLK_INTEGRITY_DEVICE_CAPABLE is only inherited if
the first device actually supports integrity.

Reported-by: M Nikhil <nikhilm@linux.ibm.com>
Link: https://lore.kernel.org/linux-block/f6130475-3ccd-45d2-abde-3ccceada0f0a@linux.ibm.com/
Fixes: c6e56cf6b2e7 ("block: move integrity information into queue_limits")
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
 block/blk-settings.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index c44dadc35e1e..8bd0d0f1479c 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -861,7 +861,8 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
 
 	if (!ti->tuple_size) {
 		/* inherit the settings from the first underlying device */
-		if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
+		if (!(ti->flags & BLK_INTEGRITY_STACKED) &&
+		    (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)) {
 			ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
 				(bi->flags & BLK_INTEGRITY_REF_TAG);
 			ti->csum_type = bi->csum_type;
-- 
2.25.1

[PATCH v2 2/2] block: Correctly initialize BLK_INTEGRITY_NOGENERATE and BLK_INTEGRITY_NOVERIFY

[Anuj Gupta]

Currently, BLK_INTEGRITY_NOGENERATE and BLK_INTEGRITY_NOVERIFY are not
explicitly set during integrity initialization. This can lead to incorrect
reporting of read_verify and write_generate sysfs values, particularly when
a device does not support integrity. This patch ensures that these flags
are correctly initialized by default.

Reported-by: M Nikhil <nikhilm@linux.ibm.com>
Link: https://lore.kernel.org/linux-block/f6130475-3ccd-45d2-abde-3ccceada0f0a@linux.ibm.com/
Fixes: 9f4aa46f2a74 ("block: invert the BLK_INTEGRITY_{GENERATE,VERIFY} flags")
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
 block/blk-settings.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 8bd0d0f1479c..bee4007b2eeb 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -114,6 +114,7 @@ static int blk_validate_integrity_limits(struct queue_limits *lim)
 			pr_warn("invalid PI settings.\n");
 			return -EINVAL;
 		}
+		bi->flags |= BLK_INTEGRITY_NOGENERATE | BLK_INTEGRITY_NOVERIFY;
 		return 0;
 	}
 
-- 
2.25.1

Re: [PATCH v2 2/2] block: Correctly initialize BLK_INTEGRITY_NOGENERATE and BLK_INTEGRITY_NOVERIFY

[Christoph Hellwig]

On Wed, Feb 26, 2025 at 04:50:35PM +0530, Anuj Gupta wrote:
> Currently, BLK_INTEGRITY_NOGENERATE and BLK_INTEGRITY_NOVERIFY are not
> explicitly set during integrity initialization. This can lead to incorrect
> reporting of read_verify and write_generate sysfs values, particularly when
> a device does not support integrity. This patch ensures that these flags
> are correctly initialized by default.

You don't need to stay "this patch" in a commit log, just state what
it is doing.  Also please wrap commit log lines at 73 characters.

The code changes themselves here look good.

Re: [PATCH v2 1/2] block: ensure correct integrity capability propagation in stacked devices

[Christoph Hellwig]

On Wed, Feb 26, 2025 at 04:50:34PM +0530, Anuj Gupta wrote:
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index c44dadc35e1e..8bd0d0f1479c 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -861,7 +861,8 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
>  
>  	if (!ti->tuple_size) {
>  		/* inherit the settings from the first underlying device */
> -		if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
> +		if (!(ti->flags & BLK_INTEGRITY_STACKED) &&
> +		    (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)) {
>  			ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
>  				(bi->flags & BLK_INTEGRITY_REF_TAG);
>  			ti->csum_type = bi->csum_type;

As mentioned last round this still does the wrong thing if the first
device(s) is/are not PI-capable but the next one(s) is/are.  Please
look into the pseudocode I posted in reply to the previous iteration
on how to fix it.

Re: [PATCH v2 1/2] block: ensure correct integrity capability propagation in stacked devices

[Anuj Gupta]

[-- Attachment #1: Type: text/plain, Size: 3046 bytes --]

On Mon, Mar 03, 2025 at 03:12:36PM +0100, Christoph Hellwig wrote:
> On Wed, Feb 26, 2025 at 04:50:34PM +0530, Anuj Gupta wrote:
> > diff --git a/block/blk-settings.c b/block/blk-settings.c
> > index c44dadc35e1e..8bd0d0f1479c 100644
> > --- a/block/blk-settings.c
> > +++ b/block/blk-settings.c
> > @@ -861,7 +861,8 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
> >  
> >  	if (!ti->tuple_size) {
> >  		/* inherit the settings from the first underlying device */
> > -		if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
> > +		if (!(ti->flags & BLK_INTEGRITY_STACKED) &&
> > +		    (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)) {
> >  			ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
> >  				(bi->flags & BLK_INTEGRITY_REF_TAG);
> >  			ti->csum_type = bi->csum_type;
> 
> As mentioned last round this still does the wrong thing if the first
> device(s) is/are not PI-capable but the next one(s) is/are.  Please
> look into the pseudocode I posted in reply to the previous iteration
> on how to fix it.

Christoph,
Right, based on your comment modified this patch.
Does this look ok?

diff --git a/block/blk-settings.c b/block/blk-settings.c
index c44dadc35e1e..d0469a812734 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -859,36 +859,28 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
 	if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
 		return true;
 
-	if (!ti->tuple_size) {
-		/* inherit the settings from the first underlying device */
-		if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
-			ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
-				(bi->flags & BLK_INTEGRITY_REF_TAG);
-			ti->csum_type = bi->csum_type;
-			ti->tuple_size = bi->tuple_size;
-			ti->pi_offset = bi->pi_offset;
-			ti->interval_exp = bi->interval_exp;
-			ti->tag_size = bi->tag_size;
-			goto done;
-		}
-		if (!bi->tuple_size)
-			goto done;
+	if (ti->flags & BLK_INTEGRITY_STACKED) {
+		if (ti->tuple_size != bi->tuple_size)
+			goto incompatible;
+		if (ti->interval_exp != bi->interval_exp)
+			goto incompatible;
+		if (ti->tag_size != bi->tag_size)
+			goto incompatible;
+		if (ti->csum_type != bi->csum_type)
+			goto incompatible;
+		if ((ti->flags & BLK_INTEGRITY_REF_TAG) !=
+		    (bi->flags & BLK_INTEGRITY_REF_TAG))
+			goto incompatible;
+	} else {
+		ti->flags = BLK_INTEGRITY_STACKED;
+		ti->flags |= (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE) |
+			     (bi->flags & BLK_INTEGRITY_REF_TAG);
+		ti->csum_type = bi->csum_type;
+		ti->tuple_size = bi->tuple_size;
+		ti->pi_offset = bi->pi_offset;
+		ti->interval_exp = bi->interval_exp;
+		ti->tag_size = bi->tag_size;
 	}
-
-	if (ti->tuple_size != bi->tuple_size)
-		goto incompatible;
-	if (ti->interval_exp != bi->interval_exp)
-		goto incompatible;
-	if (ti->tag_size != bi->tag_size)
-		goto incompatible;
-	if (ti->csum_type != bi->csum_type)
-		goto incompatible;
-	if ((ti->flags & BLK_INTEGRITY_REF_TAG) !=
-	    (bi->flags & BLK_INTEGRITY_REF_TAG))
-		goto incompatible;
-
-done:
-	ti->flags |= BLK_INTEGRITY_STACKED;
 	return true;
 
 incompatible:
-- 
2.25.1



[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 1/2] block: ensure correct integrity capability propagation in stacked devices

[Christoph Hellwig]

On Tue, Mar 04, 2025 at 02:18:33PM +0530, Anuj Gupta wrote:
> Christoph,
> Right, based on your comment modified this patch.
> Does this look ok?

Yes, this looks good.