On Sat, Jun 20, 2026 at 03:54:39PM -0500, Bryam Vargas via B4 Relay wrote: > From: Bryam Vargas <[email protected]> > > 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 <[email protected]> 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: [email protected] > Signed-off-by: Bryam Vargas <[email protected]> > --- > 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 <[email protected]> > > >
