[PATCH v3 0/2] nvme: fix FDP configuration log parsing

liuxixin posted 2 patches 1 week, 4 days ago
There is a newer version of this series
[PATCH v3 0/2] nvme: fix FDP configuration log parsing
Posted by liuxixin 1 week, 4 days ago
Hi Christoph, Keith, Nitesh,

Thanks for the reviews on v2.

v3 splits the series as Christoph suggested:
  1/2 - fdpcidx bounds check (>= instead of >), with Fixes tag
  2/2 - validate descriptor sizes while walking the log

1/2 also applies Keith's suggestion to use >= while keeping n = NUMFDPC + 1.
Uses a 12-character Fixes commit id per Christoph's feedback, and includes
Nitesh's Reviewed-by tags on both patches.

## Test plan
- Build: make M=drivers/nvme -j12 (linux-next, verified)

- QEMU (fdp-lab, tested as one patch before this split): linux-next 7.1-rc4,
  QEMU 8.2 nvme-subsys,fdp=on; fdpcidx=1 / NUMFDPC 0 - unfixed: invalid
  descriptor list; fixed: FDP index:1 out of range:1.  v3 splits commits only;
  no functional change, no re-test.

Thanks,
liuxixin
[PATCH v4 0/2] nvme: fix FDP configuration log parsing
Posted by liuxixin 1 week, 4 days ago
Hi Christoph, Keith, Nitesh,

v4 is a re-spin of v3 with no functional changes.  Fixes the garbled
mail format on v3 and adds Christoph's Reviewed-by tags on both patches
(v3 already had Nitesh's Reviewed-by).

## Test plan
- Build: make M=drivers/nvme -j12 (linux-next, verified)

- QEMU (fdp-lab, tested as one patch before this split): linux-next 7.1-rc4,
  QEMU 8.2 nvme-subsys,fdp=on; fdpcidx=1 / NUMFDPC 0 - unfixed: invalid
  descriptor list; fixed: FDP index:1 out of range:1.  v4 is tag/format
  only; no re-test.

Thanks,
liuxixin
[PATCH v4 1/2] nvme: fix FDP fdpcidx bounds check
Posted by liuxixin 1 week, 4 days ago
The fdpcidx bounds check sets n = NUMFDPC + 1 but used > instead of >=,
incorrectly accepting fdp_idx when it equals n (i.e. NUMFDPC + 1).

Fixes: 30b5f20bb2dd ("nvme: register fdp parameters with the block layer")
Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: liuxixin <gliuxen@gmail.com>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c3032d6ad..766157ba6 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2263,7 +2263,7 @@ static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl,
 	}
 
 	n = le16_to_cpu(h->numfdpc) + 1;
