[PATCH v2] nvme: skip the zoned limits update if the zone info query failed

[PATCH v2] nvme: skip the zoned limits update if the zone info query failed

[Chao Shi]

nvme_query_zone_info() returns either a negative errno or a positive
NVMe status code, but nvme_update_ns_info_block() only tests for the
negative case:

	ret = nvme_query_zone_info(ns, lbaf, &zi);
	if (ret < 0)
		goto out;

If the device fails the Identify Namespace (I/O Command Set specific)
command, or the Identify Controller command issued by
nvme_set_max_append(), the positive status falls through and setup
continues with the zero-initialized zone info.  nvme_update_zone_info()
then marks the queue zoned with chunk_sectors and ns->head->zsze set to
zero.

blk_validate_zoned_limits() does not check chunk_sectors, so the limits
commit succeeds.  blk_revalidate_disk_zones() does reject the zero zone
size, but by then the limits are live and nothing rolls them back, so
I/O keeps being submitted to a zoned queue with a zero zone size and
disk_zone_no() shifts by ilog2(0):

  nvme0n1: Invalid non power of two zone size (0)
  UBSAN: shift-out-of-bounds in include/linux/blkdev.h:747:16
  shift exponent -1 is negative
   disk_zone_no include/linux/blkdev.h:747 [inline]
   bio_straddles_zones include/linux/blkdev.h:1058 [inline]
   blk_zone_wplug_handle_write block/blk-zoned.c:1423 [inline]
   blk_zone_plug_bio.cold+0x25/0x1c8 block/blk-zoned.c:1605
   blk_mq_submit_bio+0x18fb/0x2870 block/blk-mq.c:3196
   submit_bh_wbc+0x575/0x740 fs/buffer.c:2824
   __block_write_full_folio+0x728/0xdd0 fs/buffer.c:1933

Any device, firmware or NVMe-oF target that fails this one command
reaches this.

Skip the zoned limits update in that case.  The namespace stays
registered and usable for admin commands, but the queue is not
configured from zone info that was never read.

zi.zone_size is an exact indicator: every path that returns a positive
status returns before it is assigned, and after that the only failure
left is -ENODEV, which the caller already handles.

Fixes: c85c9ab926a5 ("nvme: split nvme_update_zone_info")
Cc: stable@vger.kernel.org
Cc: Weidong Zhu <weizhu@fiu.edu>
Suggested-by: Keith Busch <kbusch@kernel.org>
Found by FuzzNvme.

Signed-off-by: Chao Shi <coshi036@gmail.com>
---
Changes since v1:
- Only skip the zoned limits instead of failing the update, as
  suggested by Keith.
- Gate on zi.zone_size, not zi.max_open_zones, where 0 is legal
  (reasoning in my reply on v1).
- Drop the "malicious device" wording.

v1: https://lore.kernel.org/linux-nvme/20260814160954.2839507-1-coshi036@gmail.com/

 drivers/nvme/host/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 453c1f0b2dd0..87e0534cde1c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2447,8 +2447,13 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
 	if (!nvme_update_disk_info(ns, id, nvm, &lim))
 		capacity = 0;
 
+	/*
+	 * A failed zone info query leaves zi zero-initialized.  Leave the
+	 * namespace registered so that it can still be used as a device
+	 * handle, but do not configure the zoned limits from it.
+	 */
 	if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) &&
-	    ns->head->ids.csi == NVME_CSI_ZNS)
+	    ns->head->ids.csi == NVME_CSI_ZNS && zi.zone_size)
 		nvme_update_zone_info(ns, &lim, &zi);
 
 	if ((ns->ctrl->vwc & NVME_CTRL_VWC_PRESENT) && !info->no_vwc)
-- 
2.43.0

Re: [PATCH v2] nvme: skip the zoned limits update if the zone info query failed

[Damien Le Moal]

On 8/17/26 04:17, Chao Shi wrote:
> nvme_query_zone_info() returns either a negative errno or a positive
> NVMe status code, but nvme_update_ns_info_block() only tests for the

