* [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; 6+ 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 [flat|nested] 6+ 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 17:41 ` Chris S
2026-08-17 6:55 ` Christoph Hellwig
1 sibling, 2 replies; 6+ 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] 6+ 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
2026-08-17 17:41 ` Chris S
1 sibling, 0 replies; 6+ 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] 6+ 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
@ 2026-08-17 17:41 ` Chris S
1 sibling, 0 replies; 6+ messages in thread
From: Chris S @ 2026-08-17 17:41 UTC (permalink / raw)
To: Damien Le Moal
Cc: kbusch, hch, sagi, axboe, joshi.k, linux-nvme, linux-kernel,
stable, Weidong Zhu
Hi Damien,
Thanks for taking a look.
Christoph already answered this, and I agree with him: the split return
is deliberate and a common pattern in the nvme driver, because some
callers do need to tell a controller-reported status apart from a local
errno. This function is one of them: it opens with "ret =
nvme_identify_ns(...); if (ret) return ret;", and nvme_validate_ns()
then only removes the namespace on "ret > 0 && (ret & NVME_STATUS_DNR)",
which it could not do if the status had been folded into an errno.
So I kept the fix at the call site rather than changing the calling
convention.
Thanks,
Chao
^ permalink raw reply [flat|nested] 6+ 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
2026-08-17 17:46 ` Chris S
1 sibling, 1 reply; 6+ 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] 6+ messages in thread
* Re: [PATCH v2] nvme: skip the zoned limits update if the zone info query failed
2026-08-17 6:55 ` Christoph Hellwig
@ 2026-08-17 17:46 ` Chris S
0 siblings, 0 replies; 6+ messages in thread
From: Chris S @ 2026-08-17 17:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: kbusch, sagi, axboe, joshi.k, linux-nvme, linux-kernel, stable,
Weidong Zhu
Hi Christoph,
Thanks for the review.
> Did you do error injections to get here?
Yes, on the device side rather than in the kernel. The splat comes
from a fuzzing setup where an emulated NVMe controller rewrites
completion status codes. In the wire log of the run that crashed, the
Identify with CNS 05h and CSI ZNS -- the one nvme_query_zone_info()
issues -- appears twice: it completed successfully at boot, and on a
later rescan the same command was completed with SCT 0 / SC 24h
instead. That positive status is what falls through the "ret < 0"
check.
> 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?
Yes, that is what happens. queue_limits_start_update() hands back a
copy of the current limits, and nvme_set_chunk_sectors() returns early
for an already zoned queue, so skipping the update leaves the
previously validated zone geometry in place. I will spell that out in
the comment.
> Also we should probably log a message that we either just register
> the handle or keep the old values?
Will do in v3. blk_queue_is_zoned() on the not yet committed queue
tells the two cases apart, so the message can say which one it is. I
was going to use dev_warn() to match the neighbouring zone code -- tell
me if you would rather have dev_warn_once(), since this runs on every
revalidation and a device that keeps failing the command would repeat
it.
Thanks,
Chao
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-17 17:46 UTC | newest]
Thread overview: 6+ 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 17:41 ` Chris S
2026-08-17 6:55 ` Christoph Hellwig
2026-08-17 17:46 ` Chris S
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®