mbox series

[v3,0/5] Improve checks in blk_revalidate_disk_zones()

Message ID 20230703024812.76778-1-dlemoal@kernel.org
Headers show
Series Improve checks in blk_revalidate_disk_zones() | expand

Message

Damien Le Moal July 3, 2023, 2:48 a.m. UTC
blk_revalidate_disk_zones() implements checks of the zones of a zoned
block device, verifying that the zone size is a power of 2 number of
sectors, that all zones (except possibly the last one) have the same
size and that zones cover the entire addressing space of the device.

While these checks are appropriate to verify that well tested hardware
devices have an adequate zone configurations, they lack in certain areas
which may result in issues with potentially buggy emulated devices
implemented with user drivers such as ublk or tcmu. Specifically, this
function does not check if the device driver indicated support for the
mandatory zone append writes, that is, if the device
max_zone_append_sectors queue limit is set to a non-zero value.
Additionally, invalid zones such as a zero length zone with a start
sector equal to the device capacity will not be detected and result in
out of bounds use of the zone bitmaps prepared with the callback
function blk_revalidate_zone_cb().

This series address these issues by modifying the 4 block device drivers
that currently support zoned block devices to ensure that they all set a
zoned device zone size and max zone append sectors limit before
executing blk_revalidate_disk_zones(). With these changes in place, 
patch 5 improves blk_revalidate_disk_zones() to address the missing
checks, relying on the fact that the zone size and zone append limit are
normally set when this function is called.

Changes from v2:
 - Rebased on current Linus tree to fix a conflict with scsi changes in
   patch 1.
 - Added Reviewed-by tags.

Changes from v1:
 - Updated this cover letter and commit messages to include better
   explain for these patches
 - Reworked patch 5 to simplify the checks
  
Damien Le Moal (5):
  scsi: sd_zbc: Set zone limits before revalidating zones
  nvme: zns: Set zone limits before revalidating zones
  block: nullblk: Set zone limits before revalidating zones
  block: virtio_blk: Set zone limits before revalidating zones
  block: improve checks in blk_revalidate_disk_zones()

 block/blk-zoned.c              | 86 ++++++++++++++++++++--------------
 drivers/block/null_blk/zoned.c | 16 ++-----
 drivers/block/virtio_blk.c     | 34 ++++++--------
 drivers/nvme/host/zns.c        |  9 ++--
 drivers/scsi/sd_zbc.c          | 12 ++---
 5 files changed, 79 insertions(+), 78 deletions(-)

Comments

Johannes Thumshirn July 3, 2023, 7:23 a.m. UTC | #1
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Johannes Thumshirn July 3, 2023, 7:24 a.m. UTC | #2
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Johannes Thumshirn July 3, 2023, 7:26 a.m. UTC | #3
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Martin K. Petersen July 6, 2023, 2:04 a.m. UTC | #4
Damien,

> blk_revalidate_disk_zones() implements checks of the zones of a zoned
> block device, verifying that the zone size is a power of 2 number of
> sectors, that all zones (except possibly the last one) have the same
> size and that zones cover the entire addressing space of the device.

Applied to 6.5/scsi-staging, thanks!
Martin K. Petersen July 11, 2023, 4:31 p.m. UTC | #5
On Mon, 03 Jul 2023 11:48:07 +0900, Damien Le Moal wrote:

> blk_revalidate_disk_zones() implements checks of the zones of a zoned
> block device, verifying that the zone size is a power of 2 number of
> sectors, that all zones (except possibly the last one) have the same
> size and that zones cover the entire addressing space of the device.
> 
> While these checks are appropriate to verify that well tested hardware
> devices have an adequate zone configurations, they lack in certain areas
> which may result in issues with potentially buggy emulated devices
> implemented with user drivers such as ublk or tcmu. Specifically, this
> function does not check if the device driver indicated support for the
> mandatory zone append writes, that is, if the device
> max_zone_append_sectors queue limit is set to a non-zero value.
> Additionally, invalid zones such as a zero length zone with a start
> sector equal to the device capacity will not be detected and result in
> out of bounds use of the zone bitmaps prepared with the callback
> function blk_revalidate_zone_cb().
> 
> [...]

Applied to 6.5/scsi-fixes, thanks!

[1/5] scsi: sd_zbc: Set zone limits before revalidating zones
      https://git.kernel.org/mkp/scsi/c/f79846ca2f04
[2/5] nvme: zns: Set zone limits before revalidating zones
      https://git.kernel.org/mkp/scsi/c/d226b0a2b683
[3/5] block: nullblk: Set zone limits before revalidating zones
      https://git.kernel.org/mkp/scsi/c/a442b899fe17
[4/5] block: virtio_blk: Set zone limits before revalidating zones
      https://git.kernel.org/mkp/scsi/c/a3d96ed21507
[5/5] block: improve checks in blk_revalidate_disk_zones()
      https://git.kernel.org/mkp/scsi/c/03e51c4a74b9