[pve-devel] [PATCH pve-kernel 4.15] fix #2354: backport 32-bit overflow fix in blk-lib

[Stoiko Ivanov]

While analyzing #2354 I found the patch
(4800bf7bc8c725e955fcbc6191cc872f43f506d3) , which seems to fix the
issue with an endless loop on deletion of a 2TB+ sized LV

The issue got introduced in Ubuntu-4.15.0-56.62 which got mitigated
(the endless loop turns into a 'Operation not supported.' warning)
in Ubuntu-4.15.0-63.72, so it is not as high a priority.
(Discards still don't work for LVS with 2T+ sizes)

Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
---

This patch fixes the issue on my reproducer. However - given that the
original commit-message indicates to fix an issue introduced by a commit
not present in the bionic-repo and that the change from unsigned int to
sector_t (a 64bit type), while bio_allowed_max_sectors (block/blk.h)
explicitly says that the field it computes is 32-bit makes me wonder
if this does not introduce some other problems.

I still wanted to send the patch - mostly as a reminder, should the issue pop
up again with pve-kernel-4.15

...t-overflow-in-__blkdev_issue_discard.patch | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 patches/kernel/0011-block-fix-32-bit-overflow-in-__blkdev_issue_discard.patch

diff --git a/patches/kernel/0011-block-fix-32-bit-overflow-in-__blkdev_issue_discard.patch b/patches/kernel/0011-block-fix-32-bit-overflow-in-__blkdev_issue_discard.patch
new file mode 100644
index 0000000..2a4c7d9
--- /dev/null
+++ b/patches/kernel/0011-block-fix-32-bit-overflow-in-__blkdev_issue_discard.patch
@@ -0,0 +1,74 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Stoiko Ivanov <s.ivanov at proxmox.com>
+Date: Tue, 10 Sep 2019 11:15:21 +0200
+Subject: [PATCH] block: fix 32 bit overflow in __blkdev_issue_discard()
+
+this commit is a backport of 4800bf7bc8c725e955fcbc6191cc872f43f506d3
+(in torvalds/master) to Ubuntu Bionic. It fixes
+https://bugzilla.proxmox.com/show_bug.cgi?id=2354
+
+However since bionic does not contain ba5d73851e71, which should be
+fixed by this commit The argumentation in the original commit is
+probably not 100% correct. It seems to me that
+3c2f83d8bcbedeb89efcaf55ae64a99dce9d7e34 (ubuntu-bionic/master)
+introduced the issue by using unsigned int for req_sects.
+This commit never made it into any LTS kernels, hence the
+issue is not reproducible with mainline kernels.
+
+reported upstream:
+https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1843324
+
+Original commit-message:
+A discard cleanup merged into 4.20-rc2 causes fstests xfs/259 to
+fall into an endless loop in the discard code. The test is creating
+a device that is exactly 2^32 sectors in size to test mkfs boundary
+conditions around the 32 bit sector overflow region.
+
+mkfs issues a discard for the entire device size by default, and
+hence this throws a sector count of 2^32 into
+blkdev_issue_discard(). It takes the number of sectors to discard as
+a sector_t - a 64 bit value.
+
+The commit ba5d73851e71 ("block: cleanup __blkdev_issue_discard")
+takes this sector count and casts it to a 32 bit value before
+comapring it against the maximum allowed discard size the device
+has. This truncates away the upper 32 bits, and so if the lower 32
+bits of the sector count is zero, it starts issuing discards of
+length 0. This causes the code to fall into an endless loop, issuing
+a zero length discards over and over again on the same sector.
+
+Fixes: ba5d73851e71 ("block: cleanup __blkdev_issue_discard")
+Tested-by: Darrick J. Wong <darrick.wong at oracle.com>
+Reviewed-by: Darrick J. Wong <darrick.wong at oracle.com>
+Signed-off-by: Dave Chinner <dchinner at redhat.com>
+
+Killed pointless WARN_ON().
+
+(backported from: 4800bf7bc8c725e955fcbc6191cc872f43f506d3)
+Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
+---
+ block/blk-lib.c | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/block/blk-lib.c b/block/blk-lib.c
+index 012425f5bd8c..24de1c14128f 100644
+--- a/block/blk-lib.c
++++ b/block/blk-lib.c
+@@ -50,7 +50,7 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
+ 		return -EINVAL;
+ 
+ 	while (nr_sects) {
+-		unsigned int req_sects = nr_sects;
++		sector_t req_sects = nr_sects;
+ 		sector_t end_sect;
+ 
+ 		req_sects = min(req_sects, bio_allowed_max_sectors(q));
+@@ -59,6 +59,8 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
+ 
+ 		end_sect = sector + req_sects;
+ 
++		WARN_ON_ONCE((req_sects << 9) > UINT_MAX);
++
+ 		bio = next_bio(bio, 0, gfp_mask);
+ 		bio->bi_iter.bi_sector = sector;
+ 		bio_set_dev(bio, bdev);
-- 
2.20.1