integrity regression fixes for 4.13-rc

integrity regression fixes for 4.13-rc

[Christoph Hellwig]

Hi Jens,

this series fixes regressions in the integrity handling update in 4.13-rc.

The first one was sent earlier by Milan and while both Martin and I aren't
exactly happy about the way dm uses the integrity code to cause this
regression this minimal fix gets us back to the status quo.

The second one makes sure that we only verify the DIF checksums on the
lowest layer where we attach the integrity information.

[PATCH 1/2] bio-integrity: Fix regression if profile verify_fn is NULL

[Christoph Hellwig]

From: Milan Broz <gmazyland@gmail.com>

In dm-integrity target we register integrity profile that have
both generate_fn and verify_fn callbacks set to NULL.

This is used if dm-integrity is stacked under a dm-crypt device
for authenticated encryption (integrity payload contains authentication
tag and IV seed).

In this case the verification is done through own crypto API
processing inside dm-crypt; integrity profile is only holder
of these data. (And memory is owned by dm-crypt as well.)

After the commit (and previous changes)
  Commit 7c20f11680a441df09de7235206f70115fbf6290
  Author: Christoph Hellwig <hch@lst.de>
  Date:   Mon Jul 3 16:58:43 2017 -0600

    bio-integrity: stop abusing bi_end_io

we get this crash:

: BUG: unable to handle kernel NULL pointer dereference at   (null)
: IP:   (null)
: *pde = 00000000
...
:
: Workqueue: kintegrityd bio_integrity_verify_fn
: task: f48ae180 task.stack: f4b5c000
: EIP:   (null)
: EFLAGS: 00210286 CPU: 0
: EAX: f4b5debc EBX: 00001000 ECX: 00000001 EDX: 00000000
: ESI: 00001000 EDI: ed25f000 EBP: f4b5dee8 ESP: f4b5dea4
:  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
: CR0: 80050033 CR2: 00000000 CR3: 32823000 CR4: 001406d0
: Call Trace:
:  ? bio_integrity_process+0xe3/0x1e0
:  bio_integrity_verify_fn+0xea/0x150
:  process_one_work+0x1c7/0x5c0
:  worker_thread+0x39/0x380
:  kthread+0xd6/0x110
:  ? process_one_work+0x5c0/0x5c0
:  ? kthread_worker_fn+0x100/0x100
:  ? kthread_worker_fn+0x100/0x100
:  ret_from_fork+0x19/0x24
: Code:  Bad EIP value.
: EIP:   (null) SS:ESP: 0068:f4b5dea4
: CR2: 0000000000000000

Patch just skip the whole verify workqueue if verify_fn is set to NULL.

Fixes: 7c20f116 ("bio-integrity: stop abusing bi_end_io")
Signed-off-by: Milan Broz <gmazyland@gmail.com>
[hch: trivial whitespace fix]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio-integrity.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 83e92beb3c9f..dcfb968c8259 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -387,7 +387,10 @@ static void bio_integrity_verify_fn(struct work_struct *work)
  */
 bool __bio_integrity_endio(struct bio *bio)
 {
-	if (bio_op(bio) == REQ_OP_READ && !bio->bi_status) {
+	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
+
+	if (bio_op(bio) == REQ_OP_READ && !bio->bi_status &&
+	    bi->profile->verify_fn)
 		struct bio_integrity_payload *bip = bio_integrity(bio);
 
 		INIT_WORK(&bip->bip_work, bio_integrity_verify_fn);
-- 
2.11.0

[PATCH 2/2] bio-integrity: only verify integrity on the lowest stacked driver

[Christoph Hellwig]

This gets us back to the behavior in 4.12 and earlier.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Fixes: 7c20f116 ("bio-integrity: stop abusing bi_end_io")
---
 block/bio-integrity.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index dcfb968c8259..9b1ea478577b 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -388,11 +388,10 @@ static void bio_integrity_verify_fn(struct work_struct *work)
 bool __bio_integrity_endio(struct bio *bio)
 {
 	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
+	struct bio_integrity_payload *bip = bio_integrity(bio);
 
 	if (bio_op(bio) == REQ_OP_READ && !bio->bi_status &&
-	    bi->profile->verify_fn)
-		struct bio_integrity_payload *bip = bio_integrity(bio);
-
+	    (bip->bip_flags & BIP_BLOCK_INTEGRITY) && bi->profile->verify_fn) {
 		INIT_WORK(&bip->bip_work, bio_integrity_verify_fn);
 		queue_work(kintegrityd_wq, &bip->bip_work);
 		return false;
-- 
2.11.0

Re: integrity regression fixes for 4.13-rc

[Jens Axboe]

On 08/09/2017 09:47 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series fixes regressions in the integrity handling update in 4.13-rc.
> 
> The first one was sent earlier by Milan and while both Martin and I aren't
> exactly happy about the way dm uses the integrity code to cause this
> regression this minimal fix gets us back to the status quo.
> 
> The second one makes sure that we only verify the DIF checksums on the
> lowest layer where we attach the integrity information.

Thanks, I was waiting on these. Applied for 4.13. I did just ship off
a pull request, but I'll get this in for the next -rc.

-- 
Jens Axboe

Re: integrity regression fixes for 4.13-rc

[Martin K. Petersen]

Christoph,

> this series fixes regressions in the integrity handling update in
> 4.13-rc.
>
> The first one was sent earlier by Milan and while both Martin and I
> aren't exactly happy about the way dm uses the integrity code to cause
> this regression this minimal fix gets us back to the status quo.
>
> The second one makes sure that we only verify the DIF checksums on the
> lowest layer where we attach the integrity information.

These look OK to me.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering