All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] libnvdimm/labels: fix the nslot product overflow and cap the slot count
@ 2026-06-24  4:19 ` Bryam Vargas
  0 siblings, 0 replies; 8+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-24  4:19 UTC (permalink / raw)
  To: Ira Weiny, Vishal Verma, Dan Williams, Dave Jiang
  Cc: David Laight, Alison Schofield, nvdimm, linux-kernel

The on-media namespace index nslot is a u32 read from the DIMM label area,
or written from userspace via ND_CMD_SET_CONFIG_DATA.  __nd_label_validate()
bounds nslot against config_size, but the product nslot * label_size is
evaluated in 32 bits and wraps, so a crafted nslot passes the check and then
drives an out-of-bounds memset in nd_label_data_init().

Patch 1 evaluates the product in 64 bits so the bound is exact; it is the
targeted fix, tagged for stable.  Patch 2 caps nslot, so a bogus
firmware-reported config_size cannot admit a large slot count -- and the large
allocation it implies -- even after the product is computed correctly.

The sibling multiply in sizeof_namespace_index() derives nslot from config_size
via nvdimm_num_label_slots(), not the on-media field, so it cannot overflow and
is left unchanged.

Verified on -m64 and -m32: the 64-bit bound agrees with an exact divide-based
check on the boundary and on randomized inputs, and the cap rejects every
wrapping one.  The same crafted nslot is an out-of-bounds write on -m32 but not
on -m64 before the fix.  An out-of-tree module mirroring nd_label_data_init()
reproduces the KASAN slab-out-of-bounds write unpatched and is clean with
either patch; harness available on request.

A negative ndctl test (test/label-compat.sh, oversize nslot) covering both
patches will follow separately, per Alison's suggestion.

v1 (single patch):
https://lore.kernel.org/all/20260620-b4-disp-7f43b155-v1-1-0cfd8017f7a0@proton.me/
v2: split the exact fix and the cap into two patches per review; the
    Reviewed-by and Suggested-by are recorded on the respective patches.

Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Bryam Vargas (2):
      libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
      libnvdimm/labels: reject an implausibly large on-media slot count

 drivers/nvdimm/label.c | 6 +++++-
 drivers/nvdimm/label.h | 7 +++++++
 2 files changed, 12 insertions(+), 1 deletion(-)
---
base-commit: 502d801f0ab03e4f32f9a33d203154ce84887921
change-id: 20260623-b4-disp-1f2c537a-50ca829a9ea6

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>



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

* [PATCH v2 0/2] libnvdimm/labels: fix the nslot product overflow and cap the slot count
@ 2026-06-24  4:19 ` Bryam Vargas
  0 siblings, 0 replies; 8+ messages in thread
From: Bryam Vargas @ 2026-06-24  4:19 UTC (permalink / raw)
  To: Ira Weiny, Vishal Verma, Dan Williams, Dave Jiang
  Cc: David Laight, Alison Schofield, nvdimm, linux-kernel

The on-media namespace index nslot is a u32 read from the DIMM label area,
or written from userspace via ND_CMD_SET_CONFIG_DATA.  __nd_label_validate()
bounds nslot against config_size, but the product nslot * label_size is
evaluated in 32 bits and wraps, so a crafted nslot passes the check and then
drives an out-of-bounds memset in nd_label_data_init().

Patch 1 evaluates the product in 64 bits so the bound is exact; it is the
targeted fix, tagged for stable.  Patch 2 caps nslot, so a bogus
firmware-reported config_size cannot admit a large slot count -- and the large
allocation it implies -- even after the product is computed correctly.

The sibling multiply in sizeof_namespace_index() derives nslot from config_size
via nvdimm_num_label_slots(), not the on-media field, so it cannot overflow and
is left unchanged.

Verified on -m64 and -m32: the 64-bit bound agrees with an exact divide-based
check on the boundary and on randomized inputs, and the cap rejects every
wrapping one.  The same crafted nslot is an out-of-bounds write on -m32 but not
on -m64 before the fix.  An out-of-tree module mirroring nd_label_data_init()
reproduces the KASAN slab-out-of-bounds write unpatched and is clean with
either patch; harness available on request.

