FAILED: patch "[PATCH] null_blk: Fix zone size initialization" failed to apply to 4.19-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 0ebcdd702f49aeb0ad2e2d894f8c124a0acc6e23 Mon Sep 17 00:00:00 2001
From: Damien Le Moal <[email protected]>
Date: Fri, 20 Nov 2020 10:55:11 +0900
Subject: [PATCH] null_blk: Fix zone size initialization
For a null_blk device with zoned mode enabled is currently initialized with a number of zones equal to the device capacity divided by the zone size, without considering if the device capacity is a multiple of the zone size. If the zone size is not a divisor of the capacity, the zones end up not covering the entire capacity, potentially resulting is out of bounds accesses to the zone array.
Fix this by adding one last smaller zone with a size equal to the remainder of the disk capacity divided by the zone size if the capacity is not a multiple of the zone size. For such smaller last zone, the zone capacity is also checked so that it does not exceed the smaller zone size.
drivers/block/null_blk_zoned.c +15 -8
@@ -6,8 +6,7 @@#define CREATE_TRACE_POINTS #include "null_blk_trace.h" -/* zone_size in MBs to sectors. */ -#define ZONE_SIZE_SHIFT 11 +#define MB_TO_SECTS (mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT)static inline unsigned int null_zone_no (struct nullb_device *dev,sector_t sect) {
@@ -16,7 +15,7 @@ static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)int null_init_zoned_dev (struct nullb_device *dev,struct request_queue *q) { -sector_t dev_size = (sector_t )dev->size *1024 *1024 ; +sector_t dev_capacity_sects, zone_capacity_sects;sector_t sector =0 ;unsigned int i;
@@ -38,9 +37,13 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)return -EINVAL ; } - dev->zone_size_sects = dev->zone_size <<ZONE_SIZE_SHIFT ; - dev->nr_zones = dev_size >> - (SECTOR_SHIFT +ilog2 (dev->zone_size_sects)); + zone_capacity_sects =MB_TO_SECTS (dev->zone_capacity); + dev_capacity_sects =MB_TO_SECTS (dev->size); + dev->zone_size_sects =MB_TO_SECTS (dev->zone_size); + dev->nr_zones = dev_capacity_sects >>ilog2 (dev->zone_size_sects); +if (dev_capacity_sects & (dev->zone_size_sects -1 )) + dev->nr_zones++; + dev->zones =kvmalloc_array (dev->nr_zones,sizeof (struct blk_zone ),GFP_KERNEL | __GFP_ZERO);if (!dev->zones)
@@ -101,8 +104,12 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)struct blk_zone *zone = &dev->zones[i]; zone->start = zone->wp = sector; - zone->len = dev->zone_size_sects; - zone->capacity = dev->zone_capacity <<ZONE_SIZE_SHIFT ; +if (zone->start + dev->zone_size_sects > dev_capacity_sects) + zone->len = dev_capacity_sects - zone->start; +else + zone->len = dev->zone_size_sects; + zone->capacity = +min_t (sector_t, zone->len, zone_capacity_sects); zone->type =BLK_ZONE_TYPE_SEQWRITE_REQ ; zone->cond =BLK_ZONE_COND_EMPTY ;
PATCH 1/1 null_blk: Fix zone size initialization
commit 0ebcdd702f49aeb0ad2e2d894f8c124a0acc6e23 upstream.
For a null_blk device with zoned mode enabled is currently initialized with a number of zones equal to the device capacity divided by the zone size, without considering if the device capacity is a multiple of the zone size. If the zone size is not a divisor of the capacity, the zones end up not covering the entire capacity, potentially resulting is out of bounds accesses to the zone array.
Fix this by adding one last smaller zone with a size equal to the remainder of the disk capacity divided by the zone size if the capacity is not a multiple of the zone size. For such smaller last zone, the zone capacity is also checked so that it does not exceed the smaller zone size.
--- drivers/block/null_blk_zoned.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
drivers/block/null_blk_zoned.c +12 -7
@@ -2,8 +2,7 @@#include <linux/vmalloc.h> #include "null_blk.h" -/* zone_size in MBs to sectors. */ -#define ZONE_SIZE_SHIFT 11 +#define MB_TO_SECTS (mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT)static inline unsigned int null_zone_no (struct nullb_device *dev,sector_t sect) {
@@ -12,7 +11,7 @@ static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)int null_zone_init (struct nullb_device *dev) { -sector_t dev_size = (sector_t )dev->size *1024 *1024 ; +sector_t dev_capacity_sects;sector_t sector =0 ;unsigned int i;
@@ -25,9 +24,12 @@ int null_zone_init(struct nullb_device *dev)return -EINVAL ; } - dev->zone_size_sects = dev->zone_size <<ZONE_SIZE_SHIFT ; - dev->nr_zones = dev_size >> - (SECTOR_SHIFT +ilog2 (dev->zone_size_sects)); + dev_capacity_sects =MB_TO_SECTS (dev->size); + dev->zone_size_sects =MB_TO_SECTS (dev->zone_size); + dev->nr_zones = dev_capacity_sects >>ilog2 (dev->zone_size_sects); +if (dev_capacity_sects & (dev->zone_size_sects -1 )) + dev->nr_zones++; + dev->zones =kvmalloc_array (dev->nr_zones,sizeof (struct blk_zone ),GFP_KERNEL | __GFP_ZERO);if (!dev->zones)
@@ -55,7 +57,10 @@ int null_zone_init(struct nullb_device *dev)struct blk_zone *zone = &dev->zones[i]; zone->start = zone->wp = sector; - zone->len = dev->zone_size_sects; +if (zone->start + dev->zone_size_sects > dev_capacity_sects) + zone->len = dev_capacity_sects - zone->start; +else + zone->len = dev->zone_size_sects; zone->type =BLK_ZONE_TYPE_SEQWRITE_REQ ; zone->cond =BLK_ZONE_COND_EMPTY ;
Signature · 1 line
2.29.2
Quoted 8 lines · On Mon, 2020-12-28 at 12:49 +0100, [email protected] wrote:
The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.thanks,
greg k-h
Hi Greg,
I sent a backported patch for 4.19-stable in reply to your email. The backport is identical to the one I sent separately for the 5.4-stable tree.
Thanks.
Quoted 106 lines
------------------ original commit in Linus's tree ------------------
From
0ebcdd702f49aeb0ad2e2d894f8c124a0acc6e23Mon Sep 17 00:00:00 2001
From: Damien Le Moal <[email protected]>
Date: Fri, 20 Nov 2020 10:55:11 +0900
Subject: [PATCH]null_blk: Fix zone size initializationFor a
null_blkdevice with zoned mode enabled is currently initialized with a number of zones equal to the device capacity divided by the zone size, without considering if the device capacity is a multiple of the zone size. If the zone size is not a divisor of the capacity, the zones end up not covering the entire capacity, potentially resulting is out of bounds accesses to the zone array.Fix this by adding one last smaller zone with a size equal to the remainder of the disk capacity divided by the zone size if the capacity is not a multiple of the zone size. For such smaller last zone, the zone capacity is also checked so that it does not exceed the smaller zone size.
Reported-by Naohiro Aota <[email protected]>Fixes ca4b2a011948 ("null_blk: add zone support")Signed-off-by Damien Le Moal <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>Reviewed-by Johannes Thumshirn <[email protected]>Signed-off-by Jens Axboe <[email protected]>drivers/block/null_blk_zoned.c +0
-0@@ -6,8 +6,7 @@ #define CREATE_TRACE_POINTS #include "null_blk_trace.h"drivers/block/null_blk_zoned.c +1
-2-/* zone_size in MBs to sectors. */ -#define ZONE_SIZE_SHIFT 11 +#define MB_TO_SECTS (mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT)static inline unsigned int
null_zone_no(struct nullb_device *dev, sector_t sect){drivers/block/null_blk_zoned.c +0
-0@@ -16,7 +15,7 @@ static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)int
null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q){drivers/block/null_blk_zoned.c +1
-1-sector_t dev_size = (sector_t )dev->size *1024 *1024 ; +sector_t dev_capacity_sects, zone_capacity_sects;
sector_tsector = 0;
unsigned int i;drivers/block/null_blk_zoned.c +0
-0@@ -38,9 +37,13 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q) return -EINVAL; }drivers/block/null_blk_zoned.c +7
-3- dev->zone_size_sects = dev->zone_size <<ZONE_SIZE_SHIFT ; - dev->nr_zones = dev_size >> - (SECTOR_SHIFT +ilog2 (dev->zone_size_sects)); + zone_capacity_sects =MB_TO_SECTS (dev->zone_capacity); + dev_capacity_sects =MB_TO_SECTS (dev->size); + dev->zone_size_sects =MB_TO_SECTS (dev->zone_size); + dev->nr_zones = dev_capacity_sects >>ilog2 (dev->zone_size_sects); +if (dev_capacity_sects & (dev->zone_size_sects -1 )) + dev->nr_zones++; +
dev->zones=kvmalloc_array(dev->nr_zones, sizeof(struct blk_zone), GFP_KERNEL | __GFP_ZERO);
if (!dev->zones)drivers/block/null_blk_zoned.c +0
-0@@ -101,8 +104,12 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q) struct blk_zone *zone = &dev->zones[i];
zone->start=zone->wp= sector;drivers/block/null_blk_zoned.c +6
-2- zone->len = dev->zone_size_sects; - zone->capacity = dev->zone_capacity <<ZONE_SIZE_SHIFT ; +if (zone->start + dev->zone_size_sects > dev_capacity_sects) + zone->len = dev_capacity_sects - zone->start; +else + zone->len = dev->zone_size_sects; + zone->capacity = +min_t (sector_t, zone->len, zone_capacity_sects);
zone->type=BLK_ZONE_TYPE_SEQWRITE_REQ;zone->cond=BLK_ZONE_COND_EMPTY;
Signature · 2 lines
Damien Le Moal Western Digital
[email protected]
maintainerQuoted 6 lines · Damien Le Moal
Quoted 5 lines · On Mon, 2020-12-28 at 12:49 +0100, [email protected] wrote:
The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Quoted 8 lines
Quoted 3 lines
thanks,
greg k-h
Hi Greg,
I sent a backported patch for 4.19-stable in reply to your email. The backport is identical to the one I sent separately for the 5.4-stable tree.
It breaks the build:
drivers/block/null_blk_zoned.c: In function ‘null_zone_init’:drivers/block/null_blk_zoned.c:5:42: error: ‘SZ_1M’ undeclared (first use in this function)
5 | #define MB_TO_SECTS(mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT)
| ^~~~~
drivers/block/null_blk_zoned.c:27:23: note: in expansion of macro ‘MB_TO_SECTS’
27 | dev_capacity_sects = MB_TO_SECTS(dev->size);
| ^~~~~~~~~~~
drivers/block/null_blk_zoned.c:5:42: note: each undeclared identifier is reported only once for each function it appears in
5 | #define MB_TO_SECTS(mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT)
| ^~~~~
drivers/block/null_blk_zoned.c:27:23: note: in expansion of macro ‘MB_TO_SECTS’
27 | dev_capacity_sects = MB_TO_SECTS(dev->size);
| ^~~~~~~~~~~:(
Quoted 32 lines · On 2021/01/04 19:51, [email protected] wrote:
Quoted 6 lines · Damien Le Moal
Quoted 5 lines · On Mon, 2020-12-28 at 12:49 +0100, [email protected] wrote:
The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.Quoted 8 lines
Quoted 3 lines
thanks,
greg k-h
Hi Greg,
I sent a backported patch for 4.19-stable in reply to your email. The backport is identical to the one I sent separately for the 5.4-stable tree.
It breaks the build:
drivers/block/null_blk_zoned.c: In function ‘null_zone_init’:drivers/block/null_blk_zoned.c:5:42: error: ‘SZ_1M’ undeclared (first use in this function)5 | #define MB_TO_SECTS(mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT) | ^~~~~ drivers/block/null_blk_zoned.c:27:23: note: in expansion of macro ‘MB_TO_SECTS’ 27 | dev_capacity_sects = MB_TO_SECTS(dev->size); | ^~~~~~~~~~~ drivers/block/null_blk_zoned.c:5:42: note: each undeclared identifier is reported only once for each function it appears in 5 | #define MB_TO_SECTS(mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT) | ^~~~~ drivers/block/null_blk_zoned.c:27:23: note: in expansion of macro ‘MB_TO_SECTS’ 27 | dev_capacity_sects = MB_TO_SECTS(dev->size); | ^~~~~~~~~~~
Quoted 3 lines
:(
Oops. Looks like I screwed up something with my tests. Sorry about that. Let me resend that.
Signature · 2 lines
Damien Le Moal Western Digital Research
PATCH 1/1 null_blk: Fix zone size initialization
In reply to [email protected]commit 0ebcdd702f49aeb0ad2e2d894f8c124a0acc6e23 upstream.
For a null_blk device with zoned mode enabled is currently initialized with a number of zones equal to the device capacity divided by the zone size, without considering if the device capacity is a multiple of the zone size. If the zone size is not a divisor of the capacity, the zones end up not covering the entire capacity, potentially resulting is out of bounds accesses to the zone array.
Fix this by adding one last smaller zone with a size equal to the remainder of the disk capacity divided by the zone size if the capacity is not a multiple of the zone size. For such smaller last zone, the zone capacity is also checked so that it does not exceed the smaller zone size.
--- drivers/block/null_blk_zoned.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/block/null_blk_zoned.c +13 -7
@@ -1,9 +1,9 @@// SPDX-License-Identifier: GPL-2.0 #include <linux/vmalloc.h> +#include <linux/sizes.h> #include "null_blk.h" -/* zone_size in MBs to sectors. */ -#define ZONE_SIZE_SHIFT 11 +#define MB_TO_SECTS (mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT)static inline unsigned int null_zone_no (struct nullb_device *dev,sector_t sect) {
@@ -12,7 +12,7 @@ static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)int null_zone_init (struct nullb_device *dev) { -sector_t dev_size = (sector_t )dev->size *1024 *1024 ; +sector_t dev_capacity_sects;sector_t sector =0 ;unsigned int i;
@@ -25,9 +25,12 @@ int null_zone_init(struct nullb_device *dev)return -EINVAL ; } - dev->zone_size_sects = dev->zone_size <<ZONE_SIZE_SHIFT ; - dev->nr_zones = dev_size >> - (SECTOR_SHIFT +ilog2 (dev->zone_size_sects)); + dev_capacity_sects =MB_TO_SECTS (dev->size); + dev->zone_size_sects =MB_TO_SECTS (dev->zone_size); + dev->nr_zones = dev_capacity_sects >>ilog2 (dev->zone_size_sects); +if (dev_capacity_sects & (dev->zone_size_sects -1 )) + dev->nr_zones++; + dev->zones =kvmalloc_array (dev->nr_zones,sizeof (struct blk_zone ),GFP_KERNEL | __GFP_ZERO);if (!dev->zones)
@@ -37,7 +40,10 @@ int null_zone_init(struct nullb_device *dev)struct blk_zone *zone = &dev->zones[i]; zone->start = zone->wp = sector; - zone->len = dev->zone_size_sects; +if (zone->start + dev->zone_size_sects > dev_capacity_sects) + zone->len = dev_capacity_sects - zone->start; +else + zone->len = dev->zone_size_sects; zone->type =BLK_ZONE_TYPE_SEQWRITE_REQ ; zone->cond =BLK_ZONE_COND_EMPTY ;
Signature · 1 line
2.29.2
Quoted 32 lines · On 2021/01/04 19:51, [email protected] wrote:
Quoted 6 lines · Damien Le Moal
Quoted 5 lines · On Mon, 2020-12-28 at 12:49 +0100, [email protected] wrote:
The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.Quoted 8 lines
Quoted 3 lines
thanks,
greg k-h
Hi Greg,
I sent a backported patch for 4.19-stable in reply to your email. The backport is identical to the one I sent separately for the 5.4-stable tree.
It breaks the build:
drivers/block/null_blk_zoned.c: In function ‘null_zone_init’:drivers/block/null_blk_zoned.c:5:42: error: ‘SZ_1M’ undeclared (first use in this function)5 | #define MB_TO_SECTS(mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT) | ^~~~~ drivers/block/null_blk_zoned.c:27:23: note: in expansion of macro ‘MB_TO_SECTS’ 27 | dev_capacity_sects = MB_TO_SECTS(dev->size); | ^~~~~~~~~~~ drivers/block/null_blk_zoned.c:5:42: note: each undeclared identifier is reported only once for each function it appears in 5 | #define MB_TO_SECTS(mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT) | ^~~~~ drivers/block/null_blk_zoned.c:27:23: note: in expansion of macro ‘MB_TO_SECTS’ 27 | dev_capacity_sects = MB_TO_SECTS(dev->size); | ^~~~~~~~~~~
Quoted 1 line
:(
Unclear where I made a mistake before sending this. It was missing an include. I sent the proper patch, correctly tested this time, in reply to the above email.
Thanks !
Signature · 2 lines
Damien Le Moal Western Digital Research
Greg Kroah-Hartman
maintainerRe: [PATCH] null_blk: Fix zone size initialization
In reply to Damien Le MoalQuoted 22 lines · Damien Le Moal
commit
0ebcdd702f49aeb0ad2e2d894f8c124a0acc6e23upstream.For a
null_blkdevice with zoned mode enabled is currently initialized with a number of zones equal to the device capacity divided by the zone size, without considering if the device capacity is a multiple of the zone size. If the zone size is not a divisor of the capacity, the zones end up not covering the entire capacity, potentially resulting is out of bounds accesses to the zone array.Fix this by adding one last smaller zone with a size equal to the remainder of the disk capacity divided by the zone size if the capacity is not a multiple of the zone size. For such smaller last zone, the zone capacity is also checked so that it does not exceed the smaller zone size.
Reported-by Naohiro Aota <[email protected]>Fixes ca4b2a011948 ("null_blk: add zone support")Signed-off-by Damien Le Moal <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>Reviewed-by Johannes Thumshirn <[email protected]>Signed-off-by Jens Axboe <[email protected]>
Quoted 3 lines
--- drivers/block/null_blk_zoned.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
That worked, thanks!
greg k-h