-	if (fdp_idx > n) {
+	if (fdp_idx >= n) {
 		dev_warn(ctrl->device, "FDP index:%d out of range:%d\n",
 			 fdp_idx, n);
 		/* Proceed without registering FDP streams */
-- 
2.43.0
Re: [PATCH v4 1/2] nvme: fix FDP fdpcidx bounds check
Posted by Keith Busch 6 days, 3 hours ago
On Thu, May 28, 2026 at 06:00:01PM +0800, liuxixin wrote:
> The fdpcidx bounds check sets n = NUMFDPC + 1 but used > instead of >=,
> incorrectly accepting fdp_idx when it equals n (i.e. NUMFDPC + 1).

Thanks, applied patch 1 to nvme-7.2.

Take a look at my omment on patch 2 and let me know if we should wait for
a new version for that one.
[PATCH v4 2/2] nvme: validate FDP configuration descriptor sizes
Posted by liuxixin 1 week, 4 days ago
Validate descriptor sizes while walking the FDP configurations log so
dsze == 0 or a descriptor past the log end cannot cause unbounded
iteration or reads past the buffer.

Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: liuxixin <gliuxen@gmail.com>
---
 drivers/nvme/host/core.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 766157ba6..40e87b563 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2275,7 +2275,15 @@ static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl,
 	desc = log;
 	end = log + size - sizeof(*h);
 	for (i = 0; i < fdp_idx; i++) {
-		log += le16_to_cpu(desc->dsze);
+		u16 dsze = le16_to_cpu(desc->dsze);
+
+		if (!dsze || log + dsze > end) {
+			dev_warn(ctrl->device,
+				 "FDP invalid config descriptor at index %d\n", i);
+			ret = 0;
+			goto out;
+		}
+		log += dsze;
 		desc = log;
 		if (log >= end) {
 			dev_warn(ctrl->device,
-- 
2.43.0
Re: [PATCH v4 2/2] nvme: validate FDP configuration descriptor sizes
Posted by Keith Busch 6 days, 3 hours ago
On Thu, May 28, 2026 at 06:00:02PM +0800, liuxixin wrote:
> @@ -2275,7 +2275,15 @@ static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl,
>  	desc = log;
>  	end = log + size - sizeof(*h);
>  	for (i = 0; i < fdp_idx; i++) {
> -		log += le16_to_cpu(desc->dsze);
> +		u16 dsze = le16_to_cpu(desc->dsze);
> +
> +		if (!dsze || log + dsze > end) {
> +			dev_warn(ctrl->device,
> +				 "FDP invalid config descriptor at index %d\n", i);
> +			ret = 0;
> +			goto out;
> +		}
> +		log += dsze;
>  		desc = log;
>  		if (log >= end) {
>  			dev_warn(ctrl->device,

I think you can delete this "log >= end" check now that you added the
same check right above.
[PATCH v5 0/1] nvme: validate FDP configuration descriptor sizes
Posted by liuxixin 6 days, 1 hour ago
Hi Keith,

Thanks for applying v4 1/2 to nvme-7.2.

v5 is only the descriptor-size validation patch, with your feedback on
v4 2/2: remove the redundant "log >= end" check inside the walk loop
now that dsze is validated before advancing.

## Test plan
- Build: make M=drivers/nvme -j$(nproc) (linux-next)
- fdp-lab: dsze==0 / walk past end -> "FDP invalid config descriptor at
  index %d" (see fdp-lab/TEST-FDP-BOUNDS.md)

Thanks,
liuxixin
[PATCH v5 1/1] nvme: validate FDP configuration descriptor sizes
Posted by liuxixin 6 days, 1 hour ago
Validate descriptor sizes while walking the FDP configurations log so
dsze == 0 or a descriptor past the log end cannot cause unbounded
iteration or reads past the buffer.

Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: liuxixin <gliuxen@gmail.com>
---
 drivers/nvme/host/core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 766157ba6..48633a8bb 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2275,14 +2275,16 @@ static int nvme_query_fdp_granularity(struct nvme_ctrl *ctrl,
 	desc = log;
 	end = log + size - sizeof(*h);
 	for (i = 0; i < fdp_idx; i++) {
-		log += le16_to_cpu(desc->dsze);
-		desc = log;
-		if (log >= end) {
+		u16 dsze = le16_to_cpu(desc->dsze);
+
+		if (!dsze || log + dsze > end) {
 			dev_warn(ctrl->device,
-				 "FDP invalid config descriptor list\n");
+				 "FDP invalid config descriptor at index %d\n", i);
 			ret = 0;
 			goto out;
 		}
+		log += dsze;
+		desc = log;
 	}
 
 	if (le32_to_cpu(desc->nrg) > 1) {
-- 
2.43.0
Re: [PATCH v5 1/1] nvme: validate FDP configuration descriptor sizes
Posted by Keith Busch 5 days, 6 hours ago
On Tue, Jun 02, 2026 at 10:00:01PM +0800, liuxixin wrote:
> Validate descriptor sizes while walking the FDP configurations log so
> dsze == 0 or a descriptor past the log end cannot cause unbounded
> iteration or reads past the buffer.

Thanks, applied to nvme-7.2.