A negative ndctl test (test/label-compat.sh, oversize nslot) covering both
patches will follow separately, per Alison's suggestion.

v1 (single patch):
https://lore.kernel.org/all/20260620-b4-disp-7f43b155-v1-1-0cfd8017f7a0@proton.me/
v2: split the exact fix and the cap into two patches per review; the
    Reviewed-by and Suggested-by are recorded on the respective patches.

Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Bryam Vargas (2):
      libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
      libnvdimm/labels: reject an implausibly large on-media slot count

 drivers/nvdimm/label.c | 6 +++++-
 drivers/nvdimm/label.h | 7 +++++++
 2 files changed, 12 insertions(+), 1 deletion(-)
---
base-commit: 502d801f0ab03e4f32f9a33d203154ce84887921
change-id: 20260623-b4-disp-1f2c537a-50ca829a9ea6

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>


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

* [PATCH v2 1/2] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
  2026-06-24  4:19 ` Bryam Vargas
@ 2026-06-24  4:19   ` Bryam Vargas
  -1 siblings, 0 replies; 8+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-24  4:19 UTC (permalink / raw)
  To: Ira Weiny, Vishal Verma, Dan Williams, Dave Jiang
  Cc: David Laight, Alison Schofield, nvdimm, linux-kernel

From: Bryam Vargas <hexlabsecurity@proton.me>

The on-media namespace index field nslot is a u32 read from the DIMM
label storage area.  __nd_label_validate() bounds it against the config
area size, but sizeof_namespace_label() returns unsigned, so the product
nslot * label_size is evaluated in 32-bit and wraps modulo 2^32 before
the comparison.  A crafted nslot passes the bound and is then used as the
loop trip count in nd_label_data_init(), whose memset() walks off the end
of the config_size buffer: an out-of-bounds write.

The field is not trusted -- it comes from the medium, or from userspace
via ND_CMD_SET_CONFIG_DATA.  Evaluate the product in 64-bit so the bound
check is exact; conforming labels are unaffected.

The check was safe when introduced by commit 4a826c83db4e ("libnvdimm:
namespace indices: read and validate") -- it multiplied by sizeof(struct
nd_namespace_label), a size_t, so the product was 64-bit.  Commit
564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
narrowed it to 32 bits when the label size became a runtime value read
via sizeof_namespace_label().

Fixes: 564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
Cc: stable@vger.kernel.org
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/nvdimm/label.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index 4218e3ac4a2a..ec12ce72cfe2 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -202,7 +202,7 @@ static int __nd_label_validate(struct nvdimm_drvdata *ndd)
 		}
 
 		nslot = __le32_to_cpu(nsindex[i]->nslot);
