* [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
@ 2026-06-20 20:54 Bryam Vargas via B4 Relay
2026-06-21 10:23 ` David Laight
2026-06-24 0:38 ` Alison Schofield
0 siblings, 2 replies; 5+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-20 20:54 UTC (permalink / raw)
To: Dave Jiang, Dan Williams, Ira Weiny, Vishal Verma; +Cc: linux-kernel, nvdimm
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.
Fixes: 564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
The check was safe when introduced: 4a826c83db4e ("libnvdimm: namespace
indices: read and validate") multiplied by sizeof(struct
nd_namespace_label), a size_t, so the product was 64-bit. 564e871aa66f
replaced that with sizeof_namespace_label(), which returns unsigned, when
the label size became a runtime value -- narrowing the product to 32 bits.
The sibling multiply in sizeof_namespace_index() uses an nslot derived
from config_size (nvdimm_num_label_slots()), not the on-media field, so it
cannot overflow and is left unchanged.
Reproduced with an out-of-tree module that mirrors nd_label_data_init() --
kvzalloc(config_size), the __nd_label_validate() bound check, and the
memset loop -- since the defect is the wrapped arithmetic into the memset,
not the DIMM-probe plumbing:
Build A (without this patch), nslot = 0x02000000, 128-byte labels:
the u32 product wraps to 0, the index is accepted, and the loop's
memset() writes past the kvzalloc'd buffer ->
right of the config_size region -> panic.
Build B (with this patch): the 64-bit product exceeds config_size, the
index is rejected, the loop never runs -> clean.
Control (legitimate small nslot): writes stay in bounds -> clean.
BUG: KASAN: slab-out-of-bounds, Write of size 128, 0 bytes to the
---
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",
---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260620-b4-disp-7f43b155-92b84c904c08
Best regards,
--
Bryam Vargas <hexlabsecurity@proton.me>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
2026-06-20 20:54 [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate() Bryam Vargas via B4 Relay
@ 2026-06-21 10:23 ` David Laight
2026-06-22 8:14 ` Bryam Vargas
2026-06-24 5:45 ` Bryam Vargas
2026-06-24 0:38 ` Alison Schofield
1 sibling, 2 replies; 5+ messages in thread
From: David Laight @ 2026-06-21 10:23 UTC (permalink / raw)
To: Bryam Vargas via B4 Relay
Cc: hexlabsecurity, Dave Jiang, Dan Williams, Ira Weiny,
Vishal Verma, linux-kernel, nvdimm
On Sat, 20 Jun 2026 15:54:39 -0500
Bryam Vargas via B4 Relay <devnull+hexlabsecurity.proton.me@kernel.org> wrote:
> 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.
Is this enough and/or a sane way to stop the overflow.
AFAICT label_size is either 128 or 258.
But I can't see where nsarea.config_size is set.
If it comes from a user ioctl there should be some associated sanity limits.
The same could be done for nslot - any value above 64k is pretty much
guaranteed to be garbage - I'd bet valid values are actually very small
integers.
David
>
> Fixes: 564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> The check was safe when introduced: 4a826c83db4e ("libnvdimm: namespace
> indices: read and validate") multiplied by sizeof(struct
> nd_namespace_label), a size_t, so the product was 64-bit. 564e871aa66f
> replaced that with sizeof_namespace_label(), which returns unsigned, when
> the label size became a runtime value -- narrowing the product to 32 bits.
>
> The sibling multiply in sizeof_namespace_index() uses an nslot derived
> from config_size (nvdimm_num_label_slots()), not the on-media field, so it
> cannot overflow and is left unchanged.
>
> Reproduced with an out-of-tree module that mirrors nd_label_data_init() --
> kvzalloc(config_size), the __nd_label_validate() bound check, and the
> memset loop -- since the defect is the wrapped arithmetic into the memset,
> not the DIMM-probe plumbing:
>
> Build A (without this patch), nslot = 0x02000000, 128-byte labels:
> the u32 product wraps to 0, the index is accepted, and the loop's
> memset() writes past the kvzalloc'd buffer ->
> right of the config_size region -> panic.
> Build B (with this patch): the 64-bit product exceeds config_size, the
> index is rejected, the loop never runs -> clean.
> Control (legitimate small nslot): writes stay in bounds -> clean.
>
> BUG: KASAN: slab-out-of-bounds, Write of size 128, 0 bytes to the
> ---
> 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",
>
> ---
> base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
> change-id: 20260620-b4-disp-7f43b155-92b84c904c08
>
> Best regards,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
2026-06-21 10:23 ` David Laight
@ 2026-06-22 8:14 ` Bryam Vargas
2026-06-24 5:45 ` Bryam Vargas
1 sibling, 0 replies; 5+ messages in thread
From: Bryam Vargas @ 2026-06-22 8:14 UTC (permalink / raw)
To: David Laight, Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny
Cc: nvdimm, linux-kernel
On 2026-06-21, David Laight wrote:
> Is this enough and/or a sane way to stop the overflow.
For the overflow, yes. In 64-bit the product no longer wraps, so the bound
rejects exactly the nslot values that don't fit config_size. Minimal change,
backports cleanly.
> AFAICT label_size is either 128 or 258.
128 or 256. nd_label_validate() probes label_size[] = { 128, 256 } (v1.1 / v1.2)
and sets ndd->nslabel_size from that.
> But I can't see where nsarea.config_size is set.
A u32 filled by nvdimm_init_nsarea() from ND_CMD_GET_CONFIG_SIZE -- reported by
the dimm provider's ->ndctl (firmware/_DSM on NFIT), not a user ioctl. No sanity
cap today.
> The same could be done for nslot - any value above 64k is pretty much
> guaranteed to be garbage
Agreed. The largest legitimate nslot is config_size / label_size: a few hundred
on a real ~128K area, ~1024 at most. The exact bound already ties nslot to
config_size; a ceiling still helps for the gap you point at: config_size is
firmware-reported and uncapped, so a bogus large config_size would otherwise
admit a large nslot and kvzalloc.
I'd keep the (u64) cast as the targeted fix here (Fixes:/stable) and add the
nslot and config_size bounds as a follow-up hardening patch, or fold them into a
v2 if you'd rather see them together. Either way I'll send it.
Bryam
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
2026-06-20 20:54 [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate() Bryam Vargas via B4 Relay
2026-06-21 10:23 ` David Laight
@ 2026-06-24 0:38 ` Alison Schofield
1 sibling, 0 replies; 5+ messages in thread
From: Alison Schofield @ 2026-06-24 0:38 UTC (permalink / raw)
To: hexlabsecurity
Cc: Dave Jiang, Dan Williams, Ira Weiny, Vishal Verma, linux-kernel, nvdimm
On Sat, Jun 20, 2026 at 03:54:39PM -0500, Bryam Vargas via B4 Relay wrote:
> 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.
Hi Bryam,
LGTM. The (u64) cast looks like the right minimal fix.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
wrt the new, or 2nd patch to bound nslot and cap config_size,that
you and David are discussing, when you say folding them into a v2,
that would be 2 separate patches, in a series, not on squashed patch.
Either way, separate or in series is OK by me.
I'd like to see a regression test added to the ndctl test suite.
You can add the case to test/label-compat.sh. It already inits labels
on the nfit_test bus and writes index/label blobs with write-labels,
then enable-region drives the path through __nd_label_validate(), which
is exactly what you're touching. Today it only covers conforming blobs,
so the truncation path has never been exercised. Which explains why
this has sat since 2017.
I think you can scope it as a single negative test case that covers
both patches with an oversize nslot. Don't worry about the config_size
cap. The nfit_test fixes config_size at SZ_128K, so it isn't testable
from userspace. Ask if you want any support with that.
-- Alison
>
> Fixes: 564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> The check was safe when introduced: 4a826c83db4e ("libnvdimm: namespace
> indices: read and validate") multiplied by sizeof(struct
> nd_namespace_label), a size_t, so the product was 64-bit. 564e871aa66f
> replaced that with sizeof_namespace_label(), which returns unsigned, when
> the label size became a runtime value -- narrowing the product to 32 bits.
>
> The sibling multiply in sizeof_namespace_index() uses an nslot derived
> from config_size (nvdimm_num_label_slots()), not the on-media field, so it
> cannot overflow and is left unchanged.
>
> Reproduced with an out-of-tree module that mirrors nd_label_data_init() --
> kvzalloc(config_size), the __nd_label_validate() bound check, and the
> memset loop -- since the defect is the wrapped arithmetic into the memset,
> not the DIMM-probe plumbing:
>
> Build A (without this patch), nslot = 0x02000000, 128-byte labels:
> the u32 product wraps to 0, the index is accepted, and the loop's
> memset() writes past the kvzalloc'd buffer ->
> right of the config_size region -> panic.
> Build B (with this patch): the 64-bit product exceeds config_size, the
> index is rejected, the loop never runs -> clean.
> Control (legitimate small nslot): writes stay in bounds -> clean.
>
> BUG: KASAN: slab-out-of-bounds, Write of size 128, 0 bytes to the
> ---
> 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",
>
> ---
> base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
> change-id: 20260620-b4-disp-7f43b155-92b84c904c08
>
> Best regards,
> --
> Bryam Vargas <hexlabsecurity@proton.me>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
2026-06-21 10:23 ` David Laight
2026-06-22 8:14 ` Bryam Vargas
@ 2026-06-24 5:45 ` Bryam Vargas
1 sibling, 0 replies; 5+ messages in thread
From: Bryam Vargas @ 2026-06-24 5:45 UTC (permalink / raw)
To: David Laight
Cc: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny,
Alison Schofield, nvdimm, linux-kernel
On 2026-06-21, David Laight wrote:
> The same could be done for nslot - any value above 64k is pretty much
> guaranteed to be garbage
I took that up in v2, but it does not hold against the code, so v3 drops it.
The allocation it was meant to bound -- ndd->data in nd_label_data_init() --
is kvzalloc(config_size), not nslot-derived, so capping nslot shrinks nothing.
And the cap is unsafe: on ND_NSINDEX_INIT the kernel writes
nslot = nvdimm_num_label_slots(ndd) = config_size / label_size, which is above
64K once config_size is past ~8.4MB. A 64K cap then rejects labels the kernel
itself wrote, so a freshly-formatted large device fails its own next probe.
The (u64) cast in patch 1 already makes the bound exact, so the overflow is
closed without the cap. v3 keeps the cast; the labelsize-shift UB the review
also turned up is a separate fix, not a stand-in for the cap.
Thanks,
Bryam
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-24 5:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-20 20:54 [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate() Bryam Vargas via B4 Relay
2026-06-21 10:23 ` David Laight
2026-06-22 8:14 ` Bryam Vargas
2026-06-24 5:45 ` Bryam Vargas
2026-06-24 0:38 ` Alison Schofield
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®