* [PATCH 0/4] nvme: a few error-path and validation fixes
@ 2026-07-30 4:31 Guixin Liu
2026-07-30 4:31 ` [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() Guixin Liu
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Guixin Liu @ 2026-07-30 4:31 UTC (permalink / raw)
To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni
Cc: linux-nvme
This series collects four independent fixes found while auditing error
and command-processing paths in the NVMe host and target drivers. Each
is standalone; they only share the same subsystem.
1. nvmet: an Identify CNS 07h (active NS list for a command set)
dereferences req->ns, which is always NULL on that path -> NULL
pointer oops. Also fixes the filter to test the iterated ns->csi.
2. nvmet: nvmet_ns_enable() ignores the percpu_ref_init() return value
and reports success to userspace when it fails, leaving the ns dead.
3. nvme-pci: the per-NUMA-node descriptor DMA pools, created lazily on
admin tag set allocation, are only freed in nvme_remove(); a probe
failure after that point leaks them.
4. nvme: nvme_query_fdp_info() trusts the device-supplied nruhsd count
and can read past the fixed-size RUH status buffer (heap OOB read).
Found by code audit. #1 and #2 are functional/crash fixes; #3 is a
resource leak; #4 is device-triggered hardening. Compile-tested.
Guixin Liu (4):
nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist()
nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable()
nvme-pci: release descriptor pools on probe failure
nvme: clamp FDP placement handle count to the buffer size
drivers/nvme/host/core.c | 1 +
drivers/nvme/host/pci.c | 1 +
drivers/nvme/target/admin-cmd.c | 2 +-
drivers/nvme/target/core.c | 3 ++-
4 files changed, 5 insertions(+), 2 deletions(-)
base-commit: 11028ab62899e4191e074ee364c712b77823a9c4
--
2.43.7
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() 2026-07-30 4:31 [PATCH 0/4] nvme: a few error-path and validation fixes Guixin Liu @ 2026-07-30 4:31 ` Guixin Liu 2026-07-30 8:22 ` Hannes Reinecke ` (2 more replies) 2026-07-30 4:31 ` [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() Guixin Liu ` (2 subsequent siblings) 3 siblings, 3 replies; 21+ messages in thread From: Guixin Liu @ 2026-07-30 4:31 UTC (permalink / raw) To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme When a host issues an Identify command with CNS 07h (Active Namespace ID List for a specific I/O Command Set), nvmet_execute_identify_nslist() is called with match_css set. The command-set filter dereferences req->ns, but this handler never calls nvmet_req_find_ns(), so req->ns is always NULL (nvmet_req_init() resets it to NULL). As soon as an enabled namespace with an NSID greater than the requested value exists, req->ns->csi dereferences a NULL pointer and oopses. Besides the crash, the comparison is logically wrong: to filter the list by command set it must test the command set of the namespace being iterated, not a single fixed value. Use the loop variable ns->csi. Fixes: 61c9967cd634 ("nvmet: implement active command set ns list") Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> --- drivers/nvme/target/admin-cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index 01b799e92ae6..ab6a0a98dd5d 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -958,7 +958,7 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req, bool match_css) nvmet_for_each_enabled_ns(&ctrl->subsys->namespaces, idx, ns) { if (ns->nsid <= min_nsid) continue; - if (match_css && req->ns->csi != req->cmd->identify.csi) + if (match_css && ns->csi != req->cmd->identify.csi) continue; list[i++] = cpu_to_le32(ns->nsid); if (i == buf_size / sizeof(__le32)) -- 2.43.7 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() 2026-07-30 4:31 ` [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() Guixin Liu @ 2026-07-30 8:22 ` Hannes Reinecke 2026-07-30 11:34 ` Christoph Hellwig 2026-07-30 12:06 ` Nilay Shroff 2 siblings, 0 replies; 21+ messages in thread From: Hannes Reinecke @ 2026-07-30 8:22 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme On 7/30/26 6:31 AM, Guixin Liu wrote: > When a host issues an Identify command with CNS 07h (Active Namespace ID > List for a specific I/O Command Set), nvmet_execute_identify_nslist() is > called with match_css set. The command-set filter dereferences req->ns, > but this handler never calls nvmet_req_find_ns(), so req->ns is always > NULL (nvmet_req_init() resets it to NULL). As soon as an enabled > namespace with an NSID greater than the requested value exists, > req->ns->csi dereferences a NULL pointer and oopses. > > Besides the crash, the comparison is logically wrong: to filter the list > by command set it must test the command set of the namespace being > iterated, not a single fixed value. Use the loop variable ns->csi. > > Fixes: 61c9967cd634 ("nvmet: implement active command set ns list") > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> > --- > drivers/nvme/target/admin-cmd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c > index 01b799e92ae6..ab6a0a98dd5d 100644 > --- a/drivers/nvme/target/admin-cmd.c > +++ b/drivers/nvme/target/admin-cmd.c > @@ -958,7 +958,7 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req, bool match_css) > nvmet_for_each_enabled_ns(&ctrl->subsys->namespaces, idx, ns) { > if (ns->nsid <= min_nsid) > continue; > - if (match_css && req->ns->csi != req->cmd->identify.csi) > + if (match_css && ns->csi != req->cmd->identify.csi) > continue; > list[i++] = cpu_to_le32(ns->nsid); > if (i == buf_size / sizeof(__le32)) Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() 2026-07-30 4:31 ` [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() Guixin Liu 2026-07-30 8:22 ` Hannes Reinecke @ 2026-07-30 11:34 ` Christoph Hellwig 2026-07-30 12:06 ` Nilay Shroff 2 siblings, 0 replies; 21+ messages in thread From: Christoph Hellwig @ 2026-07-30 11:34 UTC (permalink / raw) To: Guixin Liu Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni, linux-nvme Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> On Thu, Jul 30, 2026 at 12:31:02PM +0800, Guixin Liu wrote: > When a host issues an Identify command with CNS 07h (Active Namespace ID > List for a specific I/O Command Set), nvmet_execute_identify_nslist() is > called with match_css set. The command-set filter dereferences req->ns, > but this handler never calls nvmet_req_find_ns(), so req->ns is always > NULL (nvmet_req_init() resets it to NULL). As soon as an enabled > namespace with an NSID greater than the requested value exists, > req->ns->csi dereferences a NULL pointer and oopses. Can you submit a testcase for that to blktests? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() 2026-07-30 4:31 ` [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() Guixin Liu 2026-07-30 8:22 ` Hannes Reinecke 2026-07-30 11:34 ` Christoph Hellwig @ 2026-07-30 12:06 ` Nilay Shroff 2 siblings, 0 replies; 21+ messages in thread From: Nilay Shroff @ 2026-07-30 12:06 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Chaitanya Kulkarni Cc: linux-nvme On 7/30/26 10:01 AM, Guixin Liu wrote: > When a host issues an Identify command with CNS 07h (Active Namespace ID > List for a specific I/O Command Set), nvmet_execute_identify_nslist() is > called with match_css set. The command-set filter dereferences req->ns, > but this handler never calls nvmet_req_find_ns(), so req->ns is always > NULL (nvmet_req_init() resets it to NULL). As soon as an enabled > namespace with an NSID greater than the requested value exists, > req->ns->csi dereferences a NULL pointer and oopses. > > Besides the crash, the comparison is logically wrong: to filter the list > by command set it must test the command set of the namespace being > iterated, not a single fixed value. Use the loop variable ns->csi. > > Fixes: 61c9967cd634 ("nvmet: implement active command set ns list") > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> Looks good to me. Reviewed-by: Nilay Shroff <nilay@linux.ibm.com> ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() 2026-07-30 4:31 [PATCH 0/4] nvme: a few error-path and validation fixes Guixin Liu 2026-07-30 4:31 ` [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() Guixin Liu @ 2026-07-30 4:31 ` Guixin Liu 2026-07-30 8:23 ` Hannes Reinecke ` (2 more replies) 2026-07-30 4:31 ` [PATCH 3/4] nvme-pci: release descriptor pools on probe failure Guixin Liu 2026-07-30 4:31 ` [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size Guixin Liu 3 siblings, 3 replies; 21+ messages in thread From: Guixin Liu @ 2026-07-30 4:31 UTC (permalink / raw) To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme The return value of percpu_ref_init() is discarded. At this point ret is 0 from the preceding successful steps, so when the allocation inside percpu_ref_init() fails the code jumps to the out_pr_exit cleanup chain which ends with "return ret", i.e. reports success. The configfs enable store then tells userspace the namespace was enabled even though it was not and its backing device has already been torn down. Capture the return value so the failure is propagated. Fixes: 408232680707 ("nvmet: Fix crash when a namespace is disabled") Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> --- drivers/nvme/target/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 4477c4d6b1ee..c8e01dda4121 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -610,7 +610,8 @@ int nvmet_ns_enable(struct nvmet_ns *ns) goto out_dev_put; } - if (percpu_ref_init(&ns->ref, nvmet_destroy_namespace, 0, GFP_KERNEL)) + ret = percpu_ref_init(&ns->ref, nvmet_destroy_namespace, 0, GFP_KERNEL); + if (ret) goto out_pr_exit; nvmet_ns_changed(subsys, ns->nsid); -- 2.43.7 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() 2026-07-30 4:31 ` [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() Guixin Liu @ 2026-07-30 8:23 ` Hannes Reinecke 2026-07-30 11:38 ` Christoph Hellwig 2026-07-30 12:06 ` Nilay Shroff 2 siblings, 0 replies; 21+ messages in thread From: Hannes Reinecke @ 2026-07-30 8:23 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme On 7/30/26 6:31 AM, Guixin Liu wrote: > The return value of percpu_ref_init() is discarded. At this point ret is > 0 from the preceding successful steps, so when the allocation inside > percpu_ref_init() fails the code jumps to the out_pr_exit cleanup chain > which ends with "return ret", i.e. reports success. The configfs enable > store then tells userspace the namespace was enabled even though it was > not and its backing device has already been torn down. > > Capture the return value so the failure is propagated. > > Fixes: 408232680707 ("nvmet: Fix crash when a namespace is disabled") > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> > --- > drivers/nvme/target/core.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c > index 4477c4d6b1ee..c8e01dda4121 100644 > --- a/drivers/nvme/target/core.c > +++ b/drivers/nvme/target/core.c > @@ -610,7 +610,8 @@ int nvmet_ns_enable(struct nvmet_ns *ns) > goto out_dev_put; > } > > - if (percpu_ref_init(&ns->ref, nvmet_destroy_namespace, 0, GFP_KERNEL)) > + ret = percpu_ref_init(&ns->ref, nvmet_destroy_namespace, 0, GFP_KERNEL); > + if (ret) > goto out_pr_exit; > > nvmet_ns_changed(subsys, ns->nsid); Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() 2026-07-30 4:31 ` [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() Guixin Liu 2026-07-30 8:23 ` Hannes Reinecke @ 2026-07-30 11:38 ` Christoph Hellwig 2026-07-30 12:06 ` Nilay Shroff 2 siblings, 0 replies; 21+ messages in thread From: Christoph Hellwig @ 2026-07-30 11:38 UTC (permalink / raw) To: Guixin Liu Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni, linux-nvme On Thu, Jul 30, 2026 at 12:31:03PM +0800, Guixin Liu wrote: > The return value of percpu_ref_init() is discarded. At this point ret is > 0 from the preceding successful steps, so when the allocation inside > percpu_ref_init() fails the code jumps to the out_pr_exit cleanup chain > which ends with "return ret", i.e. reports success. The configfs enable > store then tells userspace the namespace was enabled even though it was > not and its backing device has already been torn down. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() 2026-07-30 4:31 ` [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() Guixin Liu 2026-07-30 8:23 ` Hannes Reinecke 2026-07-30 11:38 ` Christoph Hellwig @ 2026-07-30 12:06 ` Nilay Shroff 2 siblings, 0 replies; 21+ messages in thread From: Nilay Shroff @ 2026-07-30 12:06 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Chaitanya Kulkarni Cc: linux-nvme On 7/30/26 10:01 AM, Guixin Liu wrote: > The return value of percpu_ref_init() is discarded. At this point ret is > 0 from the preceding successful steps, so when the allocation inside > percpu_ref_init() fails the code jumps to the out_pr_exit cleanup chain > which ends with "return ret", i.e. reports success. The configfs enable > store then tells userspace the namespace was enabled even though it was > not and its backing device has already been torn down. > > Capture the return value so the failure is propagated. > > Fixes: 408232680707 ("nvmet: Fix crash when a namespace is disabled") > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> Looks good to me. Reviewed-by: Nilay Shroff <nilay@linux.ibm.com> ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/4] nvme-pci: release descriptor pools on probe failure 2026-07-30 4:31 [PATCH 0/4] nvme: a few error-path and validation fixes Guixin Liu 2026-07-30 4:31 ` [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() Guixin Liu 2026-07-30 4:31 ` [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() Guixin Liu @ 2026-07-30 4:31 ` Guixin Liu 2026-07-30 8:23 ` Hannes Reinecke ` (3 more replies) 2026-07-30 4:31 ` [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size Guixin Liu 3 siblings, 4 replies; 21+ messages in thread From: Guixin Liu @ 2026-07-30 4:31 UTC (permalink / raw) To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme The per-NUMA-node descriptor DMA pools are created lazily from nvme_init_hctx_common() once the admin tag set is allocated, but they are only destroyed in nvme_remove() via nvme_release_descriptor_pools(). Any probe failure after the admin tag set has been allocated unwinds through the out_disable label and nvme_pci_free_ctrl(), neither of which releases the pools, leaking the dma_pool objects. Release the descriptor pools in the out_disable error path. It must not be added to nvme_pci_free_ctrl(), as that would double-free against nvme_remove() on the normal teardown path. Fixes: d977506f8863 ("nvme-pci: make PRP list DMA pools per-NUMA-node") Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> --- drivers/nvme/host/pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 69932d640b53..f1b24a53aa02 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -3838,6 +3838,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) nvme_dev_remove_admin(dev); nvme_dbbuf_dma_free(dev); nvme_free_queues(dev, 0); + nvme_release_descriptor_pools(dev); out_release_iod_mempool: mempool_destroy(dev->dmavec_mempool); out_dev_unmap: -- 2.43.7 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] nvme-pci: release descriptor pools on probe failure 2026-07-30 4:31 ` [PATCH 3/4] nvme-pci: release descriptor pools on probe failure Guixin Liu @ 2026-07-30 8:23 ` Hannes Reinecke 2026-07-30 11:34 ` Christoph Hellwig ` (2 subsequent siblings) 3 siblings, 0 replies; 21+ messages in thread From: Hannes Reinecke @ 2026-07-30 8:23 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme On 7/30/26 6:31 AM, Guixin Liu wrote: > The per-NUMA-node descriptor DMA pools are created lazily from > nvme_init_hctx_common() once the admin tag set is allocated, but they are > only destroyed in nvme_remove() via nvme_release_descriptor_pools(). Any > probe failure after the admin tag set has been allocated unwinds through > the out_disable label and nvme_pci_free_ctrl(), neither of which releases > the pools, leaking the dma_pool objects. > > Release the descriptor pools in the out_disable error path. It must not > be added to nvme_pci_free_ctrl(), as that would double-free against > nvme_remove() on the normal teardown path. > > Fixes: d977506f8863 ("nvme-pci: make PRP list DMA pools per-NUMA-node") > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> > --- > drivers/nvme/host/pci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index 69932d640b53..f1b24a53aa02 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -3838,6 +3838,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) > nvme_dev_remove_admin(dev); > nvme_dbbuf_dma_free(dev); > nvme_free_queues(dev, 0); > + nvme_release_descriptor_pools(dev); > out_release_iod_mempool: > mempool_destroy(dev->dmavec_mempool); > out_dev_unmap: Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] nvme-pci: release descriptor pools on probe failure 2026-07-30 4:31 ` [PATCH 3/4] nvme-pci: release descriptor pools on probe failure Guixin Liu 2026-07-30 8:23 ` Hannes Reinecke @ 2026-07-30 11:34 ` Christoph Hellwig 2026-07-30 11:49 ` Kanchan Joshi 2026-07-30 12:07 ` Nilay Shroff 3 siblings, 0 replies; 21+ messages in thread From: Christoph Hellwig @ 2026-07-30 11:34 UTC (permalink / raw) To: Guixin Liu Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni, linux-nvme Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] nvme-pci: release descriptor pools on probe failure 2026-07-30 4:31 ` [PATCH 3/4] nvme-pci: release descriptor pools on probe failure Guixin Liu 2026-07-30 8:23 ` Hannes Reinecke 2026-07-30 11:34 ` Christoph Hellwig @ 2026-07-30 11:49 ` Kanchan Joshi 2026-07-30 12:07 ` Nilay Shroff 3 siblings, 0 replies; 21+ messages in thread From: Kanchan Joshi @ 2026-07-30 11:49 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme Reviewed-by: Kanchan Joshi <joshi.k@samsung.com> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] nvme-pci: release descriptor pools on probe failure 2026-07-30 4:31 ` [PATCH 3/4] nvme-pci: release descriptor pools on probe failure Guixin Liu ` (2 preceding siblings ...) 2026-07-30 11:49 ` Kanchan Joshi @ 2026-07-30 12:07 ` Nilay Shroff 3 siblings, 0 replies; 21+ messages in thread From: Nilay Shroff @ 2026-07-30 12:07 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Chaitanya Kulkarni Cc: linux-nvme On 7/30/26 10:01 AM, Guixin Liu wrote: > The per-NUMA-node descriptor DMA pools are created lazily from > nvme_init_hctx_common() once the admin tag set is allocated, but they are > only destroyed in nvme_remove() via nvme_release_descriptor_pools(). Any > probe failure after the admin tag set has been allocated unwinds through > the out_disable label and nvme_pci_free_ctrl(), neither of which releases > the pools, leaking the dma_pool objects. > > Release the descriptor pools in the out_disable error path. It must not > be added to nvme_pci_free_ctrl(), as that would double-free against > nvme_remove() on the normal teardown path. > > Fixes: d977506f8863 ("nvme-pci: make PRP list DMA pools per-NUMA-node") > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> Looks good to me. Reviewed-by: Nilay Shroff <nilay@linux.ibm.com> ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size 2026-07-30 4:31 [PATCH 0/4] nvme: a few error-path and validation fixes Guixin Liu ` (2 preceding siblings ...) 2026-07-30 4:31 ` [PATCH 3/4] nvme-pci: release descriptor pools on probe failure Guixin Liu @ 2026-07-30 4:31 ` Guixin Liu 2026-07-30 8:24 ` Hannes Reinecke ` (3 more replies) 3 siblings, 4 replies; 21+ messages in thread From: Guixin Liu @ 2026-07-30 4:31 UTC (permalink / raw) To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme nvme_query_fdp_info() allocates the RUH status buffer for at most S8_MAX - 1 descriptors and caps the io-mgmt-receive transfer to that size. head->nr_plids, however, is taken verbatim from the device-supplied nruhsd field, which can be up to 65535. If a non-conformant or malicious device reports more descriptors than the buffer holds, the copy loop reads past the end of the ruhs buffer (heap out-of-bounds read). Clamp nr_plids to the number of descriptors the buffer can actually hold. Fixes: 30b5f20bb2dd ("nvme: register fdp parameters with the block layer") Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> --- drivers/nvme/host/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 453c1f0b2dd0..b1f444cd3daf 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2358,6 +2358,7 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info) } head->nr_plids = le16_to_cpu(ruhs->nruhsd); + head->nr_plids = min_t(u16, head->nr_plids, S8_MAX - 1); if (!head->nr_plids) goto free; -- 2.43.7 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size 2026-07-30 4:31 ` [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size Guixin Liu @ 2026-07-30 8:24 ` Hannes Reinecke 2026-07-30 9:40 ` Kanchan Joshi ` (2 subsequent siblings) 3 siblings, 0 replies; 21+ messages in thread From: Hannes Reinecke @ 2026-07-30 8:24 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme On 7/30/26 6:31 AM, Guixin Liu wrote: > nvme_query_fdp_info() allocates the RUH status buffer for at most > S8_MAX - 1 descriptors and caps the io-mgmt-receive transfer to that > size. head->nr_plids, however, is taken verbatim from the device-supplied > nruhsd field, which can be up to 65535. If a non-conformant or malicious > device reports more descriptors than the buffer holds, the copy loop > reads past the end of the ruhs buffer (heap out-of-bounds read). > > Clamp nr_plids to the number of descriptors the buffer can actually hold. > > Fixes: 30b5f20bb2dd ("nvme: register fdp parameters with the block layer") > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> > --- > drivers/nvme/host/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 453c1f0b2dd0..b1f444cd3daf 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -2358,6 +2358,7 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info) > } > > head->nr_plids = le16_to_cpu(ruhs->nruhsd); > + head->nr_plids = min_t(u16, head->nr_plids, S8_MAX - 1); > if (!head->nr_plids) > goto free; > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size 2026-07-30 4:31 ` [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size Guixin Liu 2026-07-30 8:24 ` Hannes Reinecke @ 2026-07-30 9:40 ` Kanchan Joshi 2026-07-30 11:37 ` Christoph Hellwig 2026-07-30 11:36 ` Christoph Hellwig 2026-07-30 12:08 ` Nilay Shroff 3 siblings, 1 reply; 21+ messages in thread From: Kanchan Joshi @ 2026-07-30 9:40 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni Cc: linux-nvme On 7/30/2026 10:01 AM, Guixin Liu wrote: > nvme_query_fdp_info() allocates the RUH status buffer for at most > S8_MAX - 1 descriptors and caps the io-mgmt-receive transfer to that > size. head->nr_plids, however, is taken verbatim from the device-supplied > nruhsd field, which can be up to 65535. If a non-conformant or malicious > device reports more descriptors than the buffer holds, the copy loop > reads past the end of the ruhs buffer (heap out-of-bounds read). > > Clamp nr_plids to the number of descriptors the buffer can actually hold. > > Fixes: 30b5f20bb2dd ("nvme: register fdp parameters with the block layer") > Signed-off-by: Guixin Liu<kanie@linux.alibaba.com> > --- > drivers/nvme/host/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 453c1f0b2dd0..b1f444cd3daf 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -2358,6 +2358,7 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info) > } > > head->nr_plids = le16_to_cpu(ruhs->nruhsd); > + head->nr_plids = min_t(u16, head->nr_plids, S8_MAX - 1); Given what we have at this point, the fix is fine. Reviewed-by: Kanchan Joshi <joshi.k@samsung.com> But I feel the need to improve (what we have) with a follow-up patch: 1. Revise the cap, from S8_MAX - 1 to U8_MAX; since bio->bi_write_hint is u8. 2. And a dev_warn() to know if head->nr_plids go above the kernel-defined cap. It's possible/fine for device to have more placement ids, and if that happens, this can tell. Keith, Christoph - thoughts? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size 2026-07-30 9:40 ` Kanchan Joshi @ 2026-07-30 11:37 ` Christoph Hellwig 2026-07-30 12:41 ` Keith Busch 0 siblings, 1 reply; 21+ messages in thread From: Christoph Hellwig @ 2026-07-30 11:37 UTC (permalink / raw) To: Kanchan Joshi Cc: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni, linux-nvme On Thu, Jul 30, 2026 at 03:10:59PM +0530, Kanchan Joshi wrote: > But I feel the need to improve (what we have) with a follow-up patch: > 1. Revise the cap, from S8_MAX - 1 to U8_MAX; since bio->bi_write_hint > is u8. Signed 8-bit -1 is indeed a really odd value, and from looking at the original commit I'm not really sure how we arrived there. > 2. And a dev_warn() to know if head->nr_plids go above the > kernel-defined cap. It's possible/fine for device to have more placement > ids, and if that happens, this can tell. Yeah. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size 2026-07-30 11:37 ` Christoph Hellwig @ 2026-07-30 12:41 ` Keith Busch 0 siblings, 0 replies; 21+ messages in thread From: Keith Busch @ 2026-07-30 12:41 UTC (permalink / raw) To: Christoph Hellwig Cc: Kanchan Joshi, Guixin Liu, Jens Axboe, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni, linux-nvme On Thu, Jul 30, 2026 at 01:37:22PM +0200, Christoph Hellwig wrote: > On Thu, Jul 30, 2026 at 03:10:59PM +0530, Kanchan Joshi wrote: > > But I feel the need to improve (what we have) with a follow-up patch: > > 1. Revise the cap, from S8_MAX - 1 to U8_MAX; since bio->bi_write_hint > > is u8. > > Signed 8-bit -1 is indeed a really odd value, and from looking at the > original commit I'm not really sure how we arrived there. "S8_MAX - 1" is a weird way to represent it, but the reason that value was used (and it's admittedly not a good reason) was that the result fits in a page. Not that there's any restriction on admin commands using PRP lists; it's just a coincidental limit for all the driver initiated admin commands. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size 2026-07-30 4:31 ` [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size Guixin Liu 2026-07-30 8:24 ` Hannes Reinecke 2026-07-30 9:40 ` Kanchan Joshi @ 2026-07-30 11:36 ` Christoph Hellwig 2026-07-30 12:08 ` Nilay Shroff 3 siblings, 0 replies; 21+ messages in thread From: Christoph Hellwig @ 2026-07-30 11:36 UTC (permalink / raw) To: Guixin Liu Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Nilay Shroff, Chaitanya Kulkarni, linux-nvme On Thu, Jul 30, 2026 at 12:31:05PM +0800, Guixin Liu wrote: > nvme_query_fdp_info() allocates the RUH status buffer for at most > S8_MAX - 1 descriptors and caps the io-mgmt-receive transfer to that > size. head->nr_plids, however, is taken verbatim from the device-supplied > nruhsd field, which can be up to 65535. If a non-conformant or malicious > device reports more descriptors than the buffer holds, the copy loop > reads past the end of the ruhs buffer (heap out-of-bounds read). > > Clamp nr_plids to the number of descriptors the buffer can actually hold. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size 2026-07-30 4:31 ` [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size Guixin Liu ` (2 preceding siblings ...) 2026-07-30 11:36 ` Christoph Hellwig @ 2026-07-30 12:08 ` Nilay Shroff 3 siblings, 0 replies; 21+ messages in thread From: Nilay Shroff @ 2026-07-30 12:08 UTC (permalink / raw) To: Guixin Liu, Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Hannes Reinecke, Chaitanya Kulkarni Cc: linux-nvme On 7/30/26 10:01 AM, Guixin Liu wrote: > nvme_query_fdp_info() allocates the RUH status buffer for at most > S8_MAX - 1 descriptors and caps the io-mgmt-receive transfer to that > size. head->nr_plids, however, is taken verbatim from the device-supplied > nruhsd field, which can be up to 65535. If a non-conformant or malicious > device reports more descriptors than the buffer holds, the copy loop > reads past the end of the ruhs buffer (heap out-of-bounds read). > > Clamp nr_plids to the number of descriptors the buffer can actually hold. > > Fixes: 30b5f20bb2dd ("nvme: register fdp parameters with the block layer") > Signed-off-by: Guixin Liu<kanie@linux.alibaba.com> Looks good to me. Reviewed-by: Nilay Shroff <nilay@linux.ibm.com> ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2026-07-30 12:41 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-30 4:31 [PATCH 0/4] nvme: a few error-path and validation fixes Guixin Liu 2026-07-30 4:31 ` [PATCH 1/4] nvmet: fix NULL pointer dereference in nvmet_execute_identify_nslist() Guixin Liu 2026-07-30 8:22 ` Hannes Reinecke 2026-07-30 11:34 ` Christoph Hellwig 2026-07-30 12:06 ` Nilay Shroff 2026-07-30 4:31 ` [PATCH 2/4] nvmet: propagate percpu_ref_init() failure in nvmet_ns_enable() Guixin Liu 2026-07-30 8:23 ` Hannes Reinecke 2026-07-30 11:38 ` Christoph Hellwig 2026-07-30 12:06 ` Nilay Shroff 2026-07-30 4:31 ` [PATCH 3/4] nvme-pci: release descriptor pools on probe failure Guixin Liu 2026-07-30 8:23 ` Hannes Reinecke 2026-07-30 11:34 ` Christoph Hellwig 2026-07-30 11:49 ` Kanchan Joshi 2026-07-30 12:07 ` Nilay Shroff 2026-07-30 4:31 ` [PATCH 4/4] nvme: clamp FDP placement handle count to the buffer size Guixin Liu 2026-07-30 8:24 ` Hannes Reinecke 2026-07-30 9:40 ` Kanchan Joshi 2026-07-30 11:37 ` Christoph Hellwig 2026-07-30 12:41 ` Keith Busch 2026-07-30 11:36 ` Christoph Hellwig 2026-07-30 12:08 ` Nilay Shroff
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.