-		if (nslot * sizeof_namespace_label(ndd)
+		if ((u64)nslot * sizeof_namespace_label(ndd)
 				+ 2 * sizeof_namespace_index(ndd)
 				> ndd->nsarea.config_size) {
 			dev_dbg(dev, "nsindex%d nslot: %u invalid, config_size: %#x\n",

-- 
2.43.0



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

* [PATCH v2 1/2] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
@ 2026-06-24  4:19   ` Bryam Vargas
  0 siblings, 0 replies; 8+ messages in thread
From: Bryam Vargas @ 2026-06-24  4:19 UTC (permalink / raw)
  To: Ira Weiny, Vishal Verma, Dan Williams, Dave Jiang
  Cc: David Laight, Alison Schofield, nvdimm, linux-kernel

The on-media namespace index field nslot is a u32 read from the DIMM
label storage area.  __nd_label_validate() bounds it against the config
area size, but sizeof_namespace_label() returns unsigned, so the product
nslot * label_size is evaluated in 32-bit and wraps modulo 2^32 before
the comparison.  A crafted nslot passes the bound and is then used as the
loop trip count in nd_label_data_init(), whose memset() walks off the end
of the config_size buffer: an out-of-bounds write.

The field is not trusted -- it comes from the medium, or from userspace
via ND_CMD_SET_CONFIG_DATA.  Evaluate the product in 64-bit so the bound
check is exact; conforming labels are unaffected.

The check was safe when introduced by commit 4a826c83db4e ("libnvdimm:
namespace indices: read and validate") -- it multiplied by sizeof(struct
nd_namespace_label), a size_t, so the product was 64-bit.  Commit
564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
narrowed it to 32 bits when the label size became a runtime value read
via sizeof_namespace_label().

Fixes: 564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
Cc: stable@vger.kernel.org
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/nvdimm/label.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index 4218e3ac4a2a..ec12ce72cfe2 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -202,7 +202,7 @@ static int __nd_label_validate(struct nvdimm_drvdata *ndd)
 		}
 
 		nslot = __le32_to_cpu(nsindex[i]->nslot);
-		if (nslot * sizeof_namespace_label(ndd)
+		if ((u64)nslot * sizeof_namespace_label(ndd)
 				+ 2 * sizeof_namespace_index(ndd)
 				> ndd->nsarea.config_size) {
 			dev_dbg(dev, "nsindex%d nslot: %u invalid, config_size: %#x\n",

-- 
2.43.0


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

* [PATCH v2 2/2] libnvdimm/labels: reject an implausibly large on-media slot count
  2026-06-24  4:19 ` Bryam Vargas
@ 2026-06-24  4:19   ` Bryam Vargas
  -1 siblings, 0 replies; 8+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-24  4:19 UTC (permalink / raw)
  To: Ira Weiny, Vishal Verma, Dan Williams, Dave Jiang
  Cc: David Laight, Alison Schofield, nvdimm, linux-kernel

From: Bryam Vargas <hexlabsecurity@proton.me>

Even with the bound evaluated in 64-bit, nslot is constrained only by
config_size, which nvdimm_init_nsarea() takes verbatim from the dimm's
ND_CMD_GET_CONFIG_SIZE response with no upper sanity check.  A firmware
or emulated device reporting a large config_size would let a
correspondingly large on-media nslot pass validation and drive a large
allocation and memset loop in nd_label_data_init().

Reject an nslot above NSINDEX_NSLOT_MAX (64K).  The largest legitimate
count is config_size / label_size -- about 1K on a real ~128K label area
-- so this cannot affect a conforming device, and it bounds the slot loop
independently of the firmware-reported config_size.

Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/nvdimm/label.c | 4 ++++
 drivers/nvdimm/label.h | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index ec12ce72cfe2..ce1e43d67bab 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -202,6 +202,10 @@ static int __nd_label_validate(struct nvdimm_drvdata *ndd)
 		}
 
 		nslot = __le32_to_cpu(nsindex[i]->nslot);
+		if (nslot > NSINDEX_NSLOT_MAX) {
+			dev_dbg(dev, "nsindex%d nslot: %u implausibly large\n", i, nslot);
+			continue;
+		}
 		if ((u64)nslot * sizeof_namespace_label(ndd)
 				+ 2 * sizeof_namespace_index(ndd)
 				> ndd->nsarea.config_size) {
diff --git a/drivers/nvdimm/label.h b/drivers/nvdimm/label.h
index 0650fb4b9821..74d7c1cc7476 100644
--- a/drivers/nvdimm/label.h
+++ b/drivers/nvdimm/label.h
@@ -28,6 +28,13 @@ enum {
 	ND_LABEL_MIN_SIZE = 256 * 4, /* see sizeof_namespace_index() */
 	ND_LABEL_ID_SIZE = 50,
 	ND_NSINDEX_INIT = 0x1,
+	/*
+	 * A sane ceiling on the on-media slot count.  The largest legitimate
+	 * value is config_size / label_size -- about 1K on a real ~128K label
+	 * area.  A count this large cannot describe a real device; it would
+	 * only drive a large allocation in nd_label_data_init(), so reject it.
+	 */
+	NSINDEX_NSLOT_MAX = 64 * 1024,
 };
 
 /**

-- 
2.43.0



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

* [PATCH v2 2/2] libnvdimm/labels: reject an implausibly large on-media slot count
@ 2026-06-24  4:19   ` Bryam Vargas
  0 siblings, 0 replies; 8+ messages in thread
From: Bryam Vargas @ 2026-06-24  4:19 UTC (permalink / raw)
  To: Ira Weiny, Vishal Verma, Dan Williams, Dave Jiang
  Cc: David Laight, Alison Schofield, nvdimm, linux-kernel

Even with the bound evaluated in 64-bit, nslot is constrained only by
config_size, which nvdimm_init_nsarea() takes verbatim from the dimm's
ND_CMD_GET_CONFIG_SIZE response with no upper sanity check.  A firmware
or emulated device reporting a large config_size would let a
correspondingly large on-media nslot pass validation and drive a large
allocation and memset loop in nd_label_data_init().

Reject an nslot above NSINDEX_NSLOT_MAX (64K).  The largest legitimate
count is config_size / label_size -- about 1K on a real ~128K label area
-- so this cannot affect a conforming device, and it bounds the slot loop
independently of the firmware-reported config_size.

Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/nvdimm/label.c | 4 ++++
 drivers/nvdimm/label.h | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index ec12ce72cfe2..ce1e43d67bab 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -202,6 +202,10 @@ static int __nd_label_validate(struct nvdimm_drvdata *ndd)
 		}
 
 		nslot = __le32_to_cpu(nsindex[i]->nslot);
+		if (nslot > NSINDEX_NSLOT_MAX) {
+			dev_dbg(dev, "nsindex%d nslot: %u implausibly large\n", i, nslot);
+			continue;
+		}
 		if ((u64)nslot * sizeof_namespace_label(ndd)
 				+ 2 * sizeof_namespace_index(ndd)
 				> ndd->nsarea.config_size) {
diff --git a/drivers/nvdimm/label.h b/drivers/nvdimm/label.h
index 0650fb4b9821..74d7c1cc7476 100644
--- a/drivers/nvdimm/label.h
+++ b/drivers/nvdimm/label.h
@@ -28,6 +28,13 @@ enum {
 	ND_LABEL_MIN_SIZE = 256 * 4, /* see sizeof_namespace_index() */
 	ND_LABEL_ID_SIZE = 50,
 	ND_NSINDEX_INIT = 0x1,
+	/*
+	 * A sane ceiling on the on-media slot count.  The largest legitimate
+	 * value is config_size / label_size -- about 1K on a real ~128K label
+	 * area.  A count this large cannot describe a real device; it would
+	 * only drive a large allocation in nd_label_data_init(), so reject it.
+	 */
+	NSINDEX_NSLOT_MAX = 64 * 1024,
 };
 
 /**

-- 
2.43.0


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

* Re: [PATCH v2 2/2] libnvdimm/labels: reject an implausibly large on-media slot count
  2026-06-24  4:19   ` Bryam Vargas
  (?)
@ 2026-06-24  4:30   ` sashiko-bot
  -1 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-06-24  4:30 UTC (permalink / raw)
  To: Bryam Vargas via B4 Relay; +Cc: nvdimm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The hardcoded cap of 64K for `nslot` breaks legitimate devices with large label storage areas and introduces a self-bricking behavior by rejecting natively generated label formats.
--

commit e9419d2283340d1b7069d74b83a7188e43fd5de9
Author: Bryam Vargas <hexlabsecurity@proton.me>

libnvdimm/labels: reject an implausibly large on-media slot count

This commit adds a 64K upper bound check on the nslot value read from the
media to prevent large allocations. It validates the nslot field in
__nd_label_validate() against the newly introduced NSINDEX_NSLOT_MAX.

diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index ec12ce72cfe2c..ce1e43d67babe 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -202,6 +202,10 @@ static int __nd_label_validate(struct nvdimm_drvdata *ndd)
 		}
 
 		nslot = __le32_to_cpu(nsindex[i]->nslot);
+		if (nslot > NSINDEX_NSLOT_MAX) {
+			dev_dbg(dev, "nsindex%d nslot: %u implausibly large\n", i, nslot);
+			continue;
+		}
[Severity: High]
Could this cause a regression by breaking legitimate devices with large
label areas?

If a device has a config_size larger than ~8.4MB, nvdimm_num_label_slots()
will naturally generate an nslot value greater than 64K.

When the kernel formats such a device in nd_label_write_index(), it writes
this unbounded nslot to the media:

drivers/nvdimm/label.c:nd_label_write_index() {
    ...
    if (flags & ND_NSINDEX_INIT)
        nslot = nvdimm_num_label_slots(ndd);
    ...
}

Will __nd_label_validate() then reject these newly initialized labels on the
next probe, making the namespaces inaccessible?

Also, does this check prevent the large allocation mentioned in the commit
message? Looking at nd_label_data_init(), the allocation appears to use
config_size directly before nslot is even read or validated.

 		if ((u64)nslot * sizeof_namespace_label(ndd)
 				+ 2 * sizeof_namespace_index(ndd)
 				> ndd->nsarea.config_size) {
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623-b4-disp-1f2c537a-v2-0-59af73f1f090@proton.me?part=2

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

* Re: [PATCH v2 1/2] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
  2026-06-24  4:19   ` Bryam Vargas
  (?)
@ 2026-06-24  4:31   ` sashiko-bot
  -1 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-06-24  4:31 UTC (permalink / raw)
  To: Bryam Vargas via B4 Relay; +Cc: nvdimm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The expression `1 << (7 + nsindex[i]->labelsize)` invokes C undefined behavior if the untrusted `nsindex[i]->labelsize` is maliciously large.
--

commit 1590d4e94c2716631030a170a8e554c454f824c4
Author: Bryam Vargas <hexlabsecurity@proton.me>

libnvdimm/labels: Prevent integer overflow in __nd_label_validate()

This commit fixes an out-of-bounds write in nd_label_data_init() by
evaluating the product of nslot and label_size as a 64-bit value. This
prevents a crafted nslot from bypassing the config area bounds check
due to a 32-bit integer overflow.

> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
> index 4218e3ac4a2ab..ec12ce72cfe2c 100644
> --- a/drivers/nvdimm/label.c
> +++ b/drivers/nvdimm/label.c

[Severity: High]
This is a pre-existing issue, but while looking at __nd_label_validate(),
is it possible for the labelsize shift to invoke undefined behavior?

The labelsize field is read directly from the untrusted label storage
medium:

drivers/nvdimm/label.c:__nd_label_validate() {
        ...
        if (version >= 102)
                labelsize = 1 << (7 + nsindex[i]->labelsize);
        else
        ...
}

If a malicious or corrupted medium provides a value of 25 or greater for
nsindex[i]->labelsize, 7 + labelsize evaluates to 32 or more. Because 1
is a 32-bit signed integer, shifting it by 31 bits causes a signed
integer overflow, and shifting by 32 or more bits exceeds the type width.

Could this trigger a kernel panic on systems with undefined behavior
sanitizers enabled, or lead to unpredictable behavior due to compiler
optimizations?

> @@ -202,7 +202,7 @@ static int __nd_label_validate(struct nvdimm_drvdata *ndd)
>  		}
>  
>  		nslot = __le32_to_cpu(nsindex[i]->nslot);
> -		if (nslot * sizeof_namespace_label(ndd)
> +		if ((u64)nslot * sizeof_namespace_label(ndd)
>  				+ 2 * sizeof_namespace_index(ndd)
>  				> ndd->nsarea.config_size) {
>  			dev_dbg(dev, "nsindex%d nslot: %u invalid, config_size: %#x\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623-b4-disp-1f2c537a-v2-0-59af73f1f090@proton.me?part=1

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

end of thread, other threads:[~2026-06-24  4:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-24  4:19 [PATCH v2 0/2] libnvdimm/labels: fix the nslot product overflow and cap the slot count Bryam Vargas via B4 Relay
2026-06-24  4:19 ` Bryam Vargas
2026-06-24  4:19 ` [PATCH v2 1/2] libnvdimm/labels: Prevent integer overflow in __nd_label_validate() Bryam Vargas via B4 Relay
2026-06-24  4:19   ` Bryam Vargas
2026-06-24  4:31   ` sashiko-bot
2026-06-24  4:19 ` [PATCH v2 2/2] libnvdimm/labels: reject an implausibly large on-media slot count Bryam Vargas via B4 Relay
2026-06-24  4:19   ` Bryam Vargas
2026-06-24  4:30   ` sashiko-bot

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.