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 <[email protected]>

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/[email protected]?part=2

Reply via email to