That seems like a bad design of nvme_query_zone_info(). Why not fix it so that
on error it returns either negative values OR NVMe status code? That would
avoid the pitfall of the error check that you found.

> negative case:
> 
> 	ret = nvme_query_zone_info(ns, lbaf, &zi);
> 	if (ret < 0)
> 		goto out;
> 
> If the device fails the Identify Namespace (I/O Command Set specific)
> command, or the Identify Controller command issued by
> nvme_set_max_append(), the positive status falls through and setup
> continues with the zero-initialized zone info.  nvme_update_zone_info()
> then marks the queue zoned with chunk_sectors and ns->head->zsze set to
> zero.
> 
> blk_validate_zoned_limits() does not check chunk_sectors, so the limits
> commit succeeds.  blk_revalidate_disk_zones() does reject the zero zone
> size, but by then the limits are live and nothing rolls them back, so
> I/O keeps being submitted to a zoned queue with a zero zone size and
> disk_zone_no() shifts by ilog2(0):
> 
>   nvme0n1: Invalid non power of two zone size (0)
>   UBSAN: shift-out-of-bounds in include/linux/blkdev.h:747:16
>   shift exponent -1 is negative
>    disk_zone_no include/linux/blkdev.h:747 [inline]
>    bio_straddles_zones include/linux/blkdev.h:1058 [inline]
>    blk_zone_wplug_handle_write block/blk-zoned.c:1423 [inline]
>    blk_zone_plug_bio.cold+0x25/0x1c8 block/blk-zoned.c:1605
>    blk_mq_submit_bio+0x18fb/0x2870 block/blk-mq.c:3196
>    submit_bh_wbc+0x575/0x740 fs/buffer.c:2824
>    __block_write_full_folio+0x728/0xdd0 fs/buffer.c:1933
> 
> Any device, firmware or NVMe-oF target that fails this one command
> reaches this.
> 
> Skip the zoned limits update in that case.  The namespace stays
> registered and usable for admin commands, but the queue is not
> configured from zone info that was never read.
> 
> zi.zone_size is an exact indicator: every path that returns a positive
> status returns before it is assigned, and after that the only failure
> left is -ENODEV, which the caller already handles.
> 
> Fixes: c85c9ab926a5 ("nvme: split nvme_update_zone_info")
> Cc: stable@vger.kernel.org
> Cc: Weidong Zhu <weizhu@fiu.edu>
> Suggested-by: Keith Busch <kbusch@kernel.org>
> Found by FuzzNvme.
> 
> Signed-off-by: Chao Shi <coshi036@gmail.com>
> ---
> Changes since v1:
> - Only skip the zoned limits instead of failing the update, as
>   suggested by Keith.
> - Gate on zi.zone_size, not zi.max_open_zones, where 0 is legal
>   (reasoning in my reply on v1).
> - Drop the "malicious device" wording.
> 
> v1: https://lore.kernel.org/linux-nvme/20260814160954.2839507-1-coshi036@gmail.com/
> 
>  drivers/nvme/host/core.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 453c1f0b2dd0..87e0534cde1c 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2447,8 +2447,13 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
>  	if (!nvme_update_disk_info(ns, id, nvm, &lim))
>  		capacity = 0;
>  
> +	/*
> +	 * A failed zone info query leaves zi zero-initialized.  Leave the
> +	 * namespace registered so that it can still be used as a device
> +	 * handle, but do not configure the zoned limits from it.
> +	 */
>  	if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) &&
> -	    ns->head->ids.csi == NVME_CSI_ZNS)
> +	    ns->head->ids.csi == NVME_CSI_ZNS && zi.zone_size)
>  		nvme_update_zone_info(ns, &lim, &zi);
>  
>  	if ((ns->ctrl->vwc & NVME_CTRL_VWC_PRESENT) && !info->no_vwc)


-- 
Damien Le Moal
Western Digital Research