The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2] nvme: skip the zoned limits update if the zone info query failed
@ 2026-08-16 19:17 Chao Shi
  2026-08-17  6:17 ` Damien Le Moal
  2026-08-17  6:55 ` Christoph Hellwig
  0 siblings, 2 replies; 4+ messages in thread
From: Chao Shi @ 2026-08-16 19:17 UTC (permalink / raw)
  To: kbusch
  Cc: hch, sagi, axboe, joshi.k, linux-nvme, linux-kernel, stable,
	Chao Shi, Weidong Zhu

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nvme: skip the zoned limits update if the zone info query failed
  2026-08-16 19:17 [PATCH v2] nvme: skip the zoned limits update if the zone info query failed Chao Shi
@ 2026-08-17  6:17 ` Damien Le Moal
  2026-08-17  6:53   ` Christoph Hellwig
  2026-08-17  6:55 ` Christoph Hellwig
  1 sibling, 1 reply; 4+ messages in thread
From: Damien Le Moal @ 2026-08-17  6:17 UTC (permalink / raw)
  To: Chao Shi, kbusch
  Cc: hch, sagi, axboe, joshi.k, linux-nvme, linux-kernel, stable,
	Weidong Zhu

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nvme: skip the zoned limits update if the zone info query failed
  2026-08-17  6:17 ` Damien Le Moal
@ 2026-08-17  6:53   ` Christoph Hellwig
  0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2026-08-17  6:53 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Chao Shi, kbusch, hch, sagi, axboe, joshi.k, linux-nvme,
	linux-kernel, stable, Weidong Zhu

On Mon, Aug 17, 2026 at 03:17:22PM +0900, Damien Le Moal wrote:
> 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.

Because in some case the difference matters, and this thus is a common
pattern in the nvme driver.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nvme: skip the zoned limits update if the zone info query failed
  2026-08-16 19:17 [PATCH v2] nvme: skip the zoned limits update if the zone info query failed Chao Shi
  2026-08-17  6:17 ` Damien Le Moal
@ 2026-08-17  6:55 ` Christoph Hellwig
  1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2026-08-17  6:55 UTC (permalink / raw)
  To: Chao Shi
  Cc: kbusch, hch, sagi, axboe, joshi.k, linux-nvme, linux-kernel,
	stable, Weidong Zhu

On Sun, Aug 16, 2026 at 03:17:29PM -0400, 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
> 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):

Did you do error injections to get here?

> +	/*
> +	 * 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);

The most common case for this is probably during revalidation of some
kind, and then we'll just keep the old values.  Which in general
should be fine, but might be worth mentioning in this comment?

Also we should probably log a message that we either just register
the handle or keep the old values?


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-17  6:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-16 19:17 [PATCH v2] nvme: skip the zoned limits update if the zone info query failed Chao Shi
2026-08-17  6:17 ` Damien Le Moal
2026-08-17  6:53   ` Christoph Hellwig
2026-08-17  6:55 ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox